diff --git a/talk/app/webrtc/webrtcsession.cc b/talk/app/webrtc/webrtcsession.cc index 111a76cf23..33c818da99 100644 --- a/talk/app/webrtc/webrtcsession.cc +++ b/talk/app/webrtc/webrtcsession.cc @@ -1403,8 +1403,7 @@ bool WebRtcSession::InsertDtmf(const std::string& track_id, LOG(LS_ERROR) << "InsertDtmf: Track does not exist: " << track_id; return false; } - if (!voice_channel_->InsertDtmf(send_ssrc, code, duration, - cricket::DF_SEND)) { + if (!voice_channel_->InsertDtmf(send_ssrc, code, duration)) { LOG(LS_ERROR) << "Failed to insert DTMF to channel."; return false; } diff --git a/talk/app/webrtc/webrtcsession_unittest.cc b/talk/app/webrtc/webrtcsession_unittest.cc index 3ddbf876be..fa48fd3c16 100644 --- a/talk/app/webrtc/webrtcsession_unittest.cc +++ b/talk/app/webrtc/webrtcsession_unittest.cc @@ -71,8 +71,6 @@ return; \ } -using cricket::DF_PLAY; -using cricket::DF_SEND; using cricket::FakeVoiceMediaChannel; using cricket::TransportInfo; using rtc::SocketAddress; @@ -3384,7 +3382,6 @@ TEST_F(WebRtcSessionTest, InsertDtmf) { EXPECT_EQ(0U, channel->dtmf_info_queue().size()); // Insert DTMF - const int expected_flags = DF_SEND; const int expected_duration = 90; session_->InsertDtmf(kAudioTrack1, 0, expected_duration); session_->InsertDtmf(kAudioTrack1, 1, expected_duration); @@ -3394,11 +3391,11 @@ TEST_F(WebRtcSessionTest, InsertDtmf) { ASSERT_EQ(3U, channel->dtmf_info_queue().size()); const uint32_t send_ssrc = channel->send_streams()[0].first_ssrc(); EXPECT_TRUE(CompareDtmfInfo(channel->dtmf_info_queue()[0], send_ssrc, 0, - expected_duration, expected_flags)); + expected_duration)); EXPECT_TRUE(CompareDtmfInfo(channel->dtmf_info_queue()[1], send_ssrc, 1, - expected_duration, expected_flags)); + expected_duration)); EXPECT_TRUE(CompareDtmfInfo(channel->dtmf_info_queue()[2], send_ssrc, 2, - expected_duration, expected_flags)); + expected_duration)); } // This test verifies the |initial_offerer| flag when session initiates the diff --git a/talk/media/base/fakemediaengine.h b/talk/media/base/fakemediaengine.h index 6c97b4a8ab..bc454e5b9f 100644 --- a/talk/media/base/fakemediaengine.h +++ b/talk/media/base/fakemediaengine.h @@ -229,15 +229,13 @@ template class RtpHelper : public Base { class FakeVoiceMediaChannel : public RtpHelper { public: struct DtmfInfo { - DtmfInfo(uint32_t ssrc, int event_code, int duration, int flags) + DtmfInfo(uint32_t ssrc, int event_code, int duration) : ssrc(ssrc), event_code(event_code), - duration(duration), - flags(flags) {} + duration(duration) {} uint32_t ssrc; int event_code; int duration; - int flags; }; explicit FakeVoiceMediaChannel(FakeVoiceEngine* engine, const AudioOptions& options) @@ -321,9 +319,8 @@ class FakeVoiceMediaChannel : public RtpHelper { } virtual bool InsertDtmf(uint32_t ssrc, int event_code, - int duration, - int flags) { - dtmf_info_queue_.push_back(DtmfInfo(ssrc, event_code, duration, flags)); + int duration) { + dtmf_info_queue_.push_back(DtmfInfo(ssrc, event_code, duration)); return true; } @@ -427,10 +424,9 @@ class FakeVoiceMediaChannel : public RtpHelper { inline bool CompareDtmfInfo(const FakeVoiceMediaChannel::DtmfInfo& info, uint32_t ssrc, int event_code, - int duration, - int flags) { + int duration) { return (info.duration == duration && info.event_code == event_code && - info.flags == flags && info.ssrc == ssrc); + info.ssrc == ssrc); } class FakeVideoMediaChannel : public RtpHelper { diff --git a/talk/media/base/mediachannel.h b/talk/media/base/mediachannel.h index f9ffffa8b9..2509cb025e 100644 --- a/talk/media/base/mediachannel.h +++ b/talk/media/base/mediachannel.h @@ -447,12 +447,6 @@ enum VoiceMediaChannelOptions { OPT_AGC_MINUS_10DB = 0x80000000 }; -// DTMF flags to control if a DTMF tone should be played and/or sent. -enum DtmfFlags { - DF_PLAY = 0x01, - DF_SEND = 0x02, -}; - class MediaChannel : public sigslot::has_slots<> { public: class NetworkInterface { @@ -1022,16 +1016,12 @@ class VoiceMediaChannel : public MediaChannel { // Set speaker output volume of the specified ssrc. virtual bool SetOutputVolume(uint32_t ssrc, double volume) = 0; // Returns if the telephone-event has been negotiated. - virtual bool CanInsertDtmf() { return false; } - // Send and/or play a DTMF |event| according to the |flags|. - // The DTMF out-of-band signal will be used on sending. + virtual bool CanInsertDtmf() = 0; + // Send a DTMF |event|. The DTMF out-of-band signal will be used. // The |ssrc| should be either 0 or a valid send stream ssrc. // The valid value for the |event| are 0 to 15 which corresponding to // DTMF event 0-9, *, #, A-D. - virtual bool InsertDtmf(uint32_t ssrc, - int event, - int duration, - int flags) = 0; + virtual bool InsertDtmf(uint32_t ssrc, int event, int duration) = 0; // Gets quality stats for the channel. virtual bool GetStats(VoiceMediaInfo* info) = 0; }; diff --git a/talk/media/webrtc/fakewebrtcvoiceengine.h b/talk/media/webrtc/fakewebrtcvoiceengine.h index 6ca334169c..92098634fc 100644 --- a/talk/media/webrtc/fakewebrtcvoiceengine.h +++ b/talk/media/webrtc/fakewebrtcvoiceengine.h @@ -560,7 +560,6 @@ class FakeWebRtcVoiceEngine channels_[channel]->dtmf_info.dtmf_length_ms = length_ms; return 0; } - WEBRTC_FUNC(SetSendTelephoneEventPayloadType, (int channel, unsigned char type)) { channels_[channel]->dtmf_type = type; @@ -568,16 +567,10 @@ class FakeWebRtcVoiceEngine }; WEBRTC_STUB(GetSendTelephoneEventPayloadType, (int channel, unsigned char& type)); - WEBRTC_STUB(SetDtmfFeedbackStatus, (bool enable, bool directFeedback)); WEBRTC_STUB(GetDtmfFeedbackStatus, (bool& enabled, bool& directFeedback)); - - WEBRTC_FUNC(PlayDtmfTone, - (int event_code, int length_ms = 200, int attenuation_db = 10)) { - dtmf_info_.dtmf_event_code = event_code; - dtmf_info_.dtmf_length_ms = length_ms; - return 0; - } + WEBRTC_STUB(PlayDtmfTone, + (int event_code, int length_ms = 200, int attenuation_db = 10)); // webrtc::VoEHardware WEBRTC_FUNC(GetNumOfRecordingDevices, (int& num)) { diff --git a/talk/media/webrtc/webrtcvoiceengine.cc b/talk/media/webrtc/webrtcvoiceengine.cc index 76c0c0106b..d1f76ef4b2 100644 --- a/talk/media/webrtc/webrtcvoiceengine.cc +++ b/talk/media/webrtc/webrtcvoiceengine.cc @@ -2285,45 +2285,32 @@ bool WebRtcVoiceMediaChannel::CanInsertDtmf() { return dtmf_allowed_; } -bool WebRtcVoiceMediaChannel::InsertDtmf(uint32_t ssrc, - int event, - int duration, - int flags) { +bool WebRtcVoiceMediaChannel::InsertDtmf(uint32_t ssrc, int event, + int duration) { RTC_DCHECK(worker_thread_checker_.CalledOnValidThread()); if (!dtmf_allowed_) { return false; } // Send the event. - if (flags & cricket::DF_SEND) { - int channel = -1; - if (ssrc == 0) { - if (send_streams_.size() > 0) { - channel = send_streams_.begin()->second->channel(); - } - } else { - channel = GetSendChannelId(ssrc); - } - if (channel == -1) { - LOG(LS_WARNING) << "InsertDtmf - The specified ssrc " - << ssrc << " is not in use."; - return false; - } - // Send DTMF using out-of-band DTMF. ("true", as 3rd arg) - if (engine()->voe()->dtmf()->SendTelephoneEvent( - channel, event, true, duration) == -1) { - LOG_RTCERR4(SendTelephoneEvent, channel, event, true, duration); - return false; + int channel = -1; + if (ssrc == 0) { + if (send_streams_.size() > 0) { + channel = send_streams_.begin()->second->channel(); } + } else { + channel = GetSendChannelId(ssrc); } - - // Play the event. - if (flags & cricket::DF_PLAY) { - // Play DTMF tone locally. - if (engine()->voe()->dtmf()->PlayDtmfTone(event, duration) == -1) { - LOG_RTCERR2(PlayDtmfTone, event, duration); - return false; - } + if (channel == -1) { + LOG(LS_WARNING) << "InsertDtmf - The specified ssrc " + << ssrc << " is not in use."; + return false; + } + // Send DTMF using out-of-band DTMF. ("true", as 3rd arg) + if (engine()->voe()->dtmf()->SendTelephoneEvent( + channel, event, true, duration) == -1) { + LOG_RTCERR4(SendTelephoneEvent, channel, event, true, duration); + return false; } return true; diff --git a/talk/media/webrtc/webrtcvoiceengine.h b/talk/media/webrtc/webrtcvoiceengine.h index 57c9439efc..843ff797de 100644 --- a/talk/media/webrtc/webrtcvoiceengine.h +++ b/talk/media/webrtc/webrtcvoiceengine.h @@ -199,7 +199,7 @@ class WebRtcVoiceMediaChannel final : public VoiceMediaChannel, bool SetOutputVolume(uint32_t ssrc, double volume) override; bool CanInsertDtmf() override; - bool InsertDtmf(uint32_t ssrc, int event, int duration, int flags) override; + bool InsertDtmf(uint32_t ssrc, int event, int duration) override; void OnPacketReceived(rtc::Buffer* packet, const rtc::PacketTime& packet_time) override; diff --git a/talk/media/webrtc/webrtcvoiceengine_unittest.cc b/talk/media/webrtc/webrtcvoiceengine_unittest.cc index 888de7d8a4..61f17db927 100644 --- a/talk/media/webrtc/webrtcvoiceengine_unittest.cc +++ b/talk/media/webrtc/webrtcvoiceengine_unittest.cc @@ -148,39 +148,26 @@ class WebRtcVoiceEngineTestFake : public testing::Test { EXPECT_TRUE(channel_->SetSendParameters(send_parameters_)); EXPECT_TRUE(channel_->SetSend(cricket::SEND_MICROPHONE)); EXPECT_FALSE(channel_->CanInsertDtmf()); - EXPECT_FALSE(channel_->InsertDtmf(ssrc, 1, 111, cricket::DF_SEND)); + EXPECT_FALSE(channel_->InsertDtmf(ssrc, 1, 111)); send_parameters_.codecs.push_back(kTelephoneEventCodec); EXPECT_TRUE(channel_->SetSendParameters(send_parameters_)); EXPECT_TRUE(channel_->CanInsertDtmf()); if (!caller) { // If this is callee, there's no active send channel yet. - EXPECT_FALSE(channel_->InsertDtmf(ssrc, 2, 123, cricket::DF_SEND)); + EXPECT_FALSE(channel_->InsertDtmf(ssrc, 2, 123)); EXPECT_TRUE(channel_->AddSendStream( cricket::StreamParams::CreateLegacy(kSsrc1))); } // Check we fail if the ssrc is invalid. - EXPECT_FALSE(channel_->InsertDtmf(-1, 1, 111, cricket::DF_SEND)); + EXPECT_FALSE(channel_->InsertDtmf(-1, 1, 111)); // Test send int channel_id = voe_.GetLastChannel(); EXPECT_FALSE(voe_.WasSendTelephoneEventCalled(channel_id, 2, 123)); - EXPECT_TRUE(channel_->InsertDtmf(ssrc, 2, 123, cricket::DF_SEND)); + EXPECT_TRUE(channel_->InsertDtmf(ssrc, 2, 123)); EXPECT_TRUE(voe_.WasSendTelephoneEventCalled(channel_id, 2, 123)); - - // Test play - EXPECT_FALSE(voe_.WasPlayDtmfToneCalled(3, 134)); - EXPECT_TRUE(channel_->InsertDtmf(ssrc, 3, 134, cricket::DF_PLAY)); - EXPECT_TRUE(voe_.WasPlayDtmfToneCalled(3, 134)); - - // Test send and play - EXPECT_FALSE(voe_.WasSendTelephoneEventCalled(channel_id, 4, 145)); - EXPECT_FALSE(voe_.WasPlayDtmfToneCalled(4, 145)); - EXPECT_TRUE(channel_->InsertDtmf(ssrc, 4, 145, - cricket::DF_PLAY | cricket::DF_SEND)); - EXPECT_TRUE(voe_.WasSendTelephoneEventCalled(channel_id, 4, 145)); - EXPECT_TRUE(voe_.WasPlayDtmfToneCalled(4, 145)); } // Test that send bandwidth is set correctly. diff --git a/talk/session/media/channel.cc b/talk/session/media/channel.cc index ef26704f1a..f83afa1ea4 100644 --- a/talk/session/media/channel.cc +++ b/talk/session/media/channel.cc @@ -1317,15 +1317,6 @@ void VoiceChannel::SetEarlyMedia(bool enable) { } } -bool VoiceChannel::PressDTMF(int digit, bool playout) { - int flags = DF_SEND; - if (playout) { - flags |= DF_PLAY; - } - int duration_ms = 160; - return InsertDtmf(0, digit, duration_ms, flags); -} - bool VoiceChannel::CanInsertDtmf() { return InvokeOnWorker(Bind(&VoiceMediaChannel::CanInsertDtmf, media_channel())); @@ -1333,10 +1324,9 @@ bool VoiceChannel::CanInsertDtmf() { bool VoiceChannel::InsertDtmf(uint32_t ssrc, int event_code, - int duration, - int flags) { + int duration) { return InvokeOnWorker(Bind(&VoiceChannel::InsertDtmf_w, this, - ssrc, event_code, duration, flags)); + ssrc, event_code, duration)); } bool VoiceChannel::SetOutputVolume(uint32_t ssrc, double volume) { @@ -1532,13 +1522,11 @@ void VoiceChannel::HandleEarlyMediaTimeout() { bool VoiceChannel::InsertDtmf_w(uint32_t ssrc, int event, - int duration, - int flags) { + int duration) { if (!enabled()) { return false; } - - return media_channel()->InsertDtmf(ssrc, event, duration, flags); + return media_channel()->InsertDtmf(ssrc, event, duration); } void VoiceChannel::OnMessage(rtc::Message *pmsg) { diff --git a/talk/session/media/channel.h b/talk/session/media/channel.h index 9342d31a25..ef0eb562f9 100644 --- a/talk/session/media/channel.h +++ b/talk/session/media/channel.h @@ -355,8 +355,6 @@ class VoiceChannel : public BaseChannel { // own ringing sound sigslot::signal1 SignalEarlyMediaTimeout; - // TODO(ronghuawu): Replace PressDTMF with InsertDtmf. - bool PressDTMF(int digit, bool playout); // Returns if the telephone-event has been negotiated. bool CanInsertDtmf(); // Send and/or play a DTMF |event| according to the |flags|. @@ -364,7 +362,7 @@ class VoiceChannel : public BaseChannel { // The |ssrc| should be either 0 or a valid send stream ssrc. // The valid value for the |event| are 0 which corresponding to DTMF // event 0-9, *, #, A-D. - bool InsertDtmf(uint32_t ssrc, int event_code, int duration, int flags); + bool InsertDtmf(uint32_t ssrc, int event_code, int duration); bool SetOutputVolume(uint32_t ssrc, double volume); // Get statistics about the current media session. bool GetStats(VoiceMediaInfo* stats); @@ -401,7 +399,7 @@ class VoiceChannel : public BaseChannel { ContentAction action, std::string* error_desc); void HandleEarlyMediaTimeout(); - bool InsertDtmf_w(uint32_t ssrc, int event, int duration, int flags); + bool InsertDtmf_w(uint32_t ssrc, int event, int duration); bool SetOutputVolume_w(uint32_t ssrc, double volume); bool GetStats_w(VoiceMediaInfo* stats); diff --git a/talk/session/media/channel_unittest.cc b/talk/session/media/channel_unittest.cc index 6866ecea77..08d83b5f6b 100644 --- a/talk/session/media/channel_unittest.cc +++ b/talk/session/media/channel_unittest.cc @@ -2112,23 +2112,6 @@ TEST_F(VoiceChannelTest, TestMediaMonitor) { Base::TestMediaMonitor(); } -// Test that PressDTMF properly forwards to the media channel. -TEST_F(VoiceChannelTest, TestDtmf) { - CreateChannels(0, 0); - EXPECT_TRUE(SendInitiate()); - EXPECT_TRUE(SendAccept()); - EXPECT_EQ(0U, media_channel1_->dtmf_info_queue().size()); - - EXPECT_TRUE(channel1_->PressDTMF(1, true)); - EXPECT_TRUE(channel1_->PressDTMF(8, false)); - - ASSERT_EQ(2U, media_channel1_->dtmf_info_queue().size()); - EXPECT_TRUE(CompareDtmfInfo(media_channel1_->dtmf_info_queue()[0], - 0, 1, 160, cricket::DF_PLAY | cricket::DF_SEND)); - EXPECT_TRUE(CompareDtmfInfo(media_channel1_->dtmf_info_queue()[1], - 0, 8, 160, cricket::DF_SEND)); -} - // Test that InsertDtmf properly forwards to the media channel. TEST_F(VoiceChannelTest, TestInsertDtmf) { CreateChannels(0, 0); @@ -2136,18 +2119,17 @@ TEST_F(VoiceChannelTest, TestInsertDtmf) { EXPECT_TRUE(SendAccept()); EXPECT_EQ(0U, media_channel1_->dtmf_info_queue().size()); - EXPECT_TRUE(channel1_->InsertDtmf(1, 3, 100, cricket::DF_SEND)); - EXPECT_TRUE(channel1_->InsertDtmf(2, 5, 110, cricket::DF_PLAY)); - EXPECT_TRUE(channel1_->InsertDtmf(3, 7, 120, - cricket::DF_PLAY | cricket::DF_SEND)); + EXPECT_TRUE(channel1_->InsertDtmf(1, 3, 100)); + EXPECT_TRUE(channel1_->InsertDtmf(2, 5, 110)); + EXPECT_TRUE(channel1_->InsertDtmf(3, 7, 120)); ASSERT_EQ(3U, media_channel1_->dtmf_info_queue().size()); EXPECT_TRUE(CompareDtmfInfo(media_channel1_->dtmf_info_queue()[0], - 1, 3, 100, cricket::DF_SEND)); + 1, 3, 100)); EXPECT_TRUE(CompareDtmfInfo(media_channel1_->dtmf_info_queue()[1], - 2, 5, 110, cricket::DF_PLAY)); + 2, 5, 110)); EXPECT_TRUE(CompareDtmfInfo(media_channel1_->dtmf_info_queue()[2], - 3, 7, 120, cricket::DF_PLAY | cricket::DF_SEND)); + 3, 7, 120)); } TEST_F(VoiceChannelTest, TestSetContentFailure) {