From 0a214ffa8ad5c2d52d0f2d20bf5f1d686994f552 Mon Sep 17 00:00:00 2001 From: "stefan@webrtc.org" Date: Wed, 3 Sep 2014 11:46:54 +0000 Subject: [PATCH] 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 --- .../rtp_rtcp/source/rtp_sender_audio.cc | 14 +++-- .../rtp_rtcp/source/rtp_sender_unittest.cc | 55 +++++++++++++++++++ 2 files changed, 63 insertions(+), 6 deletions(-) diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_audio.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_audio.cc index 99c008513..7efe98711 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 @@ int32_t RTPSenderAudio::SendAudio( static_cast(dtmfDurationSamples), false); } else { - // set markerBit on the first packet in the burst + if (SendTelephoneEventPacket( + ended, + _dtmfTimestamp, + static_cast(dtmfDurationSamples), + !_dtmfEventFirstPacketSent) != 0) { + return -1; + } _dtmfEventFirstPacketSent = true; - return SendTelephoneEventPacket( - ended, - _dtmfTimestamp, - static_cast(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 e9b01def4..7b62d0b8b 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 @@ TEST_F(RtpSenderAudioTest, SendAudioWithAudioLevelExtension) { 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 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;