Setting marker bit on DTMF correctly

BUG=1157
R=braveyao@webrtc.org, pbos@webrtc.org, stefan@webrtc.org, tina.legrand@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/22469004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@7037 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_audio.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_audio.cc
index 99c0085..7efe987 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_sender_audio.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_audio.cc
@@ -318,13 +318,15 @@
               static_cast<uint16_t>(dtmfDurationSamples),
               false);
         } else {
-          // set markerBit on the first packet in the burst
+          if (SendTelephoneEventPacket(
+                  ended,
+                  _dtmfTimestamp,
+                  static_cast<uint16_t>(dtmfDurationSamples),
+                  !_dtmfEventFirstPacketSent) != 0) {
+            return -1;
+          }
           _dtmfEventFirstPacketSent = true;
-          return SendTelephoneEventPacket(
-              ended,
-              _dtmfTimestamp,
-              static_cast<uint16_t>(dtmfDurationSamples),
-              !_dtmfEventFirstPacketSent);
+          return 0;
         }
       }
       return 0;
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc
index e9b01de..7b62d0b 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc
@@ -1073,6 +1073,61 @@
                       sizeof(extension)));
 }
 
+// As RFC4733, named telephone events are carried as part of the audio stream
+// and must use the same sequence number and timestamp base as the regular
+// audio channel.
+// This test checks the marker bit for the first packet and the consequent
+// packets of the same telephone event. Since it is specifically for DTMF
+// events, ignoring audio packets and sending kFrameEmpty instead of those.
+TEST_F(RtpSenderAudioTest, CheckMarkerBitForTelephoneEvents) {
+  char payload_name[RTP_PAYLOAD_NAME_SIZE] = "telephone-event";
+  uint8_t payload_type = 126;
+  ASSERT_EQ(0, rtp_sender_->RegisterPayload(payload_name, payload_type, 0,
+                                            0, 0));
+  // For Telephone events, payload is not added to the registered payload list,
+  // it will register only the payload used for audio stream.
+  // Registering the payload again for audio stream with different payload name.
+  strcpy(payload_name, "payload_name");
+  ASSERT_EQ(0, rtp_sender_->RegisterPayload(payload_name, payload_type, 8000,
+                                            1, 0));
+  int64_t capture_time_ms = fake_clock_.TimeInMilliseconds();
+  // DTMF event key=9, duration=500 and attenuationdB=10
+  rtp_sender_->SendTelephoneEvent(9, 500, 10);
+  // During start, it takes the starting timestamp as last sent timestamp.
+  // The duration is calculated as the difference of current and last sent
+  // timestamp. So for first call it will skip since the duration is zero.
+  ASSERT_EQ(0, rtp_sender_->SendOutgoingData(kFrameEmpty, payload_type,
+                                             capture_time_ms,
+                                             0, NULL, 0,
+                                             NULL));
+  // DTMF Sample Length is (Frequency/1000) * Duration.
+  // So in this case, it is (8000/1000) * 500 = 4000.
+  // Sending it as two packets.
+  ASSERT_EQ(0, rtp_sender_->SendOutgoingData(kFrameEmpty, payload_type,
+                                             capture_time_ms+2000,
+                                             0, NULL, 0,
+                                             NULL));
+  scoped_ptr<webrtc::RtpHeaderParser> rtp_parser(
+      webrtc::RtpHeaderParser::Create());
+  ASSERT_TRUE(rtp_parser.get() != NULL);
+  webrtc::RTPHeader rtp_header;
+  ASSERT_TRUE(rtp_parser->Parse(transport_.last_sent_packet_,
+                                transport_.last_sent_packet_len_,
+                                &rtp_header));
+  // Marker Bit should be set to 1 for first packet.
+  EXPECT_TRUE(rtp_header.markerBit);
+
+  ASSERT_EQ(0, rtp_sender_->SendOutgoingData(kFrameEmpty, payload_type,
+                                             capture_time_ms+4000,
+                                             0, NULL, 0,
+                                             NULL));
+  ASSERT_TRUE(rtp_parser->Parse(transport_.last_sent_packet_,
+                                transport_.last_sent_packet_len_,
+                                &rtp_header));
+  // Marker Bit should be set to 0 for rest of the packets.
+  EXPECT_FALSE(rtp_header.markerBit);
+}
+
 TEST_F(RtpSenderTest, BytesReportedCorrectly) {
   const char* kPayloadName = "GENERIC";
   const uint8_t kPayloadType = 127;