diff --git a/webrtc/api/call/audio_send_stream.h b/webrtc/api/call/audio_send_stream.h index 658c9de371..aab625e7fb 100644 --- a/webrtc/api/call/audio_send_stream.h +++ b/webrtc/api/call/audio_send_stream.h @@ -127,8 +127,8 @@ class AudioSendStream { virtual void Stop() = 0; // TODO(solenberg): Make payload_type a config property instead. - virtual bool SendTelephoneEvent(int payload_type, int event, - int duration_ms) = 0; + virtual bool SendTelephoneEvent(int payload_type, int payload_frequency, + int event, int duration_ms) = 0; virtual void SetMuted(bool muted) = 0; diff --git a/webrtc/audio/audio_send_stream.cc b/webrtc/audio/audio_send_stream.cc index a96eaf83f8..cb5fe0905c 100644 --- a/webrtc/audio/audio_send_stream.cc +++ b/webrtc/audio/audio_send_stream.cc @@ -135,10 +135,12 @@ void AudioSendStream::Stop() { } } -bool AudioSendStream::SendTelephoneEvent(int payload_type, int event, +bool AudioSendStream::SendTelephoneEvent(int payload_type, + int payload_frequency, int event, int duration_ms) { RTC_DCHECK(thread_checker_.CalledOnValidThread()); - return channel_proxy_->SetSendTelephoneEventPayloadType(payload_type) && + return channel_proxy_->SetSendTelephoneEventPayloadType(payload_type, + payload_frequency) && channel_proxy_->SendTelephoneEventOutband(event, duration_ms); } diff --git a/webrtc/audio/audio_send_stream.h b/webrtc/audio/audio_send_stream.h index bb619cf2b5..af0513ccf7 100644 --- a/webrtc/audio/audio_send_stream.h +++ b/webrtc/audio/audio_send_stream.h @@ -43,7 +43,7 @@ class AudioSendStream final : public webrtc::AudioSendStream, // webrtc::AudioSendStream implementation. void Start() override; void Stop() override; - bool SendTelephoneEvent(int payload_type, int event, + bool SendTelephoneEvent(int payload_type, int payload_frequency, int event, int duration_ms) override; void SetMuted(bool muted) override; webrtc::AudioSendStream::Stats GetStats() const override; diff --git a/webrtc/audio/audio_send_stream_unittest.cc b/webrtc/audio/audio_send_stream_unittest.cc index 2cb1275021..427cda31a0 100644 --- a/webrtc/audio/audio_send_stream_unittest.cc +++ b/webrtc/audio/audio_send_stream_unittest.cc @@ -47,6 +47,7 @@ const CallStatistics kCallStats = { 1345, 1678, 1901, 1234, 112, 13456, 17890, 1567, -1890, -1123}; const ReportBlock kReportBlock = {456, 780, 123, 567, 890, 132, 143, 13354}; const int kTelephoneEventPayloadType = 123; +const int kTelephoneEventPayloadFrequency = 65432; const int kTelephoneEventCode = 45; const int kTelephoneEventDuration = 6789; const CodecInst kIsacCodec = {103, "isac", 16000, 320, 1, 32000}; @@ -154,7 +155,8 @@ struct ConfigHelper { void SetupMockForSendTelephoneEvent() { EXPECT_TRUE(channel_proxy_); EXPECT_CALL(*channel_proxy_, - SetSendTelephoneEventPayloadType(kTelephoneEventPayloadType)) + SetSendTelephoneEventPayloadType(kTelephoneEventPayloadType, + kTelephoneEventPayloadFrequency)) .WillOnce(Return(true)); EXPECT_CALL(*channel_proxy_, SendTelephoneEventOutband(kTelephoneEventCode, kTelephoneEventDuration)) @@ -268,7 +270,8 @@ TEST(AudioSendStreamTest, SendTelephoneEvent) { helper.event_log()); helper.SetupMockForSendTelephoneEvent(); EXPECT_TRUE(send_stream.SendTelephoneEvent(kTelephoneEventPayloadType, - kTelephoneEventCode, kTelephoneEventDuration)); + kTelephoneEventPayloadFrequency, kTelephoneEventCode, + kTelephoneEventDuration)); } TEST(AudioSendStreamTest, SetMuted) { diff --git a/webrtc/media/engine/fakewebrtccall.cc b/webrtc/media/engine/fakewebrtccall.cc index d2aa7bc1b4..c07a2ebadd 100644 --- a/webrtc/media/engine/fakewebrtccall.cc +++ b/webrtc/media/engine/fakewebrtccall.cc @@ -40,9 +40,11 @@ FakeAudioSendStream::TelephoneEvent return latest_telephone_event_; } -bool FakeAudioSendStream::SendTelephoneEvent(int payload_type, int event, +bool FakeAudioSendStream::SendTelephoneEvent(int payload_type, + int payload_frequency, int event, int duration_ms) { latest_telephone_event_.payload_type = payload_type; + latest_telephone_event_.payload_frequency = payload_frequency; latest_telephone_event_.event_code = event; latest_telephone_event_.duration_ms = duration_ms; return true; diff --git a/webrtc/media/engine/fakewebrtccall.h b/webrtc/media/engine/fakewebrtccall.h index 7ff80ce0cc..c6b67cfc06 100644 --- a/webrtc/media/engine/fakewebrtccall.h +++ b/webrtc/media/engine/fakewebrtccall.h @@ -37,6 +37,7 @@ class FakeAudioSendStream final : public webrtc::AudioSendStream { public: struct TelephoneEvent { int payload_type = -1; + int payload_frequency = -1; int event_code = 0; int duration_ms = 0; }; @@ -54,7 +55,7 @@ class FakeAudioSendStream final : public webrtc::AudioSendStream { void Start() override { sending_ = true; } void Stop() override { sending_ = false; } - bool SendTelephoneEvent(int payload_type, int event, + bool SendTelephoneEvent(int payload_type, int payload_frequency, int event, int duration_ms) override; void SetMuted(bool muted) override; webrtc::AudioSendStream::Stats GetStats() const override; diff --git a/webrtc/media/engine/webrtcvoiceengine.cc b/webrtc/media/engine/webrtcvoiceengine.cc index 53c8e6fd0d..2c67c56406 100644 --- a/webrtc/media/engine/webrtcvoiceengine.cc +++ b/webrtc/media/engine/webrtcvoiceengine.cc @@ -1283,10 +1283,12 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream return true; } - bool SendTelephoneEvent(int payload_type, int event, int duration_ms) { + bool SendTelephoneEvent(int payload_type, int payload_freq, int event, + int duration_ms) { RTC_DCHECK(worker_thread_checker_.CalledOnValidThread()); RTC_DCHECK(stream_); - return stream_->SendTelephoneEvent(payload_type, event, duration_ms); + return stream_->SendTelephoneEvent(payload_type, payload_freq, event, + duration_ms); } void SetSend(bool send) { @@ -1876,20 +1878,31 @@ bool WebRtcVoiceMediaChannel::SetRecvCodecs( bool WebRtcVoiceMediaChannel::SetSendCodecs( const std::vector& codecs) { RTC_DCHECK(worker_thread_checker_.CalledOnValidThread()); - // TODO(solenberg): Validate input - that payload types don't overlap, are - // within range, filter out codecs we don't support, - // redundant codecs etc - the same way it is done for - // RtpHeaderExtensions. - - // Find the DTMF telephone event "codec" payload type. dtmf_payload_type_ = rtc::Optional(); + dtmf_payload_freq_ = -1; + + // Validate supplied codecs list. + for (const AudioCodec& codec : codecs) { + // TODO(solenberg): Validate more aspects of input - that payload types + // don't overlap, remove redundant/unsupported codecs etc - + // the same way it is done for RtpHeaderExtensions. + if (codec.id < kMinPayloadType || codec.id > kMaxPayloadType) { + LOG(LS_WARNING) << "Codec payload type out of range: " << ToString(codec); + return false; + } + } + + // Find PT of telephone-event codec with lowest clockrate, as a fallback, in + // case we don't have a DTMF codec with a rate matching the send codec's, or + // if this function returns early. + std::vector dtmf_codecs; for (const AudioCodec& codec : codecs) { if (IsCodec(codec, kDtmfCodecName)) { - if (codec.id < kMinPayloadType || codec.id > kMaxPayloadType) { - return false; + dtmf_codecs.push_back(codec); + if (!dtmf_payload_type_ || codec.clockrate < dtmf_payload_freq_) { + dtmf_payload_type_ = rtc::Optional(codec.id); + dtmf_payload_freq_ = codec.clockrate; } - dtmf_payload_type_ = rtc::Optional(codec.id); - break; } } @@ -1966,6 +1979,15 @@ bool WebRtcVoiceMediaChannel::SetSendCodecs( break; } } + + // Find the telephone-event PT exactly matching the preferred send codec. + for (const AudioCodec& dtmf_codec : dtmf_codecs) { + if (dtmf_codec.clockrate == codec->clockrate) { + dtmf_payload_type_ = rtc::Optional(dtmf_codec.id); + dtmf_payload_freq_ = dtmf_codec.clockrate; + break; + } + } } // Apply new settings to all streams. @@ -2373,7 +2395,9 @@ bool WebRtcVoiceMediaChannel::InsertDtmf(uint32_t ssrc, int event, LOG(LS_WARNING) << "DTMF event duration " << duration << " out of range."; return false; } - return it->second->SendTelephoneEvent(*dtmf_payload_type_, event, duration); + RTC_DCHECK_NE(-1, dtmf_payload_freq_); + return it->second->SendTelephoneEvent(*dtmf_payload_type_, dtmf_payload_freq_, + event, duration); } void WebRtcVoiceMediaChannel::OnPacketReceived( diff --git a/webrtc/media/engine/webrtcvoiceengine.h b/webrtc/media/engine/webrtcvoiceengine.h index a0cae71478..c80c06a894 100644 --- a/webrtc/media/engine/webrtcvoiceengine.h +++ b/webrtc/media/engine/webrtcvoiceengine.h @@ -245,9 +245,10 @@ class WebRtcVoiceMediaChannel final : public VoiceMediaChannel, int max_send_bitrate_bps_ = 0; AudioOptions options_; rtc::Optional dtmf_payload_type_; - bool desired_playout_ = false; + int dtmf_payload_freq_ = -1; bool recv_transport_cc_enabled_ = false; bool recv_nack_enabled_ = false; + bool desired_playout_ = false; bool playout_ = false; bool send_ = false; webrtc::Call* const call_ = nullptr; diff --git a/webrtc/media/engine/webrtcvoiceengine_unittest.cc b/webrtc/media/engine/webrtcvoiceengine_unittest.cc index 1b7b5697aa..3bab92ddc6 100644 --- a/webrtc/media/engine/webrtcvoiceengine_unittest.cc +++ b/webrtc/media/engine/webrtcvoiceengine_unittest.cc @@ -221,7 +221,8 @@ class WebRtcVoiceEngineTestFake : public testing::Test { EXPECT_TRUE(channel_->SetAudioSend(ssrc, enable, options, source)); } - void TestInsertDtmf(uint32_t ssrc, bool caller) { + void TestInsertDtmf(uint32_t ssrc, bool caller, + const cricket::AudioCodec& codec) { EXPECT_TRUE(SetupChannel()); if (caller) { // If this is a caller, local description will be applied and add the @@ -235,7 +236,7 @@ class WebRtcVoiceEngineTestFake : public testing::Test { SetSend(true); EXPECT_FALSE(channel_->CanInsertDtmf()); EXPECT_FALSE(channel_->InsertDtmf(ssrc, 1, 111)); - send_parameters_.codecs.push_back(kTelephoneEventCodec1); + send_parameters_.codecs.push_back(codec); SetSendParameters(send_parameters_); EXPECT_TRUE(channel_->CanInsertDtmf()); @@ -255,7 +256,8 @@ class WebRtcVoiceEngineTestFake : public testing::Test { EXPECT_EQ(-1, telephone_event.payload_type); EXPECT_TRUE(channel_->InsertDtmf(ssrc, 2, 123)); telephone_event = GetSendStream(kSsrc1).GetLatestTelephoneEvent(); - EXPECT_EQ(kTelephoneEventCodec1.id, telephone_event.payload_type); + EXPECT_EQ(codec.id, telephone_event.payload_type); + EXPECT_EQ(codec.clockrate, telephone_event.payload_frequency); EXPECT_EQ(2, telephone_event.event_code); EXPECT_EQ(123, telephone_event.duration_ms); } @@ -1884,7 +1886,7 @@ TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecsDTMFOnTop) { TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecsDTMFPayloadTypeOutOfRange) { EXPECT_TRUE(SetupSendStream()); cricket::AudioSendParameters parameters; - parameters.codecs.push_back(kTelephoneEventCodec1); + parameters.codecs.push_back(kTelephoneEventCodec2); parameters.codecs.push_back(kIsacCodec); parameters.codecs[0].id = 0; // DTMF parameters.codecs[1].id = 96; @@ -1952,7 +1954,7 @@ TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecsCNandDTMFAsCallee) { // TODO(juberti): cn 32000 parameters.codecs.push_back(kCn16000Codec); parameters.codecs.push_back(kCn8000Codec); - parameters.codecs.push_back(kTelephoneEventCodec1); + parameters.codecs.push_back(kTelephoneEventCodec2); parameters.codecs[0].id = 96; parameters.codecs[2].id = 97; // wideband CN parameters.codecs[4].id = 98; // DTMF @@ -2732,22 +2734,22 @@ TEST_F(WebRtcVoiceEngineTestFake, TestNoLeakingWhenAddRecvStreamFail) { // Test the InsertDtmf on default send stream as caller. TEST_F(WebRtcVoiceEngineTestFake, InsertDtmfOnDefaultSendStreamAsCaller) { - TestInsertDtmf(0, true); + TestInsertDtmf(0, true, kTelephoneEventCodec1); } // Test the InsertDtmf on default send stream as callee TEST_F(WebRtcVoiceEngineTestFake, InsertDtmfOnDefaultSendStreamAsCallee) { - TestInsertDtmf(0, false); + TestInsertDtmf(0, false, kTelephoneEventCodec2); } // Test the InsertDtmf on specified send stream as caller. TEST_F(WebRtcVoiceEngineTestFake, InsertDtmfOnSendStreamAsCaller) { - TestInsertDtmf(kSsrc1, true); + TestInsertDtmf(kSsrc1, true, kTelephoneEventCodec2); } // Test the InsertDtmf on specified send stream as callee. TEST_F(WebRtcVoiceEngineTestFake, InsertDtmfOnSendStreamAsCallee) { - TestInsertDtmf(kSsrc1, false); + TestInsertDtmf(kSsrc1, false, kTelephoneEventCodec1); } TEST_F(WebRtcVoiceEngineTestFake, SetAudioOptions) { diff --git a/webrtc/modules/rtp_rtcp/source/dtmf_queue.cc b/webrtc/modules/rtp_rtcp/source/dtmf_queue.cc index f73be079d5..7b1c61bdda 100644 --- a/webrtc/modules/rtp_rtcp/source/dtmf_queue.cc +++ b/webrtc/modules/rtp_rtcp/source/dtmf_queue.cc @@ -10,53 +10,38 @@ #include "webrtc/modules/rtp_rtcp/source/dtmf_queue.h" -#include +namespace { +constexpr size_t kDtmfOutbandMax = 20; +} namespace webrtc { -DTMFqueue::DTMFqueue() : next_empty_index_(0) { - memset(dtmf_key_, 0, sizeof(dtmf_key_)); - memset(dtmf_length, 0, sizeof(dtmf_length)); - memset(dtmf_level_, 0, sizeof(dtmf_level_)); -} +DtmfQueue::DtmfQueue() {} -DTMFqueue::~DTMFqueue() {} +DtmfQueue::~DtmfQueue() {} -int32_t DTMFqueue::AddDTMF(uint8_t key, uint16_t len, uint8_t level) { +bool DtmfQueue::AddDtmf(const Event& event) { rtc::CritScope lock(&dtmf_critsect_); - - if (next_empty_index_ >= DTMF_OUTBAND_MAX) { - return -1; + if (queue_.size() >= kDtmfOutbandMax) { + return false; } - int32_t index = next_empty_index_; - dtmf_key_[index] = key; - dtmf_length[index] = len; - dtmf_level_[index] = level; - next_empty_index_++; - return 0; + queue_.push_back(event); + return true; } -int8_t DTMFqueue::NextDTMF(uint8_t* dtmf_key, uint16_t* len, uint8_t* level) { +bool DtmfQueue::NextDtmf(Event* event) { + RTC_DCHECK(event); rtc::CritScope lock(&dtmf_critsect_); - if (next_empty_index_ == 0) - return -1; + if (queue_.empty()) { + return false; + } - *dtmf_key = dtmf_key_[0]; - *len = dtmf_length[0]; - *level = dtmf_level_[0]; - - memmove(&(dtmf_key_[0]), &(dtmf_key_[1]), - next_empty_index_ * sizeof(uint8_t)); - memmove(&(dtmf_length[0]), &(dtmf_length[1]), - next_empty_index_ * sizeof(uint16_t)); - memmove(&(dtmf_level_[0]), &(dtmf_level_[1]), - next_empty_index_ * sizeof(uint8_t)); - - next_empty_index_--; - return 0; + *event = queue_.front(); + queue_.pop_front(); + return true; } -bool DTMFqueue::PendingDTMF() { +bool DtmfQueue::PendingDtmf() const { rtc::CritScope lock(&dtmf_critsect_); - return next_empty_index_ > 0; + return !queue_.empty(); } } // namespace webrtc diff --git a/webrtc/modules/rtp_rtcp/source/dtmf_queue.h b/webrtc/modules/rtp_rtcp/source/dtmf_queue.h index fbfc4efcb1..af546dc230 100644 --- a/webrtc/modules/rtp_rtcp/source/dtmf_queue.h +++ b/webrtc/modules/rtp_rtcp/source/dtmf_queue.h @@ -11,26 +11,30 @@ #ifndef WEBRTC_MODULES_RTP_RTCP_SOURCE_DTMF_QUEUE_H_ #define WEBRTC_MODULES_RTP_RTCP_SOURCE_DTMF_QUEUE_H_ +#include + #include "webrtc/base/criticalsection.h" -#include "webrtc/modules/rtp_rtcp/source/rtp_rtcp_config.h" -#include "webrtc/typedefs.h" namespace webrtc { -class DTMFqueue { +class DtmfQueue { public: - DTMFqueue(); - ~DTMFqueue(); + struct Event { + uint16_t duration_ms = 0; + uint8_t payload_type = 0; + uint8_t key = 0; + uint8_t level = 0; + }; - int32_t AddDTMF(uint8_t dtmf_key, uint16_t len, uint8_t level); - int8_t NextDTMF(uint8_t* dtmf_key, uint16_t* len, uint8_t* level); - bool PendingDTMF(); + DtmfQueue(); + ~DtmfQueue(); + + bool AddDtmf(const Event& event); + bool NextDtmf(Event* event); + bool PendingDtmf() const; private: rtc::CriticalSection dtmf_critsect_; - uint8_t next_empty_index_; - uint8_t dtmf_key_[DTMF_OUTBAND_MAX]; - uint16_t dtmf_length[DTMF_OUTBAND_MAX]; - uint8_t dtmf_level_[DTMF_OUTBAND_MAX]; + std::list queue_; }; } // namespace webrtc diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_config.h b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_config.h index a2cd52736f..f09bf55617 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_config.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_config.h @@ -39,8 +39,6 @@ enum { BW_HISTORY_SIZE = 35 }; #define MIN_AUDIO_BW_MANAGEMENT_BITRATE 6 #define MIN_VIDEO_BW_MANAGEMENT_BITRATE 30 -enum { DTMF_OUTBAND_MAX = 20 }; - enum { RTP_MAX_BURST_SLEEP_TIME = 500 }; enum { RTP_AUDIO_LEVEL_UNIQUE_ID = 0xbede }; enum { RTP_MAX_PACKETS_PER_FRAME = 512 }; // must be multiple of 32 diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_audio.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_audio.cc index 63d2792038..69eb208f10 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_audio.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_audio.cc @@ -25,28 +25,9 @@ namespace webrtc { -static const int kDtmfFrequencyHz = 8000; - RTPSenderAudio::RTPSenderAudio(Clock* clock, RTPSender* rtp_sender) : clock_(clock), - rtp_sender_(rtp_sender), - packet_size_samples_(160), - dtmf_event_is_on_(false), - dtmf_event_first_packet_sent_(false), - dtmf_payload_type_(-1), - dtmf_timestamp_(0), - dtmf_key_(0), - dtmf_length_samples_(0), - dtmf_level_(0), - dtmf_time_last_sent_(0), - dtmf_timestamp_last_sent_(0), - inband_vad_active_(false), - cngnb_payload_type_(-1), - cngwb_payload_type_(-1), - cngswb_payload_type_(-1), - cngfb_payload_type_(-1), - last_payload_type_(-1), - audio_level_dbov_(0) {} + rtp_sender_(rtp_sender) {} RTPSenderAudio::~RTPSenderAudio() {} @@ -89,8 +70,8 @@ int32_t RTPSenderAudio::RegisterAudioPayload( // Don't add it to the list // we dont want to allow send with a DTMF payloadtype dtmf_payload_type_ = payload_type; + dtmf_payload_freq_ = frequency; return 0; - // The default timestamp rate is 8000 Hz, but other rates may be defined. } *payload = new RtpUtility::Payload; (*payload)->typeSpecific.Audio.frequency = frequency; @@ -151,31 +132,25 @@ bool RTPSenderAudio::SendAudio(FrameType frame_type, const uint8_t* payload_data, size_t payload_size, const RTPFragmentationHeader* fragmentation) { - // TODO(pwestin) Breakup function in smaller functions. - uint16_t dtmf_length_ms = 0; - uint8_t key = 0; - uint8_t audio_level_dbov; - int8_t dtmf_payload_type; - uint16_t packet_size_samples; + uint8_t audio_level_dbov = 0; + uint16_t packet_size_samples = 0; + uint32_t dtmf_payload_freq = 0; { rtc::CritScope cs(&send_audio_critsect_); audio_level_dbov = audio_level_dbov_; - dtmf_payload_type = dtmf_payload_type_; packet_size_samples = packet_size_samples_; + dtmf_payload_freq = dtmf_payload_freq_; } // Check if we have pending DTMFs to send - if (!dtmf_event_is_on_ && dtmf_queue_.PendingDTMF()) { - int64_t delaySinceLastDTMF = - clock_->TimeInMilliseconds() - dtmf_time_last_sent_; - - if (delaySinceLastDTMF > 100) { + if (!dtmf_event_is_on_ && dtmf_queue_.PendingDtmf()) { + if ((clock_->TimeInMilliseconds() - dtmf_time_last_sent_) > 50) { // New tone to play dtmf_timestamp_ = rtp_timestamp; - if (dtmf_queue_.NextDTMF(&key, &dtmf_length_ms, &dtmf_level_) >= 0) { + if (dtmf_queue_.NextDtmf(&dtmf_current_event_)) { dtmf_event_first_packet_sent_ = false; - dtmf_key_ = key; - dtmf_length_samples_ = (kDtmfFrequencyHz / 1000) * dtmf_length_ms; + dtmf_length_samples_ = + dtmf_current_event_.duration_ms * (dtmf_payload_freq / 1000); dtmf_event_is_on_ = true; } } @@ -211,7 +186,7 @@ bool RTPSenderAudio::SendAudio(FrameType frame_type, if (send) { if (dtmf_duration_samples > 0xffff) { // RFC 4733 2.5.2.3 Long-Duration Events - SendTelephoneEventPacket(ended, dtmf_payload_type, dtmf_timestamp_, + SendTelephoneEventPacket(ended, dtmf_timestamp_, static_cast(0xffff), false); // set new timestap for this segment @@ -219,11 +194,10 @@ bool RTPSenderAudio::SendAudio(FrameType frame_type, dtmf_duration_samples -= 0xffff; dtmf_length_samples_ -= 0xffff; - return SendTelephoneEventPacket( - ended, dtmf_payload_type, dtmf_timestamp_, + return SendTelephoneEventPacket(ended, dtmf_timestamp_, static_cast(dtmf_duration_samples), false); } else { - if (!SendTelephoneEventPacket(ended, dtmf_payload_type, dtmf_timestamp_, + if (!SendTelephoneEventPacket(ended, dtmf_timestamp_, dtmf_duration_samples, !dtmf_event_first_packet_sent_)) { return false; @@ -242,6 +216,7 @@ bool RTPSenderAudio::SendAudio(FrameType frame_type, } return false; } + std::unique_ptr packet = rtp_sender_->AllocatePacket(); packet->SetMarker(MarkerBit(frame_type, payload_type)); packet->SetPayloadType(payload_type); @@ -299,18 +274,22 @@ int32_t RTPSenderAudio::SetAudioLevel(uint8_t level_dbov) { int32_t RTPSenderAudio::SendTelephoneEvent(uint8_t key, uint16_t time_ms, uint8_t level) { + DtmfQueue::Event event; { rtc::CritScope lock(&send_audio_critsect_); if (dtmf_payload_type_ < 0) { // TelephoneEvent payloadtype not configured return -1; } + event.payload_type = dtmf_payload_type_; } - return dtmf_queue_.AddDTMF(key, time_ms, level); + event.key = key; + event.duration_ms = time_ms; + event.level = level; + return dtmf_queue_.AddDtmf(event) ? 0 : -1; } bool RTPSenderAudio::SendTelephoneEventPacket(bool ended, - int8_t dtmf_payload_type, uint32_t dtmf_timestamp, uint16_t duration, bool marker_bit) { @@ -327,7 +306,7 @@ bool RTPSenderAudio::SendTelephoneEventPacket(bool ended, constexpr size_t kDtmfSize = 4; std::unique_ptr packet( new RtpPacketToSend(kNoExtensions, kRtpHeaderSize + kDtmfSize)); - packet->SetPayloadType(dtmf_payload_type); + packet->SetPayloadType(dtmf_current_event_.payload_type); packet->SetMarker(marker_bit); packet->SetSsrc(rtp_sender_->SSRC()); packet->SetTimestamp(dtmf_timestamp); @@ -347,13 +326,13 @@ bool RTPSenderAudio::SendTelephoneEventPacket(bool ended, */ // R bit always cleared uint8_t R = 0x00; - uint8_t volume = dtmf_level_; + uint8_t volume = dtmf_current_event_.level; // First packet un-ended uint8_t E = ended ? 0x80 : 0x00; // First byte is Event number, equals key number - dtmfbuffer[0] = dtmf_key_; + dtmfbuffer[0] = dtmf_current_event_.key; dtmfbuffer[1] = E | R | volume; ByteWriter::WriteBigEndian(dtmfbuffer + 2, duration); diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_audio.h b/webrtc/modules/rtp_rtcp/source/rtp_sender_audio.h index aaee73956c..ee4831c634 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_audio.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_audio.h @@ -57,7 +57,6 @@ class RTPSenderAudio { protected: bool SendTelephoneEventPacket( bool ended, - int8_t dtmf_payload_type, uint32_t dtmf_timestamp, uint16_t duration, bool marker_bit); // set on first packet in talk burst @@ -65,36 +64,36 @@ class RTPSenderAudio { bool MarkerBit(FrameType frame_type, int8_t payload_type); private: - Clock* const clock_; - RTPSender* const rtp_sender_; + Clock* const clock_ = nullptr; + RTPSender* const rtp_sender_ = nullptr; rtc::CriticalSection send_audio_critsect_; - uint16_t packet_size_samples_ GUARDED_BY(send_audio_critsect_); + uint16_t packet_size_samples_ GUARDED_BY(send_audio_critsect_) = 160; // DTMF. - bool dtmf_event_is_on_; - bool dtmf_event_first_packet_sent_; - int8_t dtmf_payload_type_ GUARDED_BY(send_audio_critsect_); - uint32_t dtmf_timestamp_; - uint8_t dtmf_key_; - uint32_t dtmf_length_samples_; - uint8_t dtmf_level_; - int64_t dtmf_time_last_sent_; - uint32_t dtmf_timestamp_last_sent_; - DTMFqueue dtmf_queue_; + bool dtmf_event_is_on_ = false; + bool dtmf_event_first_packet_sent_ = false; + int8_t dtmf_payload_type_ GUARDED_BY(send_audio_critsect_) = -1; + uint32_t dtmf_payload_freq_ GUARDED_BY(send_audio_critsect_) = 8000; + uint32_t dtmf_timestamp_ = 0; + uint32_t dtmf_length_samples_ = 0; + int64_t dtmf_time_last_sent_ = 0; + uint32_t dtmf_timestamp_last_sent_ = 0; + DtmfQueue::Event dtmf_current_event_; + DtmfQueue dtmf_queue_; // VAD detection, used for marker bit. - bool inband_vad_active_ GUARDED_BY(send_audio_critsect_); - int8_t cngnb_payload_type_ GUARDED_BY(send_audio_critsect_); - int8_t cngwb_payload_type_ GUARDED_BY(send_audio_critsect_); - int8_t cngswb_payload_type_ GUARDED_BY(send_audio_critsect_); - int8_t cngfb_payload_type_ GUARDED_BY(send_audio_critsect_); - int8_t last_payload_type_ GUARDED_BY(send_audio_critsect_); + bool inband_vad_active_ GUARDED_BY(send_audio_critsect_) = false; + int8_t cngnb_payload_type_ GUARDED_BY(send_audio_critsect_) = -1; + int8_t cngwb_payload_type_ GUARDED_BY(send_audio_critsect_) = -1; + int8_t cngswb_payload_type_ GUARDED_BY(send_audio_critsect_) = -1; + int8_t cngfb_payload_type_ GUARDED_BY(send_audio_critsect_) = -1; + int8_t last_payload_type_ GUARDED_BY(send_audio_critsect_) = -1; // Audio level indication. // (https://datatracker.ietf.org/doc/draft-lennox-avt-rtp-audio-level-exthdr/) - uint8_t audio_level_dbov_ GUARDED_BY(send_audio_critsect_); + uint8_t audio_level_dbov_ GUARDED_BY(send_audio_critsect_) = 0; OneTimeEvent first_packet_sent_; RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(RTPSenderAudio); diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc index c86d92b307..d8f0baf9a6 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -1299,37 +1299,38 @@ TEST_F(RtpSenderAudioTest, SendAudioWithAudioLevelExtension) { // packets of the same telephone event. Since it is specifically for DTMF // events, ignoring audio packets and sending kEmptyFrame 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)); + const char* kDtmfPayloadName = "telephone-event"; + const uint32_t kPayloadFrequency = 8000; + const uint8_t kPayloadType = 126; + ASSERT_EQ(0, rtp_sender_->RegisterPayload(kDtmfPayloadName, kPayloadType, + kPayloadFrequency, 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. - const char kPayloadName[] = "payload_name"; - ASSERT_EQ( - 0, rtp_sender_->RegisterPayload(kPayloadName, payload_type, 8000, 1, 0)); + const char* kPayloadName = "payload_name"; + ASSERT_EQ(0, rtp_sender_->RegisterPayload(kPayloadName, kPayloadType, + kPayloadFrequency, 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_TRUE(rtp_sender_->SendOutgoingData(kEmptyFrame, payload_type, - capture_time_ms, 0, nullptr, 0, - nullptr, nullptr, nullptr)); + ASSERT_TRUE(rtp_sender_->SendOutgoingData(kEmptyFrame, kPayloadType, + capture_time_ms, 0, nullptr, 0, + nullptr, nullptr, nullptr)); // DTMF Sample Length is (Frequency/1000) * Duration. // So in this case, it is (8000/1000) * 500 = 4000. // Sending it as two packets. ASSERT_TRUE(rtp_sender_->SendOutgoingData( - kEmptyFrame, payload_type, capture_time_ms + 2000, 0, + kEmptyFrame, kPayloadType, capture_time_ms + 2000, 0, nullptr, 0, nullptr, nullptr, nullptr)); // Marker Bit should be set to 1 for first packet. EXPECT_TRUE(transport_.last_sent_packet().Marker()); ASSERT_TRUE(rtp_sender_->SendOutgoingData( - kEmptyFrame, payload_type, capture_time_ms + 4000, 0, + kEmptyFrame, kPayloadType, capture_time_ms + 4000, 0, nullptr, 0, nullptr, nullptr, nullptr)); // Marker Bit should be set to 0 for rest of the packets. EXPECT_FALSE(transport_.last_sent_packet().Marker()); diff --git a/webrtc/test/mock_voe_channel_proxy.h b/webrtc/test/mock_voe_channel_proxy.h index aea30e838e..3676e89153 100644 --- a/webrtc/test/mock_voe_channel_proxy.h +++ b/webrtc/test/mock_voe_channel_proxy.h @@ -42,7 +42,8 @@ class MockVoEChannelProxy : public voe::ChannelProxy { MOCK_CONST_METHOD0(GetDecodingCallStatistics, AudioDecodingCallStats()); MOCK_CONST_METHOD0(GetSpeechOutputLevelFullRange, int32_t()); MOCK_CONST_METHOD0(GetDelayEstimate, uint32_t()); - MOCK_METHOD1(SetSendTelephoneEventPayloadType, bool(int payload_type)); + MOCK_METHOD2(SetSendTelephoneEventPayloadType, bool(int payload_type, + int payload_frequency)); MOCK_METHOD2(SendTelephoneEventOutband, bool(int event, int duration_ms)); MOCK_METHOD1(SetBitrate, void(int bitrate_bps)); // TODO(solenberg): Talk the compiler into accepting this mock method: diff --git a/webrtc/voice_engine/channel.cc b/webrtc/voice_engine/channel.cc index 6a30fbce47..c1410fa8b8 100644 --- a/webrtc/voice_engine/channel.cc +++ b/webrtc/voice_engine/channel.cc @@ -2296,14 +2296,15 @@ int Channel::SendTelephoneEventOutband(int event, int duration_ms) { return 0; } -int Channel::SetSendTelephoneEventPayloadType(int payload_type) { +int Channel::SetSendTelephoneEventPayloadType(int payload_type, + int payload_frequency) { WEBRTC_TRACE(kTraceInfo, kTraceVoice, VoEId(_instanceId, _channelId), "Channel::SetSendTelephoneEventPayloadType()"); RTC_DCHECK_LE(0, payload_type); RTC_DCHECK_GE(127, payload_type); CodecInst codec = {0}; - codec.plfreq = 8000; codec.pltype = payload_type; + codec.plfreq = payload_frequency; memcpy(codec.plname, "telephone-event", 16); if (_rtpRtcpModule->RegisterSendPayload(codec) != 0) { _rtpRtcpModule->DeRegisterSendPayload(codec.pltype); diff --git a/webrtc/voice_engine/channel.h b/webrtc/voice_engine/channel.h index da020e6e5d..31c44efccd 100644 --- a/webrtc/voice_engine/channel.h +++ b/webrtc/voice_engine/channel.h @@ -286,7 +286,7 @@ class Channel // DTMF int SendTelephoneEventOutband(int event, int duration_ms); - int SetSendTelephoneEventPayloadType(int payload_type); + int SetSendTelephoneEventPayloadType(int payload_type, int payload_frequency); // VoEAudioProcessingImpl int VoiceActivityIndicator(int& activity); diff --git a/webrtc/voice_engine/channel_proxy.cc b/webrtc/voice_engine/channel_proxy.cc index bfd5b17b28..1d67f689a7 100644 --- a/webrtc/voice_engine/channel_proxy.cc +++ b/webrtc/voice_engine/channel_proxy.cc @@ -136,9 +136,11 @@ uint32_t ChannelProxy::GetDelayEstimate() const { return channel()->GetDelayEstimate(); } -bool ChannelProxy::SetSendTelephoneEventPayloadType(int payload_type) { +bool ChannelProxy::SetSendTelephoneEventPayloadType(int payload_type, + int payload_frequency) { RTC_DCHECK(thread_checker_.CalledOnValidThread()); - return channel()->SetSendTelephoneEventPayloadType(payload_type) == 0; + return channel()->SetSendTelephoneEventPayloadType(payload_type, + payload_frequency) == 0; } bool ChannelProxy::SendTelephoneEventOutband(int event, int duration_ms) { diff --git a/webrtc/voice_engine/channel_proxy.h b/webrtc/voice_engine/channel_proxy.h index 182e2524b5..6e0171b395 100644 --- a/webrtc/voice_engine/channel_proxy.h +++ b/webrtc/voice_engine/channel_proxy.h @@ -69,7 +69,8 @@ class ChannelProxy { virtual AudioDecodingCallStats GetDecodingCallStatistics() const; virtual int32_t GetSpeechOutputLevelFullRange() const; virtual uint32_t GetDelayEstimate() const; - virtual bool SetSendTelephoneEventPayloadType(int payload_type); + virtual bool SetSendTelephoneEventPayloadType(int payload_type, + int payload_frequency); virtual bool SendTelephoneEventOutband(int event, int duration_ms); virtual void SetBitrate(int bitrate_bps); virtual void SetSink(std::unique_ptr sink); diff --git a/webrtc/voice_engine/test/auto_test/standard/dtmf_test.cc b/webrtc/voice_engine/test/auto_test/standard/dtmf_test.cc index 896da037e3..d6dbf4b3d0 100644 --- a/webrtc/voice_engine/test/auto_test/standard/dtmf_test.cc +++ b/webrtc/voice_engine/test/auto_test/standard/dtmf_test.cc @@ -59,7 +59,8 @@ TEST_F(DtmfTest, ManualCanChangeDtmfPayloadType) { // Next, we must modify the sending side as well. EXPECT_TRUE( - channel_proxy_->SetSendTelephoneEventPayloadType(codec_instance.pltype)); + channel_proxy_->SetSendTelephoneEventPayloadType(codec_instance.pltype, + codec_instance.plfreq)); RunSixteenDtmfEvents(); }