From a1f66611dc2d44e2ac9be53f240bad85d5be1c73 Mon Sep 17 00:00:00 2001 From: Harald Alvestrand Date: Wed, 21 Feb 2018 11:24:23 +0100 Subject: [PATCH] Check that channel is in "send" before OKing DTMF This fix will ensure that attempts to send DTMF events before the channel is opened will return a failure rather than disappearing the event. Bug: webrtc:8908 Change-Id: I5044a0398dfd3dfe73b6ae1d48395e9809f81ad4 Reviewed-on: https://webrtc-review.googlesource.com/55480 Reviewed-by: Fredrik Solenberg Commit-Queue: Harald Alvestrand Cr-Commit-Position: refs/heads/master@{#22121} --- media/engine/webrtcvoiceengine.cc | 4 ++-- media/engine/webrtcvoiceengine_unittest.cc | 21 +++++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/media/engine/webrtcvoiceengine.cc b/media/engine/webrtcvoiceengine.cc index d96bb1d019..1c1b4e5b57 100644 --- a/media/engine/webrtcvoiceengine.cc +++ b/media/engine/webrtcvoiceengine.cc @@ -1933,14 +1933,14 @@ bool WebRtcVoiceMediaChannel::SetOutputVolume(uint32_t ssrc, double volume) { } bool WebRtcVoiceMediaChannel::CanInsertDtmf() { - return dtmf_payload_type_ ? true : false; + return dtmf_payload_type_.has_value() && send_; } bool WebRtcVoiceMediaChannel::InsertDtmf(uint32_t ssrc, int event, int duration) { RTC_DCHECK(worker_thread_checker_.CalledOnValidThread()); RTC_LOG(LS_INFO) << "WebRtcVoiceMediaChannel::InsertDtmf"; - if (!dtmf_payload_type_) { + if (!CanInsertDtmf()) { return false; } diff --git a/media/engine/webrtcvoiceengine_unittest.cc b/media/engine/webrtcvoiceengine_unittest.cc index f5fc9a0e38..31a092caf0 100644 --- a/media/engine/webrtcvoiceengine_unittest.cc +++ b/media/engine/webrtcvoiceengine_unittest.cc @@ -1775,9 +1775,26 @@ TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecsDTMFOnTop) { const auto& spec = *GetSendStreamConfig(kSsrcX).send_codec_spec; EXPECT_EQ(96, spec.payload_type); EXPECT_STRCASEEQ("ISAC", spec.format.name.c_str()); + SetSend(true); EXPECT_TRUE(channel_->CanInsertDtmf()); } +// Test that CanInsertDtmf() is governed by the send flag +TEST_F(WebRtcVoiceEngineTestFake, DTMFControlledBySendFlag) { + EXPECT_TRUE(SetupSendStream()); + cricket::AudioSendParameters parameters; + parameters.codecs.push_back(kTelephoneEventCodec1); + parameters.codecs.push_back(kPcmuCodec); + parameters.codecs[0].id = 98; // DTMF + parameters.codecs[1].id = 96; + SetSendParameters(parameters); + EXPECT_FALSE(channel_->CanInsertDtmf()); + SetSend(true); + EXPECT_TRUE(channel_->CanInsertDtmf()); + SetSend(false); + EXPECT_FALSE(channel_->CanInsertDtmf()); +} + // Test that payload type range is limited for telephone-event codec. TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecsDTMFPayloadTypeOutOfRange) { EXPECT_TRUE(SetupSendStream()); @@ -1787,6 +1804,7 @@ TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecsDTMFPayloadTypeOutOfRange) { parameters.codecs[0].id = 0; // DTMF parameters.codecs[1].id = 96; SetSendParameters(parameters); + SetSend(true); EXPECT_TRUE(channel_->CanInsertDtmf()); parameters.codecs[0].id = 128; // DTMF EXPECT_FALSE(channel_->SetSendParameters(parameters)); @@ -1835,6 +1853,7 @@ TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecsCNandDTMFAsCaller) { EXPECT_STRCASEEQ("ISAC", send_codec_spec.format.name.c_str()); EXPECT_EQ(1, send_codec_spec.format.num_channels); EXPECT_EQ(97, send_codec_spec.cng_payload_type); + SetSend(true); EXPECT_TRUE(channel_->CanInsertDtmf()); } @@ -1860,6 +1879,7 @@ TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecsCNandDTMFAsCallee) { EXPECT_STRCASEEQ("ISAC", send_codec_spec.format.name.c_str()); EXPECT_EQ(1, send_codec_spec.format.num_channels); EXPECT_EQ(97, send_codec_spec.cng_payload_type); + SetSend(true); EXPECT_TRUE(channel_->CanInsertDtmf()); } @@ -1925,6 +1945,7 @@ TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecsCaseInsensitive) { EXPECT_STRCASEEQ("ISAC", send_codec_spec.format.name.c_str()); EXPECT_EQ(1, send_codec_spec.format.num_channels); EXPECT_EQ(97, send_codec_spec.cng_payload_type); + SetSend(true); EXPECT_TRUE(channel_->CanInsertDtmf()); }