diff --git a/ortc/ortcrtpsenderadapter.cc b/ortc/ortcrtpsenderadapter.cc index 306db64a57..2b25d46eeb 100644 --- a/ortc/ortcrtpsenderadapter.cc +++ b/ortc/ortcrtpsenderadapter.cc @@ -156,14 +156,20 @@ OrtcRtpSenderAdapter::OrtcRtpSenderAdapter( void OrtcRtpSenderAdapter::CreateInternalSender() { switch (kind_) { case cricket::MEDIA_TYPE_AUDIO: { - auto* audio_sender = new AudioRtpSender(nullptr); - audio_sender->SetChannel(rtp_transport_controller_->voice_channel()); + auto* audio_sender = new AudioRtpSender( + rtp_transport_controller_->worker_thread(), nullptr); + auto* voice_channel = rtp_transport_controller_->voice_channel(); + RTC_DCHECK(voice_channel); + audio_sender->SetMediaChannel(voice_channel->media_channel()); internal_sender_ = audio_sender; break; } case cricket::MEDIA_TYPE_VIDEO: { - auto* video_sender = new VideoRtpSender(); - video_sender->SetChannel(rtp_transport_controller_->video_channel()); + auto* video_sender = + new VideoRtpSender(rtp_transport_controller_->worker_thread()); + auto* video_channel = rtp_transport_controller_->video_channel(); + RTC_DCHECK(video_channel); + video_sender->SetMediaChannel(video_channel->media_channel()); internal_sender_ = video_sender; break; } diff --git a/pc/channel.cc b/pc/channel.cc index 5c91c0b961..e1c103b39b 100644 --- a/pc/channel.cc +++ b/pc/channel.cc @@ -1212,42 +1212,6 @@ void VoiceChannel::SetEarlyMedia(bool enable) { } } -bool VoiceChannel::CanInsertDtmf() { - return InvokeOnWorker( - RTC_FROM_HERE, Bind(&VoiceMediaChannel::CanInsertDtmf, media_channel())); -} - -bool VoiceChannel::InsertDtmf(uint32_t ssrc, - int event_code, - int duration) { - return InvokeOnWorker( - RTC_FROM_HERE, - Bind(&VoiceChannel::InsertDtmf_w, this, ssrc, event_code, duration)); -} - -webrtc::RtpParameters VoiceChannel::GetRtpSendParameters(uint32_t ssrc) const { - return worker_thread()->Invoke( - RTC_FROM_HERE, Bind(&VoiceChannel::GetRtpSendParameters_w, this, ssrc)); -} - -webrtc::RtpParameters VoiceChannel::GetRtpSendParameters_w( - uint32_t ssrc) const { - return media_channel()->GetRtpSendParameters(ssrc); -} - -bool VoiceChannel::SetRtpSendParameters( - uint32_t ssrc, - const webrtc::RtpParameters& parameters) { - return InvokeOnWorker( - RTC_FROM_HERE, - Bind(&VoiceChannel::SetRtpSendParameters_w, this, ssrc, parameters)); -} - -bool VoiceChannel::SetRtpSendParameters_w(uint32_t ssrc, - webrtc::RtpParameters parameters) { - return media_channel()->SetRtpSendParameters(ssrc, parameters); -} - bool VoiceChannel::GetStats(VoiceMediaInfo* stats) { return InvokeOnWorker(RTC_FROM_HERE, Bind(&VoiceMediaChannel::GetStats, media_channel(), stats)); @@ -1441,15 +1405,6 @@ void VoiceChannel::HandleEarlyMediaTimeout() { } } -bool VoiceChannel::InsertDtmf_w(uint32_t ssrc, - int event, - int duration) { - if (!enabled()) { - return false; - } - return media_channel()->InsertDtmf(ssrc, event, duration); -} - void VoiceChannel::OnMessage(rtc::Message *pmsg) { switch (pmsg->message_id) { case MSG_EARLYMEDIATIMEOUT: @@ -1501,39 +1456,6 @@ VideoChannel::~VideoChannel() { Deinit(); } -bool VideoChannel::SetVideoSend( - uint32_t ssrc, - bool mute, - const VideoOptions* options, - rtc::VideoSourceInterface* source) { - return InvokeOnWorker( - RTC_FROM_HERE, Bind(&VideoMediaChannel::SetVideoSend, media_channel(), - ssrc, mute, options, source)); -} - -webrtc::RtpParameters VideoChannel::GetRtpSendParameters(uint32_t ssrc) const { - return worker_thread()->Invoke( - RTC_FROM_HERE, Bind(&VideoChannel::GetRtpSendParameters_w, this, ssrc)); -} - -webrtc::RtpParameters VideoChannel::GetRtpSendParameters_w( - uint32_t ssrc) const { - return media_channel()->GetRtpSendParameters(ssrc); -} - -bool VideoChannel::SetRtpSendParameters( - uint32_t ssrc, - const webrtc::RtpParameters& parameters) { - return InvokeOnWorker( - RTC_FROM_HERE, - Bind(&VideoChannel::SetRtpSendParameters_w, this, ssrc, parameters)); -} - -bool VideoChannel::SetRtpSendParameters_w(uint32_t ssrc, - webrtc::RtpParameters parameters) { - return media_channel()->SetRtpSendParameters(ssrc, parameters); -} - void VideoChannel::UpdateMediaSendRecvState_w() { // Send outgoing data if we're the active call, we have the remote content, // and we have had some form of connectivity. diff --git a/pc/channel.h b/pc/channel.h index ec9f9138ee..ca36dcdfe1 100644 --- a/pc/channel.h +++ b/pc/channel.h @@ -482,18 +482,6 @@ class VoiceChannel : public BaseChannel { // own ringing sound sigslot::signal1 SignalEarlyMediaTimeout; - // Returns if the telephone-event has been negotiated. - bool CanInsertDtmf(); - // Send and/or play a DTMF |event| according to the |flags|. - // The DTMF out-of-band signal will be used on sending. - // 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); - webrtc::RtpParameters GetRtpSendParameters(uint32_t ssrc) const; - bool SetRtpSendParameters(uint32_t ssrc, - const webrtc::RtpParameters& parameters); - // Get statistics about the current media session. bool GetStats(VoiceMediaInfo* stats); @@ -530,7 +518,6 @@ class VoiceChannel : public BaseChannel { webrtc::SdpType type, std::string* error_desc) override; void HandleEarlyMediaTimeout(); - bool InsertDtmf_w(uint32_t ssrc, int event, int duration); void OnMessage(rtc::Message* pmsg) override; void OnConnectionMonitorUpdate( @@ -582,15 +569,6 @@ class VideoChannel : public BaseChannel { void StopMediaMonitor(); sigslot::signal2 SignalMediaMonitor; - // Register a source and set options. - // The |ssrc| must correspond to a registered send stream. - bool SetVideoSend(uint32_t ssrc, - bool enable, - const VideoOptions* options, - rtc::VideoSourceInterface* source); - webrtc::RtpParameters GetRtpSendParameters(uint32_t ssrc) const; - bool SetRtpSendParameters(uint32_t ssrc, - const webrtc::RtpParameters& parameters); cricket::MediaType media_type() override { return cricket::MEDIA_TYPE_VIDEO; } private: @@ -603,8 +581,6 @@ class VideoChannel : public BaseChannel { webrtc::SdpType type, std::string* error_desc) override; bool GetStats_w(VideoMediaInfo* stats); - webrtc::RtpParameters GetRtpSendParameters_w(uint32_t ssrc) const; - bool SetRtpSendParameters_w(uint32_t ssrc, webrtc::RtpParameters parameters); void OnConnectionMonitorUpdate( ConnectionMonitor* monitor, diff --git a/pc/channel_unittest.cc b/pc/channel_unittest.cc index ae5782c915..7e3b45993a 100644 --- a/pc/channel_unittest.cc +++ b/pc/channel_unittest.cc @@ -1885,25 +1885,6 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { rtc::nullopt); } - void CanChangeMaxBitrate() { - CreateChannels(0, 0); - EXPECT_TRUE(channel1_->SetLocalContent(&local_media_content1_, - SdpType::kOffer, NULL)); - - EXPECT_TRUE(channel1_->SetRtpSendParameters( - kSsrc1, BitrateLimitedParameters(1000))); - VerifyMaxBitrate(channel1_->GetRtpSendParameters(kSsrc1), 1000); - VerifyMaxBitrate(media_channel1_->GetRtpSendParameters(kSsrc1), 1000); - EXPECT_EQ(-1, media_channel1_->max_bps()); - - EXPECT_TRUE(channel1_->SetRtpSendParameters( - kSsrc1, BitrateLimitedParameters(rtc::nullopt))); - VerifyMaxBitrate(channel1_->GetRtpSendParameters(kSsrc1), rtc::nullopt); - VerifyMaxBitrate(media_channel1_->GetRtpSendParameters(kSsrc1), - rtc::nullopt); - EXPECT_EQ(-1, media_channel1_->max_bps()); - } - // Test that when a channel gets new transports with a call to // |SetTransports|, the socket options from the old transports are merged with // the options on the new transport. @@ -2347,26 +2328,6 @@ TEST_F(VoiceChannelSingleThreadTest, TestMediaMonitor) { Base::TestMediaMonitor(); } -// Test that InsertDtmf properly forwards to the media channel. -TEST_F(VoiceChannelSingleThreadTest, TestInsertDtmf) { - CreateChannels(0, 0); - EXPECT_TRUE(SendInitiate()); - EXPECT_TRUE(SendAccept()); - EXPECT_EQ(0U, media_channel1_->dtmf_info_queue().size()); - - 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)); - EXPECT_TRUE(CompareDtmfInfo(media_channel1_->dtmf_info_queue()[1], - 2, 5, 110)); - EXPECT_TRUE(CompareDtmfInfo(media_channel1_->dtmf_info_queue()[2], - 3, 7, 120)); -} - TEST_F(VoiceChannelSingleThreadTest, TestSetContentFailure) { Base::TestSetContentFailure(); } @@ -2419,10 +2380,6 @@ TEST_F(VoiceChannelSingleThreadTest, DefaultMaxBitrateIsUnlimited) { Base::DefaultMaxBitrateIsUnlimited(); } -TEST_F(VoiceChannelSingleThreadTest, CanChangeMaxBitrate) { - Base::CanChangeMaxBitrate(); -} - TEST_F(VoiceChannelSingleThreadTest, SocketOptionsMergedOnSetTransport) { Base::SocketOptionsMergedOnSetTransport(); } @@ -2636,26 +2593,6 @@ TEST_F(VoiceChannelDoubleThreadTest, TestMediaMonitor) { Base::TestMediaMonitor(); } -// Test that InsertDtmf properly forwards to the media channel. -TEST_F(VoiceChannelDoubleThreadTest, TestInsertDtmf) { - CreateChannels(0, 0); - EXPECT_TRUE(SendInitiate()); - EXPECT_TRUE(SendAccept()); - EXPECT_EQ(0U, media_channel1_->dtmf_info_queue().size()); - - 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)); - EXPECT_TRUE( - CompareDtmfInfo(media_channel1_->dtmf_info_queue()[1], 2, 5, 110)); - EXPECT_TRUE( - CompareDtmfInfo(media_channel1_->dtmf_info_queue()[2], 3, 7, 120)); -} - TEST_F(VoiceChannelDoubleThreadTest, TestSetContentFailure) { Base::TestSetContentFailure(); } @@ -2708,10 +2645,6 @@ TEST_F(VoiceChannelDoubleThreadTest, DefaultMaxBitrateIsUnlimited) { Base::DefaultMaxBitrateIsUnlimited(); } -TEST_F(VoiceChannelDoubleThreadTest, CanChangeMaxBitrate) { - Base::CanChangeMaxBitrate(); -} - TEST_F(VoiceChannelDoubleThreadTest, SocketOptionsMergedOnSetTransport) { Base::SocketOptionsMergedOnSetTransport(); } @@ -2753,26 +2686,6 @@ TEST_F(VideoChannelSingleThreadTest, TestPlayoutAndSendingStates) { Base::TestPlayoutAndSendingStates(); } -TEST_F(VideoChannelSingleThreadTest, TestMuteStream) { - CreateChannels(0, 0); - // Test that we can Mute the default channel even though the sending SSRC - // is unknown. - EXPECT_FALSE(media_channel1_->IsStreamMuted(0)); - EXPECT_TRUE(channel1_->SetVideoSend(0, false, nullptr, nullptr)); - EXPECT_TRUE(media_channel1_->IsStreamMuted(0)); - EXPECT_TRUE(channel1_->SetVideoSend(0, true, nullptr, nullptr)); - EXPECT_FALSE(media_channel1_->IsStreamMuted(0)); - // Test that we can not mute an unknown SSRC. - EXPECT_FALSE(channel1_->SetVideoSend(kSsrc1, false, nullptr, nullptr)); - SendInitiate(); - // After the local session description has been set, we can mute a stream - // with its SSRC. - EXPECT_TRUE(channel1_->SetVideoSend(kSsrc1, false, nullptr, nullptr)); - EXPECT_TRUE(media_channel1_->IsStreamMuted(kSsrc1)); - EXPECT_TRUE(channel1_->SetVideoSend(kSsrc1, true, nullptr, nullptr)); - EXPECT_FALSE(media_channel1_->IsStreamMuted(kSsrc1)); -} - TEST_F(VideoChannelSingleThreadTest, TestMediaContentDirection) { Base::TestMediaContentDirection(); } @@ -2931,10 +2844,6 @@ TEST_F(VideoChannelSingleThreadTest, DefaultMaxBitrateIsUnlimited) { Base::DefaultMaxBitrateIsUnlimited(); } -TEST_F(VideoChannelSingleThreadTest, CanChangeMaxBitrate) { - Base::CanChangeMaxBitrate(); -} - TEST_F(VideoChannelSingleThreadTest, SocketOptionsMergedOnSetTransport) { Base::SocketOptionsMergedOnSetTransport(); } @@ -2976,26 +2885,6 @@ TEST_F(VideoChannelDoubleThreadTest, TestPlayoutAndSendingStates) { Base::TestPlayoutAndSendingStates(); } -TEST_F(VideoChannelDoubleThreadTest, TestMuteStream) { - CreateChannels(0, 0); - // Test that we can Mute the default channel even though the sending SSRC - // is unknown. - EXPECT_FALSE(media_channel1_->IsStreamMuted(0)); - EXPECT_TRUE(channel1_->SetVideoSend(0, false, nullptr, nullptr)); - EXPECT_TRUE(media_channel1_->IsStreamMuted(0)); - EXPECT_TRUE(channel1_->SetVideoSend(0, true, nullptr, nullptr)); - EXPECT_FALSE(media_channel1_->IsStreamMuted(0)); - // Test that we can not mute an unknown SSRC. - EXPECT_FALSE(channel1_->SetVideoSend(kSsrc1, false, nullptr, nullptr)); - SendInitiate(); - // After the local session description has been set, we can mute a stream - // with its SSRC. - EXPECT_TRUE(channel1_->SetVideoSend(kSsrc1, false, nullptr, nullptr)); - EXPECT_TRUE(media_channel1_->IsStreamMuted(kSsrc1)); - EXPECT_TRUE(channel1_->SetVideoSend(kSsrc1, true, nullptr, nullptr)); - EXPECT_FALSE(media_channel1_->IsStreamMuted(kSsrc1)); -} - TEST_F(VideoChannelDoubleThreadTest, TestMediaContentDirection) { Base::TestMediaContentDirection(); } @@ -3154,10 +3043,6 @@ TEST_F(VideoChannelDoubleThreadTest, DefaultMaxBitrateIsUnlimited) { Base::DefaultMaxBitrateIsUnlimited(); } -TEST_F(VideoChannelDoubleThreadTest, CanChangeMaxBitrate) { - Base::CanChangeMaxBitrate(); -} - TEST_F(VideoChannelDoubleThreadTest, SocketOptionsMergedOnSetTransport) { Base::SocketOptionsMergedOnSetTransport(); } diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc index 0888acced2..6281d57e3d 100644 --- a/pc/peerconnection.cc +++ b/pc/peerconnection.cc @@ -1073,7 +1073,7 @@ PeerConnection::AddTrackPlanB( auto new_sender = CreateSender(media_type, track, stream_labels); if (track->kind() == MediaStreamTrackInterface::kAudioKind) { static_cast(new_sender->internal()) - ->SetChannel(voice_channel()); + ->SetMediaChannel(voice_media_channel()); GetAudioTransceiver()->internal()->AddSender(new_sender); const RtpSenderInfo* sender_info = FindSenderInfo(local_audio_sender_infos_, @@ -1084,7 +1084,7 @@ PeerConnection::AddTrackPlanB( } else { RTC_DCHECK_EQ(MediaStreamTrackInterface::kVideoKind, track->kind()); static_cast(new_sender->internal()) - ->SetChannel(video_channel()); + ->SetMediaChannel(video_media_channel()); GetVideoTransceiver()->internal()->AddSender(new_sender); const RtpSenderInfo* sender_info = FindSenderInfo(local_video_sender_infos_, @@ -1289,7 +1289,8 @@ PeerConnection::CreateSender( (track->kind() == MediaStreamTrackInterface::kAudioKind)); sender = RtpSenderProxyWithInternal::Create( signaling_thread(), - new AudioRtpSender(static_cast(track.get()), + new AudioRtpSender(worker_thread(), + static_cast(track.get()), stream_labels, stats_.get())); } else { RTC_DCHECK_EQ(media_type, cricket::MEDIA_TYPE_VIDEO); @@ -1297,7 +1298,8 @@ PeerConnection::CreateSender( (track->kind() == MediaStreamTrackInterface::kVideoKind)); sender = RtpSenderProxyWithInternal::Create( signaling_thread(), - new VideoRtpSender(static_cast(track.get()), + new VideoRtpSender(worker_thread(), + static_cast(track.get()), stream_labels)); } sender->internal()->set_stream_ids(stream_labels); @@ -1368,15 +1370,16 @@ rtc::scoped_refptr PeerConnection::CreateSender( // TODO(steveanton): Move construction of the RtpSenders to RtpTransceiver. rtc::scoped_refptr> new_sender; if (kind == MediaStreamTrackInterface::kAudioKind) { - auto* audio_sender = - new AudioRtpSender(nullptr, stream_labels, stats_.get()); - audio_sender->SetChannel(voice_channel()); + auto* audio_sender = new AudioRtpSender(worker_thread(), nullptr, + stream_labels, stats_.get()); + audio_sender->SetMediaChannel(voice_media_channel()); new_sender = RtpSenderProxyWithInternal::Create( signaling_thread(), audio_sender); GetAudioTransceiver()->internal()->AddSender(new_sender); } else if (kind == MediaStreamTrackInterface::kVideoKind) { - auto* video_sender = new VideoRtpSender(nullptr, stream_labels); - video_sender->SetChannel(video_channel()); + auto* video_sender = + new VideoRtpSender(worker_thread(), nullptr, stream_labels); + video_sender->SetMediaChannel(video_media_channel()); new_sender = RtpSenderProxyWithInternal::Create( signaling_thread(), video_sender); GetVideoTransceiver()->internal()->AddSender(new_sender); @@ -2828,7 +2831,7 @@ void PeerConnection::AddAudioTrack(AudioTrackInterface* track, auto new_sender = CreateSender(cricket::MEDIA_TYPE_AUDIO, track, {stream->label()}); static_cast(new_sender->internal()) - ->SetChannel(voice_channel()); + ->SetMediaChannel(voice_media_channel()); GetAudioTransceiver()->internal()->AddSender(new_sender); // If the sender has already been configured in SDP, we call SetSsrc, // which will connect the sender to the underlying transport. This can @@ -2872,7 +2875,7 @@ void PeerConnection::AddVideoTrack(VideoTrackInterface* track, auto new_sender = CreateSender(cricket::MEDIA_TYPE_VIDEO, track, {stream->label()}); static_cast(new_sender->internal()) - ->SetChannel(video_channel()); + ->SetMediaChannel(video_media_channel()); GetVideoTransceiver()->internal()->AddSender(new_sender); const RtpSenderInfo* sender_info = FindSenderInfo(local_video_sender_infos_, stream->label(), track->id()); diff --git a/pc/rtpsender.cc b/pc/rtpsender.cc index cad86f2f77..7c85764279 100644 --- a/pc/rtpsender.cc +++ b/pc/rtpsender.cc @@ -57,13 +57,17 @@ void LocalAudioSinkAdapter::SetSink(cricket::AudioSource::Sink* sink) { sink_ = sink; } -AudioRtpSender::AudioRtpSender(StatsCollector* stats) - : AudioRtpSender(nullptr, {rtc::CreateRandomUuid()}, stats) {} +AudioRtpSender::AudioRtpSender(rtc::Thread* worker_thread, + StatsCollector* stats) + : AudioRtpSender(worker_thread, nullptr, {rtc::CreateRandomUuid()}, stats) { +} -AudioRtpSender::AudioRtpSender(rtc::scoped_refptr track, +AudioRtpSender::AudioRtpSender(rtc::Thread* worker_thread, + rtc::scoped_refptr track, const std::vector& stream_labels, StatsCollector* stats) - : id_(track ? track->id() : rtc::CreateRandomUuid()), + : worker_thread_(worker_thread), + id_(track ? track->id() : rtc::CreateRandomUuid()), stream_ids_(stream_labels), stats_(stats), track_(track), @@ -73,6 +77,7 @@ AudioRtpSender::AudioRtpSender(rtc::scoped_refptr track, cached_track_enabled_(track ? track->enabled() : false), sink_adapter_(new LocalAudioSinkAdapter()), attachment_id_(track ? GenerateUniqueId() : 0) { + RTC_DCHECK(worker_thread); // TODO(bugs.webrtc.org/7932): Remove once zero or multiple streams are // supported. RTC_DCHECK_EQ(stream_labels.size(), 1u); @@ -89,7 +94,7 @@ AudioRtpSender::~AudioRtpSender() { } bool AudioRtpSender::CanInsertDtmf() { - if (!channel_) { + if (!media_channel_) { RTC_LOG(LS_ERROR) << "CanInsertDtmf: No audio channel exists."; return false; } @@ -99,11 +104,12 @@ bool AudioRtpSender::CanInsertDtmf() { RTC_LOG(LS_ERROR) << "CanInsertDtmf: Sender does not have SSRC."; return false; } - return channel_->CanInsertDtmf(); + return worker_thread_->Invoke( + RTC_FROM_HERE, [&] { return media_channel_->CanInsertDtmf(); }); } bool AudioRtpSender::InsertDtmf(int code, int duration) { - if (!channel_) { + if (!media_channel_) { RTC_LOG(LS_ERROR) << "CanInsertDtmf: No audio channel exists."; return false; } @@ -111,11 +117,13 @@ bool AudioRtpSender::InsertDtmf(int code, int duration) { RTC_LOG(LS_ERROR) << "CanInsertDtmf: Sender does not have SSRC."; return false; } - if (!channel_->InsertDtmf(ssrc_, code, duration)) { + bool success = worker_thread_->Invoke(RTC_FROM_HERE, [&] { + return media_channel_->InsertDtmf(ssrc_, code, duration); + }); + if (!success) { RTC_LOG(LS_ERROR) << "Failed to insert DTMF to channel."; - return false; } - return true; + return success; } sigslot::signal0<>* AudioRtpSender::GetOnDestroyedSignal() { @@ -182,18 +190,22 @@ bool AudioRtpSender::SetTrack(MediaStreamTrackInterface* track) { } RtpParameters AudioRtpSender::GetParameters() const { - if (!channel_ || stopped_) { + if (!media_channel_ || stopped_) { return RtpParameters(); } - return channel_->GetRtpSendParameters(ssrc_); + return worker_thread_->Invoke(RTC_FROM_HERE, [&] { + return media_channel_->GetRtpSendParameters(ssrc_); + }); } bool AudioRtpSender::SetParameters(const RtpParameters& parameters) { TRACE_EVENT0("webrtc", "AudioRtpSender::SetParameters"); - if (!channel_ || stopped_) { + if (!media_channel_ || stopped_) { return false; } - return channel_->SetRtpSendParameters(ssrc_, parameters); + return worker_thread_->Invoke(RTC_FROM_HERE, [&] { + return media_channel_->SetRtpSendParameters(ssrc_, parameters); + }); } rtc::scoped_refptr AudioRtpSender::GetDtmfSender() const { @@ -243,7 +255,7 @@ void AudioRtpSender::Stop() { void AudioRtpSender::SetAudioSend() { RTC_DCHECK(!stopped_); RTC_DCHECK(can_send_track()); - if (!channel_) { + if (!media_channel_) { RTC_LOG(LS_ERROR) << "SetAudioSend: No audio channel exists."; return; } @@ -260,9 +272,14 @@ void AudioRtpSender::SetAudioSend() { } #endif - cricket::AudioSource* source = sink_adapter_.get(); - RTC_DCHECK(source != nullptr); - if (!channel_->SetAudioSend(ssrc_, track_->enabled(), &options, source)) { + // |track_->enabled()| hops to the signaling thread, so call it before we hop + // to the worker thread or else it will deadlock. + bool track_enabled = track_->enabled(); + bool success = worker_thread_->Invoke(RTC_FROM_HERE, [&] { + return media_channel_->SetAudioSend(ssrc_, track_enabled, &options, + sink_adapter_.get()); + }); + if (!success) { RTC_LOG(LS_ERROR) << "SetAudioSend: ssrc is incorrect: " << ssrc_; } } @@ -270,29 +287,35 @@ void AudioRtpSender::SetAudioSend() { void AudioRtpSender::ClearAudioSend() { RTC_DCHECK(ssrc_ != 0); RTC_DCHECK(!stopped_); - if (!channel_) { + if (!media_channel_) { RTC_LOG(LS_WARNING) << "ClearAudioSend: No audio channel exists."; return; } cricket::AudioOptions options; - if (!channel_->SetAudioSend(ssrc_, false, &options, nullptr)) { + bool success = worker_thread_->Invoke(RTC_FROM_HERE, [&] { + return media_channel_->SetAudioSend(ssrc_, false, &options, nullptr); + }); + if (!success) { RTC_LOG(LS_WARNING) << "ClearAudioSend: ssrc is incorrect: " << ssrc_; } } -VideoRtpSender::VideoRtpSender() - : VideoRtpSender(nullptr, {rtc::CreateRandomUuid()}) {} +VideoRtpSender::VideoRtpSender(rtc::Thread* worker_thread) + : VideoRtpSender(worker_thread, nullptr, {rtc::CreateRandomUuid()}) {} -VideoRtpSender::VideoRtpSender(rtc::scoped_refptr track, +VideoRtpSender::VideoRtpSender(rtc::Thread* worker_thread, + rtc::scoped_refptr track, const std::vector& stream_labels) - : id_(track ? track->id() : rtc::CreateRandomUuid()), + : worker_thread_(worker_thread), + id_(track ? track->id() : rtc::CreateRandomUuid()), stream_ids_(stream_labels), track_(track), cached_track_enabled_(track ? track->enabled() : false), - cached_track_content_hint_(track - ? track->content_hint() - : VideoTrackInterface::ContentHint::kNone), + cached_track_content_hint_( + track ? track->content_hint() + : VideoTrackInterface::ContentHint::kNone), attachment_id_(track ? GenerateUniqueId() : 0) { + RTC_DCHECK(worker_thread); // TODO(bugs.webrtc.org/7932): Remove once zero or multiple streams are // supported. RTC_DCHECK_EQ(stream_labels.size(), 1u); @@ -359,18 +382,22 @@ bool VideoRtpSender::SetTrack(MediaStreamTrackInterface* track) { } RtpParameters VideoRtpSender::GetParameters() const { - if (!channel_ || stopped_) { + if (!media_channel_ || stopped_) { return RtpParameters(); } - return channel_->GetRtpSendParameters(ssrc_); + return worker_thread_->Invoke(RTC_FROM_HERE, [&] { + return media_channel_->GetRtpSendParameters(ssrc_); + }); } bool VideoRtpSender::SetParameters(const RtpParameters& parameters) { TRACE_EVENT0("webrtc", "VideoRtpSender::SetParameters"); - if (!channel_ || stopped_) { + if (!media_channel_ || stopped_) { return false; } - return channel_->SetRtpSendParameters(ssrc_, parameters); + return worker_thread_->Invoke(RTC_FROM_HERE, [&] { + return media_channel_->SetRtpSendParameters(ssrc_, parameters); + }); } rtc::scoped_refptr VideoRtpSender::GetDtmfSender() const { @@ -411,7 +438,7 @@ void VideoRtpSender::Stop() { void VideoRtpSender::SetVideoSend() { RTC_DCHECK(!stopped_); RTC_DCHECK(can_send_track()); - if (!channel_) { + if (!media_channel_) { RTC_LOG(LS_ERROR) << "SetVideoSend: No video channel exists."; return; } @@ -431,22 +458,28 @@ void VideoRtpSender::SetVideoSend() { options.is_screencast = true; break; } - if (!channel_->SetVideoSend(ssrc_, track_->enabled(), &options, track_)) { - RTC_NOTREACHED(); - } + // |track_->enabled()| hops to the signaling thread, so call it before we hop + // to the worker thread or else it will deadlock. + bool track_enabled = track_->enabled(); + bool success = worker_thread_->Invoke(RTC_FROM_HERE, [&] { + return media_channel_->SetVideoSend(ssrc_, track_enabled, &options, track_); + }); + RTC_DCHECK(success); } void VideoRtpSender::ClearVideoSend() { RTC_DCHECK(ssrc_ != 0); RTC_DCHECK(!stopped_); - if (!channel_) { + if (!media_channel_) { RTC_LOG(LS_WARNING) << "SetVideoSend: No video channel exists."; return; } // Allow SetVideoSend to fail since |enable| is false and |source| is null. // This the normal case when the underlying media channel has already been // deleted. - channel_->SetVideoSend(ssrc_, false, nullptr, nullptr); + worker_thread_->Invoke(RTC_FROM_HERE, [&] { + return media_channel_->SetVideoSend(ssrc_, false, nullptr, nullptr); + }); } } // namespace webrtc diff --git a/pc/rtpsender.h b/pc/rtpsender.h index eb09d0b13f..119d91f11a 100644 --- a/pc/rtpsender.h +++ b/pc/rtpsender.h @@ -26,7 +26,6 @@ // Adding 'nogncheck' to disable the gn include headers check to support modular // WebRTC build targets. #include "media/base/audiosource.h" // nogncheck -#include "pc/channel.h" #include "pc/dtmfsender.h" #include "pc/statscollector.h" @@ -85,11 +84,12 @@ class AudioRtpSender : public DtmfProviderInterface, // Construct an AudioRtpSender with a null track, a single, randomly generated // stream label, and a randomly generated ID. - explicit AudioRtpSender(StatsCollector* stats); + AudioRtpSender(rtc::Thread* worker_thread, StatsCollector* stats); // Construct an AudioRtpSender with the given track and stream labels. The // sender ID will be set to the track's ID. - AudioRtpSender(rtc::scoped_refptr track, + AudioRtpSender(rtc::Thread* worker_thread, + rtc::scoped_refptr track, const std::vector& stream_labels, StatsCollector* stats); @@ -140,8 +140,10 @@ class AudioRtpSender : public DtmfProviderInterface, int AttachmentId() const override { return attachment_id_; } // Does not take ownership. - // Should call SetChannel(nullptr) before |channel| is destroyed. - void SetChannel(cricket::VoiceChannel* channel) { channel_ = channel; } + // Should call SetMediaChannel(nullptr) before |media_channel| is destroyed. + void SetMediaChannel(cricket::VoiceMediaChannel* media_channel) { + media_channel_ = media_channel; + } private: // TODO(nisse): Since SSRC == 0 is technically valid, figure out @@ -155,11 +157,12 @@ class AudioRtpSender : public DtmfProviderInterface, sigslot::signal0<> SignalDestroyed; + rtc::Thread* const worker_thread_; const std::string id_; // TODO(steveanton): Until more Unified Plan work is done, this can only have // exactly one element. std::vector stream_ids_; - cricket::VoiceChannel* channel_ = nullptr; + cricket::VoiceMediaChannel* media_channel_ = nullptr; StatsCollector* stats_; rtc::scoped_refptr track_; rtc::scoped_refptr dtmf_sender_proxy_; @@ -178,11 +181,12 @@ class VideoRtpSender : public ObserverInterface, public: // Construct a VideoRtpSender with a null track, a single, randomly generated // stream label, and a randomly generated ID. - VideoRtpSender(); + explicit VideoRtpSender(rtc::Thread* worker_thread); // Construct a VideoRtpSender with the given track and stream labels. The // sender ID will be set to the track's ID. - VideoRtpSender(rtc::scoped_refptr track, + VideoRtpSender(rtc::Thread* worker_thread, + rtc::scoped_refptr track, const std::vector& stream_labels); virtual ~VideoRtpSender(); @@ -226,8 +230,10 @@ class VideoRtpSender : public ObserverInterface, int AttachmentId() const override { return attachment_id_; } // Does not take ownership. - // Should call SetChannel(nullptr) before |channel| is destroyed. - void SetChannel(cricket::VideoChannel* channel) { channel_ = channel; } + // Should call SetMediaChannel(nullptr) before |media_channel| is destroyed. + void SetMediaChannel(cricket::VideoMediaChannel* media_channel) { + media_channel_ = media_channel; + } private: bool can_send_track() const { return track_ && ssrc_; } @@ -237,11 +243,12 @@ class VideoRtpSender : public ObserverInterface, // Helper function to call SetVideoSend with "stop sending" parameters. void ClearVideoSend(); + rtc::Thread* worker_thread_; const std::string id_; // TODO(steveanton): Until more Unified Plan work is done, this can only have // exactly one element. std::vector stream_ids_; - cricket::VideoChannel* channel_ = nullptr; + cricket::VideoMediaChannel* media_channel_ = nullptr; rtc::scoped_refptr track_; uint32_t ssrc_ = 0; bool cached_track_enabled_ = false; diff --git a/pc/rtpsenderreceiver_unittest.cc b/pc/rtpsenderreceiver_unittest.cc index 9ccbe95026..9da9c6dc01 100644 --- a/pc/rtpsenderreceiver_unittest.cc +++ b/pc/rtpsenderreceiver_unittest.cc @@ -55,13 +55,15 @@ class RtpSenderReceiverTest : public testing::Test, public sigslot::has_slots<> { public: RtpSenderReceiverTest() - : // Create fake media engine/etc. so we can create channels to use to - // test RtpSenders/RtpReceivers. + : network_thread_(rtc::Thread::Current()), + worker_thread_(rtc::Thread::Current()), + // Create fake media engine/etc. so we can create channels to use to + // test RtpSenders/RtpReceivers. media_engine_(new cricket::FakeMediaEngine()), channel_manager_(rtc::WrapUnique(media_engine_), rtc::MakeUnique(), - rtc::Thread::Current(), - rtc::Thread::Current()), + worker_thread_, + network_thread_), fake_call_(Call::Config(&event_log_)), local_stream_(MediaStream::Create(kStreamLabel1)) { // Create channels to be used by the RtpSenders and RtpReceivers. @@ -134,9 +136,10 @@ class RtpSenderReceiverTest : public testing::Test, const rtc::scoped_refptr& source) { audio_track_ = AudioTrack::Create(kAudioTrackId, source); EXPECT_TRUE(local_stream_->AddTrack(audio_track_)); - audio_rtp_sender_ = new AudioRtpSender(local_stream_->GetAudioTracks()[0], - {local_stream_->label()}, nullptr); - audio_rtp_sender_->SetChannel(voice_channel_); + audio_rtp_sender_ = + new AudioRtpSender(worker_thread_, local_stream_->GetAudioTracks()[0], + {local_stream_->label()}, nullptr); + audio_rtp_sender_->SetMediaChannel(voice_media_channel_); audio_rtp_sender_->SetSsrc(kAudioSsrc); audio_rtp_sender_->GetOnDestroyedSignal()->connect( this, &RtpSenderReceiverTest::OnAudioSenderDestroyed); @@ -144,8 +147,8 @@ class RtpSenderReceiverTest : public testing::Test, } void CreateAudioRtpSenderWithNoTrack() { - audio_rtp_sender_ = new AudioRtpSender(nullptr); - audio_rtp_sender_->SetChannel(voice_channel_); + audio_rtp_sender_ = new AudioRtpSender(worker_thread_, nullptr); + audio_rtp_sender_->SetMediaChannel(voice_media_channel_); } void OnAudioSenderDestroyed() { audio_sender_destroyed_signal_fired_ = true; } @@ -154,16 +157,17 @@ class RtpSenderReceiverTest : public testing::Test, void CreateVideoRtpSender(bool is_screencast) { AddVideoTrack(is_screencast); - video_rtp_sender_ = new VideoRtpSender(local_stream_->GetVideoTracks()[0], - {local_stream_->label()}); - video_rtp_sender_->SetChannel(video_channel_); + video_rtp_sender_ = + new VideoRtpSender(worker_thread_, local_stream_->GetVideoTracks()[0], + {local_stream_->label()}); + video_rtp_sender_->SetMediaChannel(video_media_channel_); video_rtp_sender_->SetSsrc(kVideoSsrc); VerifyVideoChannelInput(); } void CreateVideoRtpSenderWithNoTrack() { - video_rtp_sender_ = new VideoRtpSender(); - video_rtp_sender_->SetChannel(video_channel_); + video_rtp_sender_ = new VideoRtpSender(worker_thread_); + video_rtp_sender_->SetMediaChannel(video_media_channel_); } void DestroyAudioRtpSender() { @@ -259,6 +263,8 @@ class RtpSenderReceiverTest : public testing::Test, } protected: + rtc::Thread* const network_thread_; + rtc::Thread* const worker_thread_; webrtc::RtcEventLogNullImpl event_log_; // |media_engine_| is actually owned by |channel_manager_|. cricket::FakeMediaEngine* media_engine_; @@ -772,9 +778,10 @@ TEST_F(RtpSenderReceiverTest, // Setting detailed overrides the default non-screencast mode. This should be // applied even if the track is set on construction. video_track_->set_content_hint(VideoTrackInterface::ContentHint::kDetailed); - video_rtp_sender_ = new VideoRtpSender(local_stream_->GetVideoTracks()[0], + video_rtp_sender_ = new VideoRtpSender(worker_thread_, + local_stream_->GetVideoTracks()[0], {local_stream_->label()}); - video_rtp_sender_->SetChannel(video_channel_); + video_rtp_sender_->SetMediaChannel(video_media_channel_); video_track_->set_enabled(true); // Sender is not ready to send (no SSRC) so no option should have been set. diff --git a/pc/rtptransceiver.cc b/pc/rtptransceiver.cc index 0dfc8b52a3..ecb8ec879f 100644 --- a/pc/rtptransceiver.cc +++ b/pc/rtptransceiver.cc @@ -60,11 +60,15 @@ void RtpTransceiver::SetChannel(cricket::BaseChannel* channel) { for (auto sender : senders_) { if (media_type() == cricket::MEDIA_TYPE_AUDIO) { + auto* voice_channel = static_cast(channel); static_cast(sender->internal()) - ->SetChannel(static_cast(channel)); + ->SetMediaChannel(voice_channel ? voice_channel->media_channel() + : nullptr); } else { + auto* video_channel = static_cast(channel); static_cast(sender->internal()) - ->SetChannel(static_cast(channel)); + ->SetMediaChannel(video_channel ? video_channel->media_channel() + : nullptr); } }