diff --git a/webrtc/api/rtpsenderinterface.h b/webrtc/api/rtpsenderinterface.h index 7129376d42..68547b09cf 100644 --- a/webrtc/api/rtpsenderinterface.h +++ b/webrtc/api/rtpsenderinterface.h @@ -17,6 +17,7 @@ #include #include +#include "webrtc/api/dtmfsenderinterface.h" #include "webrtc/api/mediatypes.h" #include "webrtc/api/mediastreaminterface.h" #include "webrtc/api/proxy.h" @@ -51,6 +52,9 @@ class RtpSenderInterface : public rtc::RefCountInterface { virtual RtpParameters GetParameters() const = 0; virtual bool SetParameters(const RtpParameters& parameters) = 0; + // Returns null for a video sender. + virtual rtc::scoped_refptr GetDtmfSender() const = 0; + protected: virtual ~RtpSenderInterface() {} }; @@ -66,6 +70,7 @@ BEGIN_SIGNALING_PROXY_MAP(RtpSender) PROXY_CONSTMETHOD0(std::vector, stream_ids) PROXY_CONSTMETHOD0(RtpParameters, GetParameters); PROXY_METHOD1(bool, SetParameters, const RtpParameters&) + PROXY_CONSTMETHOD0(rtc::scoped_refptr, GetDtmfSender); END_PROXY_MAP() } // namespace webrtc diff --git a/webrtc/api/test/mock_rtpsender.h b/webrtc/api/test/mock_rtpsender.h index 7abc4e5427..7458f45cd5 100644 --- a/webrtc/api/test/mock_rtpsender.h +++ b/webrtc/api/test/mock_rtpsender.h @@ -29,6 +29,7 @@ class MockRtpSender : public rtc::RefCountedObject { MOCK_CONST_METHOD0(stream_ids, std::vector()); MOCK_CONST_METHOD0(GetParameters, RtpParameters()); MOCK_METHOD1(SetParameters, bool(const RtpParameters&)); + MOCK_CONST_METHOD0(GetDtmfSender, rtc::scoped_refptr()); }; } // namespace webrtc diff --git a/webrtc/pc/dtmfsender.cc b/webrtc/pc/dtmfsender.cc index 2ef921bdc1..bbb100ea61 100644 --- a/webrtc/pc/dtmfsender.cc +++ b/webrtc/pc/dtmfsender.cc @@ -63,8 +63,8 @@ rtc::scoped_refptr DtmfSender::Create( AudioTrackInterface* track, rtc::Thread* signaling_thread, DtmfProviderInterface* provider) { - if (!track || !signaling_thread) { - return NULL; + if (!signaling_thread) { + return nullptr; } rtc::scoped_refptr dtmf_sender( new rtc::RefCountedObject(track, signaling_thread, @@ -81,7 +81,6 @@ DtmfSender::DtmfSender(AudioTrackInterface* track, provider_(provider), duration_(kDtmfDefaultDurationMs), inter_tone_gap_(kDtmfDefaultGapMs) { - RTC_DCHECK(track_ != NULL); RTC_DCHECK(signaling_thread_ != NULL); // TODO(deadbeef): Once we can use shared_ptr and weak_ptr, // do that instead of relying on a "destroyed" signal. @@ -109,7 +108,7 @@ bool DtmfSender::CanInsertDtmf() { if (!provider_) { return false; } - return provider_->CanInsertDtmf(track_->id()); + return provider_->CanInsertDtmf(); } bool DtmfSender::InsertDtmf(const std::string& tones, int duration, @@ -206,7 +205,7 @@ void DtmfSender::DoInsertDtmf() { } // The provider starts playout of the given tone on the // associated RTP media stream, using the appropriate codec. - if (!provider_->InsertDtmf(track_->id(), code, duration_)) { + if (!provider_->InsertDtmf(code, duration_)) { LOG(LS_ERROR) << "The DtmfProvider can no longer send DTMF."; return; } diff --git a/webrtc/pc/dtmfsender.h b/webrtc/pc/dtmfsender.h index a019e788e4..5eab055185 100644 --- a/webrtc/pc/dtmfsender.h +++ b/webrtc/pc/dtmfsender.h @@ -34,14 +34,13 @@ namespace webrtc { // to send DTMF. class DtmfProviderInterface { public: - // Returns true if the audio track with given id (|track_id|) is capable - // of sending DTMF. Otherwise returns false. - virtual bool CanInsertDtmf(const std::string& track_id) = 0; - // Sends DTMF |code| via the audio track with given id (|track_id|). + // Returns true if the audio sender is capable of sending DTMF. Otherwise + // returns false. + virtual bool CanInsertDtmf() = 0; + // Sends DTMF |code|. // The |duration| indicates the length of the DTMF tone in ms. // Returns true on success and false on failure. - virtual bool InsertDtmf(const std::string& track_id, - int code, int duration) = 0; + virtual bool InsertDtmf(int code, int duration) = 0; // Returns a |sigslot::signal0<>| signal. The signal should fire before // the provider is destroyed. virtual sigslot::signal0<>* GetOnDestroyedSignal() = 0; @@ -55,6 +54,8 @@ class DtmfSender public sigslot::has_slots<>, public rtc::MessageHandler { public: + // |track| is only there for backwards compatibility, since there's a track + // accessor method. static rtc::scoped_refptr Create( AudioTrackInterface* track, rtc::Thread* signaling_thread, diff --git a/webrtc/pc/dtmfsender_unittest.cc b/webrtc/pc/dtmfsender_unittest.cc index a58c1ec91f..109760c382 100644 --- a/webrtc/pc/dtmfsender_unittest.cc +++ b/webrtc/pc/dtmfsender_unittest.cc @@ -78,13 +78,9 @@ class FakeDtmfProvider : public DtmfProviderInterface { } // Implements DtmfProviderInterface. - bool CanInsertDtmf(const std::string& track_label) override { - return (can_insert_dtmf_tracks_.count(track_label) != 0); - } + bool CanInsertDtmf() override { return can_insert_; } - bool InsertDtmf(const std::string& track_label, - int code, - int duration) override { + bool InsertDtmf(int code, int duration) override { int gap = 0; // TODO(ronghuawu): Make the timer (basically the rtc::TimeNanos) // mockable and use a fake timer in the unit tests. @@ -110,15 +106,10 @@ class FakeDtmfProvider : public DtmfProviderInterface { } // helper functions - void AddCanInsertDtmfTrack(const std::string& label) { - can_insert_dtmf_tracks_.insert(label); - } - void RemoveCanInsertDtmfTrack(const std::string& label) { - can_insert_dtmf_tracks_.erase(label); - } + void SetCanInsertDtmf(bool can_insert) { can_insert_ = can_insert; } private: - std::set can_insert_dtmf_tracks_; + bool can_insert_ = false; std::vector dtmf_info_queue_; int64_t last_insert_dtmf_call_; sigslot::signal0<> SignalDestroyed; @@ -130,7 +121,7 @@ class DtmfSenderTest : public testing::Test { : track_(AudioTrack::Create(kTestAudioLabel, NULL)), observer_(new rtc::RefCountedObject()), provider_(new FakeDtmfProvider()) { - provider_->AddCanInsertDtmfTrack(kTestAudioLabel); + provider_->SetCanInsertDtmf(true); dtmf_ = DtmfSender::Create(track_, rtc::Thread::Current(), provider_.get()); dtmf_->RegisterObserver(observer_.get()); @@ -227,7 +218,7 @@ class DtmfSenderTest : public testing::Test { TEST_F(DtmfSenderTest, CanInsertDtmf) { EXPECT_TRUE(dtmf_->CanInsertDtmf()); - provider_->RemoveCanInsertDtmfTrack(kTestAudioLabel); + provider_->SetCanInsertDtmf(false); EXPECT_FALSE(dtmf_->CanInsertDtmf()); } @@ -333,7 +324,7 @@ TEST_F(DtmfSenderTest, TryInsertDtmfWhenItDoesNotWork) { std::string tones = "3,4"; int duration = 100; int inter_tone_gap = 50; - provider_->RemoveCanInsertDtmfTrack(kTestAudioLabel); + provider_->SetCanInsertDtmf(false); EXPECT_FALSE(dtmf_->InsertDtmf(tones, duration, inter_tone_gap)); } diff --git a/webrtc/pc/peerconnection.cc b/webrtc/pc/peerconnection.cc index 14e18095af..a38c98601b 100644 --- a/webrtc/pc/peerconnection.cc +++ b/webrtc/pc/peerconnection.cc @@ -949,20 +949,15 @@ rtc::scoped_refptr PeerConnection::CreateDtmfSender( } if (!track) { LOG(LS_ERROR) << "CreateDtmfSender - track is NULL."; - return NULL; + return nullptr; } - if (!local_streams_->FindAudioTrack(track->id())) { - LOG(LS_ERROR) << "CreateDtmfSender is called with a non local audio track."; - return NULL; + auto it = FindSenderForTrack(track); + if (it == senders_.end()) { + LOG(LS_ERROR) << "CreateDtmfSender called with a non-added track."; + return nullptr; } - rtc::scoped_refptr sender( - DtmfSender::Create(track, signaling_thread(), session_.get())); - if (!sender.get()) { - LOG(LS_ERROR) << "CreateDtmfSender failed on DtmfSender::Create."; - return NULL; - } - return DtmfSenderProxy::Create(signaling_thread(), sender.get()); + return (*it)->GetDtmfSender(); } rtc::scoped_refptr PeerConnection::CreateSender( diff --git a/webrtc/pc/rtpsender.cc b/webrtc/pc/rtpsender.cc index 3e8c7e122e..c8ac830bf3 100644 --- a/webrtc/pc/rtpsender.cc +++ b/webrtc/pc/rtpsender.cc @@ -57,6 +57,7 @@ AudioRtpSender::AudioRtpSender(AudioTrackInterface* track, sink_adapter_(new LocalAudioSinkAdapter()) { track_->RegisterObserver(this); track_->AddSink(sink_adapter_.get()); + CreateDtmfSender(); } AudioRtpSender::AudioRtpSender(AudioTrackInterface* track, @@ -71,6 +72,7 @@ AudioRtpSender::AudioRtpSender(AudioTrackInterface* track, sink_adapter_(new LocalAudioSinkAdapter()) { track_->RegisterObserver(this); track_->AddSink(sink_adapter_.get()); + CreateDtmfSender(); } AudioRtpSender::AudioRtpSender(cricket::VoiceChannel* channel, @@ -79,12 +81,50 @@ AudioRtpSender::AudioRtpSender(cricket::VoiceChannel* channel, stream_id_(rtc::CreateRandomUuid()), channel_(channel), stats_(stats), - sink_adapter_(new LocalAudioSinkAdapter()) {} + sink_adapter_(new LocalAudioSinkAdapter()) { + CreateDtmfSender(); +} AudioRtpSender::~AudioRtpSender() { + // For DtmfSender. + SignalDestroyed(); Stop(); } +bool AudioRtpSender::CanInsertDtmf() { + if (!channel_) { + LOG(LS_ERROR) << "CanInsertDtmf: No audio channel exists."; + return false; + } + // Check that this RTP sender is active (description has been applied that + // matches an SSRC to its ID). + if (!ssrc_) { + LOG(LS_ERROR) << "CanInsertDtmf: Sender does not have SSRC."; + return false; + } + return channel_->CanInsertDtmf(); +} + +bool AudioRtpSender::InsertDtmf(int code, int duration) { + if (!channel_) { + LOG(LS_ERROR) << "CanInsertDtmf: No audio channel exists."; + return false; + } + if (!ssrc_) { + LOG(LS_ERROR) << "CanInsertDtmf: Sender does not have SSRC."; + return false; + } + if (!channel_->InsertDtmf(ssrc_, code, duration)) { + LOG(LS_ERROR) << "Failed to insert DTMF to channel."; + return false; + } + return true; +} + +sigslot::signal0<>* AudioRtpSender::GetOnDestroyedSignal() { + return &SignalDestroyed; +} + void AudioRtpSender::OnChanged() { TRACE_EVENT0("webrtc", "AudioRtpSender::OnChanged"); RTC_DCHECK(!stopped_); @@ -158,6 +198,10 @@ bool AudioRtpSender::SetParameters(const RtpParameters& parameters) { return channel_->SetRtpSendParameters(ssrc_, parameters); } +rtc::scoped_refptr AudioRtpSender::GetDtmfSender() const { + return dtmf_sender_proxy_; +} + void AudioRtpSender::SetSsrc(uint32_t ssrc) { TRACE_EVENT0("webrtc", "AudioRtpSender::SetSsrc"); if (stopped_ || ssrc == ssrc_) { @@ -237,6 +281,20 @@ void AudioRtpSender::ClearAudioSend() { } } +void AudioRtpSender::CreateDtmfSender() { + // Should be on signaling thread. + // TODO(deadbeef): Add thread checking to RtpSender/RtpReceiver + // implementations. + rtc::scoped_refptr sender( + DtmfSender::Create(track_, rtc::Thread::Current(), this)); + if (!sender.get()) { + LOG(LS_ERROR) << "CreateDtmfSender failed on DtmfSender::Create."; + RTC_NOTREACHED(); + } + dtmf_sender_proxy_ = + DtmfSenderProxy::Create(rtc::Thread::Current(), sender.get()); +} + VideoRtpSender::VideoRtpSender(VideoTrackInterface* track, const std::string& stream_id, cricket::VideoChannel* channel) @@ -336,6 +394,11 @@ bool VideoRtpSender::SetParameters(const RtpParameters& parameters) { return channel_->SetRtpSendParameters(ssrc_, parameters); } +rtc::scoped_refptr VideoRtpSender::GetDtmfSender() const { + LOG(LS_ERROR) << "Tried to get DTMF sender from video sender."; + return nullptr; +} + void VideoRtpSender::SetSsrc(uint32_t ssrc) { TRACE_EVENT0("webrtc", "VideoRtpSender::SetSsrc"); if (stopped_ || ssrc == ssrc_) { diff --git a/webrtc/pc/rtpsender.h b/webrtc/pc/rtpsender.h index ed244b416a..0322399d5e 100644 --- a/webrtc/pc/rtpsender.h +++ b/webrtc/pc/rtpsender.h @@ -24,6 +24,7 @@ #include "webrtc/base/criticalsection.h" #include "webrtc/media/base/audiosource.h" #include "webrtc/pc/channel.h" +#include "webrtc/pc/dtmfsender.h" #include "webrtc/pc/statscollector.h" namespace webrtc { @@ -68,7 +69,8 @@ class LocalAudioSinkAdapter : public AudioTrackSinkInterface, rtc::CriticalSection lock_; }; -class AudioRtpSender : public ObserverInterface, +class AudioRtpSender : public DtmfProviderInterface, + public ObserverInterface, public rtc::RefCountedObject { public: // StatsCollector provided so that Add/RemoveLocalAudioTrack can be called @@ -91,10 +93,15 @@ class AudioRtpSender : public ObserverInterface, virtual ~AudioRtpSender(); - // ObserverInterface implementation + // DtmfSenderProvider implementation. + bool CanInsertDtmf() override; + bool InsertDtmf(int code, int duration) override; + sigslot::signal0<>* GetOnDestroyedSignal() override; + + // ObserverInterface implementation. void OnChanged() override; - // RtpSenderInterface implementation + // RtpSenderInterface implementation. bool SetTrack(MediaStreamTrackInterface* track) override; rtc::scoped_refptr track() const override { return track_; @@ -116,6 +123,8 @@ class AudioRtpSender : public ObserverInterface, RtpParameters GetParameters() const override; bool SetParameters(const RtpParameters& parameters) override; + rtc::scoped_refptr GetDtmfSender() const override; + // RtpSenderInternal implementation. void SetSsrc(uint32_t ssrc) override; @@ -140,11 +149,16 @@ class AudioRtpSender : public ObserverInterface, // Helper function to call SetAudioSend with "stop sending" parameters. void ClearAudioSend(); + void CreateDtmfSender(); + + sigslot::signal0<> SignalDestroyed; + std::string id_; std::string stream_id_; cricket::VoiceChannel* channel_ = nullptr; StatsCollector* stats_; rtc::scoped_refptr track_; + rtc::scoped_refptr dtmf_sender_proxy_; uint32_t ssrc_ = 0; bool cached_track_enabled_ = false; bool stopped_ = false; @@ -197,6 +211,8 @@ class VideoRtpSender : public ObserverInterface, RtpParameters GetParameters() const override; bool SetParameters(const RtpParameters& parameters) override; + rtc::scoped_refptr GetDtmfSender() const override; + // RtpSenderInternal implementation. void SetSsrc(uint32_t ssrc) override; diff --git a/webrtc/pc/rtpsenderreceiver_unittest.cc b/webrtc/pc/rtpsenderreceiver_unittest.cc index 86c0612114..508a417733 100644 --- a/webrtc/pc/rtpsenderreceiver_unittest.cc +++ b/webrtc/pc/rtpsenderreceiver_unittest.cc @@ -13,6 +13,7 @@ #include #include "webrtc/base/gunit.h" +#include "webrtc/base/sigslot.h" #include "webrtc/logging/rtc_event_log/rtc_event_log.h" #include "webrtc/media/base/fakemediaengine.h" #include "webrtc/media/base/mediachannel.h" @@ -38,6 +39,8 @@ using ::testing::Exactly; using ::testing::InvokeWithoutArgs; using ::testing::Return; +namespace { + static const char kStreamLabel1[] = "local_stream_1"; static const char kVideoTrackId[] = "video_1"; static const char kAudioTrackId[] = "audio_1"; @@ -45,10 +48,14 @@ static const uint32_t kVideoSsrc = 98; static const uint32_t kVideoSsrc2 = 100; static const uint32_t kAudioSsrc = 99; static const uint32_t kAudioSsrc2 = 101; +static const int kDefaultTimeout = 10000; // 10 seconds. + +} // namespace namespace webrtc { -class RtpSenderReceiverTest : public testing::Test { +class RtpSenderReceiverTest : public testing::Test, + public sigslot::has_slots<> { public: RtpSenderReceiverTest() : // Create fake media engine/etc. so we can create channels to use to @@ -75,6 +82,8 @@ class RtpSenderReceiverTest : public testing::Test { &fake_media_controller_, rtp_transport, nullptr, rtc::Thread::Current(), cricket::CN_VIDEO, nullptr, rtcp_mux_required, srtp_required, cricket::VideoOptions()); + voice_channel_->Enable(true); + video_channel_->Enable(true); voice_media_channel_ = media_engine_->GetVoiceChannel(0); video_media_channel_ = media_engine_->GetVideoChannel(0); RTC_CHECK(voice_channel_); @@ -104,7 +113,14 @@ class RtpSenderReceiverTest : public testing::Test { cricket::StreamParams::CreateLegacy(kVideoSsrc2)); } - void TearDown() override { channel_manager_.Terminate(); } + // Needed to use DTMF sender. + void AddDtmfCodec() { + cricket::AudioSendParameters params; + const cricket::AudioCodec kTelephoneEventCodec(106, "telephone-event", 8000, + 0, 1); + params.codecs.push_back(kTelephoneEventCodec); + voice_media_channel_->SetSendParameters(params); + } void AddVideoTrack() { AddVideoTrack(false); } @@ -124,9 +140,13 @@ class RtpSenderReceiverTest : public testing::Test { new AudioRtpSender(stream_->GetAudioTracks()[0], stream_->label(), voice_channel_, nullptr); audio_rtp_sender_->SetSsrc(kAudioSsrc); + audio_rtp_sender_->GetOnDestroyedSignal()->connect( + this, &RtpSenderReceiverTest::OnAudioSenderDestroyed); VerifyVoiceChannelInput(); } + void OnAudioSenderDestroyed() { audio_sender_destroyed_signal_fired_ = true; } + void CreateVideoRtpSender() { CreateVideoRtpSender(false); } void CreateVideoRtpSender(bool is_screencast) { @@ -247,6 +267,7 @@ class RtpSenderReceiverTest : public testing::Test { rtc::scoped_refptr stream_; rtc::scoped_refptr video_track_; rtc::scoped_refptr audio_track_; + bool audio_sender_destroyed_signal_fired_ = false; }; // Test that |voice_channel_| is updated when an audio track is associated @@ -721,4 +742,66 @@ TEST_F(RtpSenderReceiverTest, DestroyVideoRtpSender(); } +TEST_F(RtpSenderReceiverTest, AudioSenderHasDtmfSender) { + CreateAudioRtpSender(); + EXPECT_NE(nullptr, audio_rtp_sender_->GetDtmfSender()); +} + +TEST_F(RtpSenderReceiverTest, VideoSenderDoesNotHaveDtmfSender) { + CreateVideoRtpSender(); + EXPECT_EQ(nullptr, video_rtp_sender_->GetDtmfSender()); +} + +// Test that the DTMF sender is really using |voice_channel_|, and thus returns +// true/false from CanSendDtmf based on what |voice_channel_| returns. +TEST_F(RtpSenderReceiverTest, CanInsertDtmf) { + AddDtmfCodec(); + CreateAudioRtpSender(); + auto dtmf_sender = audio_rtp_sender_->GetDtmfSender(); + ASSERT_NE(nullptr, dtmf_sender); + EXPECT_TRUE(dtmf_sender->CanInsertDtmf()); +} + +TEST_F(RtpSenderReceiverTest, CanNotInsertDtmf) { + CreateAudioRtpSender(); + auto dtmf_sender = audio_rtp_sender_->GetDtmfSender(); + ASSERT_NE(nullptr, dtmf_sender); + // DTMF codec has not been added, as it was in the above test. + EXPECT_FALSE(dtmf_sender->CanInsertDtmf()); +} + +TEST_F(RtpSenderReceiverTest, InsertDtmf) { + AddDtmfCodec(); + CreateAudioRtpSender(); + auto dtmf_sender = audio_rtp_sender_->GetDtmfSender(); + ASSERT_NE(nullptr, dtmf_sender); + + EXPECT_EQ(0U, voice_media_channel_->dtmf_info_queue().size()); + + // Insert DTMF + const int expected_duration = 90; + dtmf_sender->InsertDtmf("012", expected_duration, 100); + + // Verify + ASSERT_EQ_WAIT(3U, voice_media_channel_->dtmf_info_queue().size(), + kDefaultTimeout); + const uint32_t send_ssrc = + voice_media_channel_->send_streams()[0].first_ssrc(); + EXPECT_TRUE(CompareDtmfInfo(voice_media_channel_->dtmf_info_queue()[0], + send_ssrc, 0, expected_duration)); + EXPECT_TRUE(CompareDtmfInfo(voice_media_channel_->dtmf_info_queue()[1], + send_ssrc, 1, expected_duration)); + EXPECT_TRUE(CompareDtmfInfo(voice_media_channel_->dtmf_info_queue()[2], + send_ssrc, 2, expected_duration)); +} + +// Make sure the signal from "GetOnDestroyedSignal()" fires when the sender is +// destroyed, which is needed for the DTMF sender. +TEST_F(RtpSenderReceiverTest, TestOnDestroyedSignal) { + CreateAudioRtpSender(); + EXPECT_FALSE(audio_sender_destroyed_signal_fired_); + audio_rtp_sender_ = nullptr; + EXPECT_TRUE(audio_sender_destroyed_signal_fired_); +} + } // namespace webrtc diff --git a/webrtc/pc/webrtcsession.cc b/webrtc/pc/webrtcsession.cc index cfcc4871c3..a02aa5796a 100644 --- a/webrtc/pc/webrtcsession.cc +++ b/webrtc/pc/webrtcsession.cc @@ -231,29 +231,6 @@ static bool VerifyIceUfragPwdPresent(const SessionDescription* desc) { return true; } -static bool GetAudioSsrcByTrackId(const SessionDescription* session_description, - const std::string& track_id, - uint32_t* ssrc) { - const cricket::ContentInfo* audio_info = - cricket::GetFirstAudioContent(session_description); - if (!audio_info) { - LOG(LS_ERROR) << "Audio not used in this call"; - return false; - } - - const cricket::MediaContentDescription* audio_content = - static_cast( - audio_info->description); - const cricket::StreamParams* stream = - cricket::GetStreamByIds(audio_content->streams(), "", track_id); - if (!stream) { - return false; - } - - *ssrc = stream->first_ssrc(); - return true; -} - static bool GetTrackIdBySsrc(const SessionDescription* session_description, uint32_t ssrc, std::string* track_id) { @@ -523,7 +500,6 @@ WebRtcSession::~WebRtcSession() { quic_data_transport_.reset(); } #endif - SignalDestroyed(); LOG(LS_INFO) << "Session: " << id() << " is destroyed."; } @@ -1248,49 +1224,6 @@ std::string WebRtcSession::BadStateErrMsg(State state) { return desc.str(); } -bool WebRtcSession::CanInsertDtmf(const std::string& track_id) { - RTC_DCHECK(signaling_thread()->IsCurrent()); - if (!voice_channel_) { - LOG(LS_ERROR) << "CanInsertDtmf: No audio channel exists."; - return false; - } - uint32_t send_ssrc = 0; - // The Dtmf is negotiated per channel not ssrc, so we only check if the ssrc - // exists. - if (!local_description() || - !GetAudioSsrcByTrackId(local_description()->description(), track_id, - &send_ssrc)) { - LOG(LS_ERROR) << "CanInsertDtmf: Track does not exist: " << track_id; - return false; - } - return voice_channel_->CanInsertDtmf(); -} - -bool WebRtcSession::InsertDtmf(const std::string& track_id, - int code, int duration) { - RTC_DCHECK(signaling_thread()->IsCurrent()); - if (!voice_channel_) { - LOG(LS_ERROR) << "InsertDtmf: No audio channel exists."; - return false; - } - uint32_t send_ssrc = 0; - if (!(local_description() && - GetAudioSsrcByTrackId(local_description()->description(), - track_id, &send_ssrc))) { - LOG(LS_ERROR) << "InsertDtmf: Track does not exist: " << track_id; - return false; - } - if (!voice_channel_->InsertDtmf(send_ssrc, code, duration)) { - LOG(LS_ERROR) << "Failed to insert DTMF to channel."; - return false; - } - return true; -} - -sigslot::signal0<>* WebRtcSession::GetOnDestroyedSignal() { - return &SignalDestroyed; -} - bool WebRtcSession::SendData(const cricket::SendDataParams& params, const rtc::CopyOnWriteBuffer& payload, cricket::SendDataResult* result) { diff --git a/webrtc/pc/webrtcsession.h b/webrtc/pc/webrtcsession.h index c250833307..f8e0830ad0 100644 --- a/webrtc/pc/webrtcsession.h +++ b/webrtc/pc/webrtcsession.h @@ -27,7 +27,6 @@ #include "webrtc/p2p/base/candidate.h" #include "webrtc/p2p/base/transportcontroller.h" #include "webrtc/pc/datachannel.h" -#include "webrtc/pc/dtmfsender.h" #include "webrtc/pc/mediacontroller.h" #include "webrtc/pc/mediasession.h" @@ -140,8 +139,6 @@ struct ChannelNamePairs { // packets are represented by TransportChannels. The application-level protocol // is represented by SessionDecription objects. class WebRtcSession : - - public DtmfProviderInterface, public DataChannelProviderInterface, public sigslot::has_slots<> { public: @@ -285,12 +282,6 @@ class WebRtcSession : virtual bool GetLocalTrackIdBySsrc(uint32_t ssrc, std::string* track_id); virtual bool GetRemoteTrackIdBySsrc(uint32_t ssrc, std::string* track_id); - // Implements DtmfProviderInterface. - bool CanInsertDtmf(const std::string& track_id) override; - bool InsertDtmf(const std::string& track_id, - int code, int duration) override; - sigslot::signal0<>* GetOnDestroyedSignal() override; - // Implements DataChannelProviderInterface. bool SendData(const cricket::SendDataParams& params, const rtc::CopyOnWriteBuffer& payload, @@ -361,8 +352,6 @@ class WebRtcSession : sigslot::signal0<> SignalVideoChannelDestroyed; sigslot::signal0<> SignalDataChannelCreated; sigslot::signal0<> SignalDataChannelDestroyed; - // Called when the whole session is destroyed. - sigslot::signal0<> SignalDestroyed; // Called when a valid data channel OPEN message is received. // std::string represents the data channel label. diff --git a/webrtc/pc/webrtcsession_unittest.cc b/webrtc/pc/webrtcsession_unittest.cc index efd5d0c870..716ba25146 100644 --- a/webrtc/pc/webrtcsession_unittest.cc +++ b/webrtc/pc/webrtcsession_unittest.cc @@ -434,8 +434,6 @@ class WebRtcSessionTest fake_sctp_transport_factory_))); session_->SignalDataChannelOpenMessage.connect( this, &WebRtcSessionTest::OnDataChannelOpenMessage); - session_->GetOnDestroyedSignal()->connect( - this, &WebRtcSessionTest::OnSessionDestroyed); configuration_.rtcp_mux_policy = rtcp_mux_policy; EXPECT_EQ(PeerConnectionInterface::kIceConnectionNew, @@ -454,8 +452,6 @@ class WebRtcSessionTest last_data_channel_config_ = config; } - void OnSessionDestroyed() { session_destroyed_ = true; } - void Init() { Init(nullptr, PeerConnectionInterface::kRtcpMuxPolicyNegotiate); } @@ -502,17 +498,6 @@ class WebRtcSessionTest PeerConnectionInterface::kRtcpMuxPolicyNegotiate); } - void InitWithDtmfCodec() { - // Add kTelephoneEventCodec for dtmf test. - const cricket::AudioCodec kTelephoneEventCodec(106, "telephone-event", 8000, - 0, 1); - std::vector codecs; - codecs.push_back(kTelephoneEventCodec); - media_engine_->SetAudioCodecs(codecs); - desc_factory_->set_audio_codecs(codecs, codecs); - Init(); - } - void InitWithGcm() { rtc::CryptoOptions crypto_options; crypto_options.enable_gcm_crypto_suites = true; @@ -1197,18 +1182,6 @@ class WebRtcSessionTest EXPECT_EQ(expected_candidate_num, observer_.mline_1_candidates_.size()); } } - // Tests that we can only send DTMF when the dtmf codec is supported. - void TestCanInsertDtmf(bool can) { - if (can) { - InitWithDtmfCodec(); - } else { - Init(); - } - SendAudioVideoStream1(); - CreateAndSetRemoteOfferAndLocalAnswer(); - EXPECT_FALSE(session_->CanInsertDtmf("")); - EXPECT_EQ(can, session_->CanInsertDtmf(kAudioTrack1)); - } bool ContainsVideoCodecWithName(const SessionDescriptionInterface* desc, const std::string& codec_name) { @@ -1567,7 +1540,6 @@ class WebRtcSessionTest // Last values received from data channel creation signal. std::string last_data_channel_label_; InternalDataChannelInit last_data_channel_config_; - bool session_destroyed_ = false; bool with_gcm_ = false; }; @@ -3507,39 +3479,6 @@ TEST_F(WebRtcSessionTest, SetSetupGcm) { CreateAndSetRemoteOfferAndLocalAnswer(); } -TEST_F(WebRtcSessionTest, CanNotInsertDtmf) { - TestCanInsertDtmf(false); -} - -TEST_F(WebRtcSessionTest, CanInsertDtmf) { - TestCanInsertDtmf(true); -} - -TEST_F(WebRtcSessionTest, InsertDtmf) { - // Setup - Init(); - SendAudioVideoStream1(); - CreateAndSetRemoteOfferAndLocalAnswer(); - FakeVoiceMediaChannel* channel = media_engine_->GetVoiceChannel(0); - EXPECT_EQ(0U, channel->dtmf_info_queue().size()); - - // Insert DTMF - const int expected_duration = 90; - session_->InsertDtmf(kAudioTrack1, 0, expected_duration); - session_->InsertDtmf(kAudioTrack1, 1, expected_duration); - session_->InsertDtmf(kAudioTrack1, 2, expected_duration); - - // Verify - 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)); - EXPECT_TRUE(CompareDtmfInfo(channel->dtmf_info_queue()[1], send_ssrc, 1, - expected_duration)); - EXPECT_TRUE(CompareDtmfInfo(channel->dtmf_info_queue()[2], send_ssrc, 2, - expected_duration)); -} - // This test verifies the |initial_offerer| flag when session initiates the // call. TEST_F(WebRtcSessionTest, TestInitiatorFlagAsOriginator) { @@ -4400,14 +4339,6 @@ TEST_F(WebRtcSessionTest, TestPacketOptionsAndOnPacketSent) { TestPacketOptions(); } -// Make sure the signal from "GetOnDestroyedSignal()" fires when the session -// is destroyed. -TEST_F(WebRtcSessionTest, TestOnDestroyedSignal) { - Init(); - session_.reset(); - EXPECT_TRUE(session_destroyed_); -} - // TODO(bemasc): Add a TestIceStatesBundle with BUNDLE enabled. That test // currently fails because upon disconnection and reconnection OnIceComplete is // called more than once without returning to IceGatheringGathering.