Remove 2 Invokes to the network thread when creating a channel. ...and one when destroying a channel object. This CL removes Init_n() and Deinit_n() from the BaseChannel class. Channel classes now use SetRtpTransport to do initialization and uninitialization on the network thread. Notably if an implementation has called SetRtpTransport() with a valid transport pointer, it is required that SetRtpTransport be called again with a nullptr before the channel object can be deleted. In situations where multiple channels are created, this can mean a substantial reduction in thread hops. We still hop to the worker in order to construct the objects - this can probably be avoided and SetChannel() is still a synchronous operation for the transceivers. Furthermore, teardown of channel objects also still happens synchronously and across network/worker/signaling threads. Bug: webrtc:11992 Change-Id: I68ca7596e181fc82996e3e290733d97381aa5e78 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/246740 Reviewed-by: Harald Alvestrand <hta@webrtc.org> Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org> Cr-Commit-Position: refs/heads/main@{#35738}
diff --git a/pc/channel.cc b/pc/channel.cc index fa1cddf..dac0cbc 100644 --- a/pc/channel.cc +++ b/pc/channel.cc
@@ -184,28 +184,7 @@ rtp_transport_->SignalWritableState.disconnect(this); rtp_transport_->SignalSentPacket.disconnect(this); rtp_transport_ = nullptr; -} - -void BaseChannel::Init_n(webrtc::RtpTransportInternal* rtp_transport) { - // Set the transport before we call SetInterface() since setting the interface - // pointer will call us back to set transport options. - SetRtpTransport(rtp_transport); - - // Both RTP and RTCP channels should be set, we can call SetInterface on - // the media channel and it can set network options. - RTC_DCHECK(!media_channel_->HasNetworkInterface()); - media_channel_->SetInterface(this); -} - -void BaseChannel::Deinit_n() { - // Packets arrive on the network thread, processing packets calls virtual - // functions, so need to stop this process in Deinit that is called in - // derived classes destructor. - media_channel_->SetInterface(/*iface=*/nullptr); - if (rtp_transport_) { - DisconnectFromRtpTransport_n(); - } - RTC_DCHECK(!network_initialized()); + media_channel_->SetInterface(nullptr); } bool BaseChannel::SetRtpTransport(webrtc::RtpTransportInternal* rtp_transport) { @@ -229,6 +208,10 @@ if (!ConnectToRtpTransport_n()) { return false; } + + RTC_DCHECK(!media_channel_->HasNetworkInterface()); + media_channel_->SetInterface(this); + media_channel_->OnReadyToSend(rtp_transport_->IsReadyToSend()); UpdateWritableState_n(); @@ -242,6 +225,7 @@ } } } + return true; }
diff --git a/pc/channel.h b/pc/channel.h index 7d7aeb6..fd5495c 100644 --- a/pc/channel.h +++ b/pc/channel.h
@@ -111,10 +111,6 @@ rtc::UniqueRandomIdGenerator* ssrc_generator); virtual ~BaseChannel(); - void Init_n(webrtc::RtpTransportInternal* rtp_transport) - RTC_RUN_ON(network_thread()); - void Deinit_n() RTC_RUN_ON(network_thread()); - rtc::Thread* worker_thread() const { return worker_thread_; } rtc::Thread* network_thread() const { return network_thread_; } const std::string& content_name() const override {
diff --git a/pc/channel_manager.cc b/pc/channel_manager.cc index 7f1cd98..304283e 100644 --- a/pc/channel_manager.cc +++ b/pc/channel_manager.cc
@@ -143,7 +143,6 @@ VoiceChannel* ChannelManager::CreateVoiceChannel( webrtc::Call* call, const MediaConfig& media_config, - webrtc::RtpTransportInternal* rtp_transport, rtc::Thread* signaling_thread, const std::string& content_name, bool srtp_required, @@ -157,9 +156,9 @@ // thread. if (!worker_thread_->IsCurrent()) { return worker_thread_->Invoke<VoiceChannel*>(RTC_FROM_HERE, [&] { - return CreateVoiceChannel(call, media_config, rtp_transport, - signaling_thread, content_name, srtp_required, - crypto_options, ssrc_generator, options); + return CreateVoiceChannel(call, media_config, signaling_thread, + content_name, srtp_required, crypto_options, + ssrc_generator, options); }); } @@ -176,11 +175,6 @@ absl::WrapUnique(media_channel), content_name, srtp_required, crypto_options, ssrc_generator); - network_thread_->Invoke<void>(RTC_FROM_HERE, [&] { - RTC_DCHECK_RUN_ON(voice_channel->network_thread()); - voice_channel->Init_n(rtp_transport); - }); - VoiceChannel* voice_channel_ptr = voice_channel.get(); voice_channels_.push_back(std::move(voice_channel)); return voice_channel_ptr; @@ -190,11 +184,6 @@ TRACE_EVENT0("webrtc", "ChannelManager::DestroyVoiceChannel"); RTC_DCHECK(voice_channel); - network_thread_->Invoke<void>(RTC_FROM_HERE, [&] { - RTC_DCHECK_RUN_ON(voice_channel->network_thread()); - voice_channel->Deinit_n(); - }); - if (!worker_thread_->IsCurrent()) { worker_thread_->Invoke<void>(RTC_FROM_HERE, [&] { DestroyVoiceChannel(voice_channel); }); @@ -212,7 +201,6 @@ VideoChannel* ChannelManager::CreateVideoChannel( webrtc::Call* call, const MediaConfig& media_config, - webrtc::RtpTransportInternal* rtp_transport, rtc::Thread* signaling_thread, const std::string& content_name, bool srtp_required, @@ -227,9 +215,9 @@ // thread. if (!worker_thread_->IsCurrent()) { return worker_thread_->Invoke<VideoChannel*>(RTC_FROM_HERE, [&] { - return CreateVideoChannel(call, media_config, rtp_transport, - signaling_thread, content_name, srtp_required, - crypto_options, ssrc_generator, options, + return CreateVideoChannel(call, media_config, signaling_thread, + content_name, srtp_required, crypto_options, + ssrc_generator, options, video_bitrate_allocator_factory); }); } @@ -248,11 +236,6 @@ absl::WrapUnique(media_channel), content_name, srtp_required, crypto_options, ssrc_generator); - network_thread_->Invoke<void>(RTC_FROM_HERE, [&] { - RTC_DCHECK_RUN_ON(video_channel->network_thread()); - video_channel->Init_n(rtp_transport); - }); - VideoChannel* video_channel_ptr = video_channel.get(); video_channels_.push_back(std::move(video_channel)); return video_channel_ptr; @@ -262,11 +245,6 @@ TRACE_EVENT0("webrtc", "ChannelManager::DestroyVideoChannel"); RTC_DCHECK(video_channel); - network_thread_->Invoke<void>(RTC_FROM_HERE, [&] { - RTC_DCHECK_RUN_ON(video_channel->network_thread()); - video_channel->Deinit_n(); - }); - if (!worker_thread_->IsCurrent()) { worker_thread_->Invoke<void>(RTC_FROM_HERE, [&] { DestroyVideoChannel(video_channel); });
diff --git a/pc/channel_manager.h b/pc/channel_manager.h index 43fa279..0952099 100644 --- a/pc/channel_manager.h +++ b/pc/channel_manager.h
@@ -81,7 +81,6 @@ // Creates a voice channel, to be associated with the specified session. VoiceChannel* CreateVoiceChannel(webrtc::Call* call, const MediaConfig& media_config, - webrtc::RtpTransportInternal* rtp_transport, rtc::Thread* signaling_thread, const std::string& content_name, bool srtp_required, @@ -97,7 +96,6 @@ VideoChannel* CreateVideoChannel( webrtc::Call* call, const MediaConfig& media_config, - webrtc::RtpTransportInternal* rtp_transport, rtc::Thread* signaling_thread, const std::string& content_name, bool srtp_required,
diff --git a/pc/channel_manager_unittest.cc b/pc/channel_manager_unittest.cc index 88de1f6..80f30c1 100644 --- a/pc/channel_manager_unittest.cc +++ b/pc/channel_manager_unittest.cc
@@ -69,16 +69,18 @@ void TestCreateDestroyChannels(webrtc::RtpTransportInternal* rtp_transport) { RTC_DCHECK_RUN_ON(worker_); cricket::VoiceChannel* voice_channel = cm_->CreateVoiceChannel( - &fake_call_, cricket::MediaConfig(), rtp_transport, - rtc::Thread::Current(), cricket::CN_AUDIO, kDefaultSrtpRequired, - webrtc::CryptoOptions(), &ssrc_generator_, AudioOptions()); - EXPECT_TRUE(voice_channel != nullptr); + &fake_call_, cricket::MediaConfig(), rtc::Thread::Current(), + cricket::CN_AUDIO, kDefaultSrtpRequired, webrtc::CryptoOptions(), + &ssrc_generator_, AudioOptions()); + ASSERT_TRUE(voice_channel != nullptr); + cricket::VideoChannel* video_channel = cm_->CreateVideoChannel( - &fake_call_, cricket::MediaConfig(), rtp_transport, - rtc::Thread::Current(), cricket::CN_VIDEO, kDefaultSrtpRequired, - webrtc::CryptoOptions(), &ssrc_generator_, VideoOptions(), + &fake_call_, cricket::MediaConfig(), rtc::Thread::Current(), + cricket::CN_VIDEO, kDefaultSrtpRequired, webrtc::CryptoOptions(), + &ssrc_generator_, VideoOptions(), video_bitrate_allocator_factory_.get()); - EXPECT_TRUE(video_channel != nullptr); + ASSERT_TRUE(video_channel != nullptr); + cm_->DestroyVideoChannel(video_channel); cm_->DestroyVoiceChannel(voice_channel); }
diff --git a/pc/channel_unittest.cc b/pc/channel_unittest.cc index 42497b8..0a12a7f 100644 --- a/pc/channel_unittest.cc +++ b/pc/channel_unittest.cc
@@ -280,11 +280,11 @@ network_thread_->Invoke<void>(RTC_FROM_HERE, [this]() { if (channel1_) { RTC_DCHECK_RUN_ON(channel1_->network_thread()); - channel1_->Deinit_n(); + channel1_->SetRtpTransport(nullptr); } if (channel2_) { RTC_DCHECK_RUN_ON(channel2_->network_thread()); - channel2_->Deinit_n(); + channel2_->SetRtpTransport(nullptr); } }); } @@ -1450,7 +1450,7 @@ &ssrc_generator_); network_thread->Invoke<void>(RTC_FROM_HERE, [&]() { RTC_DCHECK_RUN_ON(channel->network_thread()); - channel->Init_n(rtp_transport); + channel->SetRtpTransport(rtp_transport); }); return channel; } @@ -1536,7 +1536,7 @@ &ssrc_generator_); network_thread->Invoke<void>(RTC_FROM_HERE, [&]() { RTC_DCHECK_RUN_ON(channel->network_thread()); - channel->Init_n(rtp_transport); + channel->SetRtpTransport(rtp_transport); }); return channel; }
diff --git a/pc/rtp_sender_receiver_unittest.cc b/pc/rtp_sender_receiver_unittest.cc index 8043dd0..6127622 100644 --- a/pc/rtp_sender_receiver_unittest.cc +++ b/pc/rtp_sender_receiver_unittest.cc
@@ -122,14 +122,18 @@ rtp_transport_ = CreateDtlsSrtpTransport(); voice_channel_ = channel_manager_->CreateVoiceChannel( - &fake_call_, cricket::MediaConfig(), rtp_transport_.get(), - rtc::Thread::Current(), cricket::CN_AUDIO, srtp_required, - webrtc::CryptoOptions(), &ssrc_generator_, cricket::AudioOptions()); + &fake_call_, cricket::MediaConfig(), rtc::Thread::Current(), + cricket::CN_AUDIO, srtp_required, webrtc::CryptoOptions(), + &ssrc_generator_, cricket::AudioOptions()); video_channel_ = channel_manager_->CreateVideoChannel( - &fake_call_, cricket::MediaConfig(), rtp_transport_.get(), - rtc::Thread::Current(), cricket::CN_VIDEO, srtp_required, - webrtc::CryptoOptions(), &ssrc_generator_, cricket::VideoOptions(), + &fake_call_, cricket::MediaConfig(), rtc::Thread::Current(), + cricket::CN_VIDEO, srtp_required, webrtc::CryptoOptions(), + &ssrc_generator_, cricket::VideoOptions(), video_bitrate_allocator_factory_.get()); + + voice_channel_->SetRtpTransport(rtp_transport_.get()); + video_channel_->SetRtpTransport(rtp_transport_.get()); + voice_channel_->Enable(true); video_channel_->Enable(true); voice_media_channel_ = media_engine_->GetVoiceChannel(0); @@ -170,6 +174,9 @@ video_track_ = nullptr; audio_track_ = nullptr; + voice_channel_->SetRtpTransport(nullptr); + video_channel_->SetRtpTransport(nullptr); + channel_manager_->DestroyVoiceChannel(voice_channel_); channel_manager_->DestroyVideoChannel(video_channel_);
diff --git a/pc/rtp_transceiver.cc b/pc/rtp_transceiver.cc index 25ae804..e152d76 100644 --- a/pc/rtp_transceiver.cc +++ b/pc/rtp_transceiver.cc
@@ -156,7 +156,9 @@ } } -void RtpTransceiver::SetChannel(cricket::ChannelInterface* channel) { +void RtpTransceiver::SetChannel( + cricket::ChannelInterface* channel, + std::function<RtpTransportInternal*(const std::string&)> transport_lookup) { RTC_DCHECK_RUN_ON(thread_); // Cannot set a non-null channel on a stopped transceiver. if (stopped_ && channel) { @@ -164,6 +166,7 @@ } RTC_DCHECK(channel || channel_); + RTC_DCHECK(!channel || transport_lookup) << "lookup function not supplied"; RTC_LOG_THREAD_BLOCK_COUNT(); @@ -189,11 +192,13 @@ channel_manager_->network_thread()->Invoke<void>(RTC_FROM_HERE, [&]() { if (channel_) { channel_->SetFirstPacketReceivedCallback(nullptr); + channel_->SetRtpTransport(nullptr); } channel_ = channel; if (channel_) { + channel_->SetRtpTransport(transport_lookup(channel_->content_name())); channel_->SetFirstPacketReceivedCallback( [thread = thread_, flag = signaling_thread_safety_, this]() mutable { thread->PostTask(ToQueuedTask(
diff --git a/pc/rtp_transceiver.h b/pc/rtp_transceiver.h index c995329..b8dbb67 100644 --- a/pc/rtp_transceiver.h +++ b/pc/rtp_transceiver.h
@@ -100,8 +100,36 @@ cricket::ChannelInterface* channel() const { return channel_; } // Sets the Voice/VideoChannel. The caller must pass in the correct channel - // implementation based on the type of the transceiver. - void SetChannel(cricket::ChannelInterface* channel); + // implementation based on the type of the transceiver. The call must + // furthermore be made on the signaling thread. + // + // `channel`: The channel instance to be associated with the transceiver. + // When a valid pointer is passed for `channel`, the state of the object + // is expected to be newly constructed and not initalized for network + // activity (see next parameter for more). + // + // NOTE: For all practical purposes, the ownership of the channel + // object should be considered to lie with the transceiver until + // `SetChannel()` is called again with nullptr set as the new channel. + // Moving forward, this parameter will change to either be a + // std::unique_ptr<> or the full construction of the channel object will + // be moved to happen within the context of the transceiver class. + // + // `transport_lookup`: When `channel` points to a valid channel object, this + // callback function will be used to look up the `RtpTransport` object + // to associate with the channel via `BaseChannel::SetRtpTransport`. + // The lookup function will be called on the network thread, synchronously + // during the call to `SetChannel`. This means that the caller of + // `SetChannel()` may provide a callback function that references state + // that exists within the calling scope of SetChannel (e.g. a variable + // on the stack). + // The reason for this design is to limit the number of times we jump + // synchronously to the network thread from the signaling thread. + // The callback allows us to combine the transport lookup with network + // state initialization of the channel object. + void SetChannel(cricket::ChannelInterface* channel, + std::function<RtpTransportInternal*(const std::string&)> + transport_lookup); // Adds an RtpSender of the appropriate type to be owned by this transceiver. // Must not be null.
diff --git a/pc/rtp_transceiver_unittest.cc b/pc/rtp_transceiver_unittest.cc index 35d9265..a1a2a19 100644 --- a/pc/rtp_transceiver_unittest.cc +++ b/pc/rtp_transceiver_unittest.cc
@@ -36,13 +36,19 @@ TEST(RtpTransceiverTest, CannotSetChannelOnStoppedTransceiver) { auto cm = cricket::ChannelManager::Create( nullptr, true, rtc::Thread::Current(), rtc::Thread::Current()); + const std::string content_name("my_mid"); RtpTransceiver transceiver(cricket::MediaType::MEDIA_TYPE_AUDIO, cm.get()); cricket::MockChannelInterface channel1; EXPECT_CALL(channel1, media_type()) .WillRepeatedly(Return(cricket::MediaType::MEDIA_TYPE_AUDIO)); + EXPECT_CALL(channel1, content_name()).WillRepeatedly(ReturnRef(content_name)); EXPECT_CALL(channel1, SetFirstPacketReceivedCallback(_)); + EXPECT_CALL(channel1, SetRtpTransport(_)).WillRepeatedly(Return(true)); - transceiver.SetChannel(&channel1); + transceiver.SetChannel(&channel1, [&](const std::string& mid) { + EXPECT_EQ(mid, content_name); + return nullptr; + }); EXPECT_EQ(&channel1, transceiver.channel()); // Stop the transceiver. @@ -54,7 +60,7 @@ .WillRepeatedly(Return(cricket::MediaType::MEDIA_TYPE_AUDIO)); // Channel can no longer be set, so this call should be a no-op. - transceiver.SetChannel(&channel2); + transceiver.SetChannel(&channel2, [](const std::string&) { return nullptr; }); EXPECT_EQ(&channel1, transceiver.channel()); } @@ -62,14 +68,20 @@ TEST(RtpTransceiverTest, CanUnsetChannelOnStoppedTransceiver) { auto cm = cricket::ChannelManager::Create( nullptr, true, rtc::Thread::Current(), rtc::Thread::Current()); + const std::string content_name("my_mid"); RtpTransceiver transceiver(cricket::MediaType::MEDIA_TYPE_VIDEO, cm.get()); cricket::MockChannelInterface channel; EXPECT_CALL(channel, media_type()) .WillRepeatedly(Return(cricket::MediaType::MEDIA_TYPE_VIDEO)); + EXPECT_CALL(channel, content_name()).WillRepeatedly(ReturnRef(content_name)); EXPECT_CALL(channel, SetFirstPacketReceivedCallback(_)) .WillRepeatedly(testing::Return()); + EXPECT_CALL(channel, SetRtpTransport(_)).WillRepeatedly(Return(true)); - transceiver.SetChannel(&channel); + transceiver.SetChannel(&channel, [&](const std::string& mid) { + EXPECT_EQ(mid, content_name); + return nullptr; + }); EXPECT_EQ(&channel, transceiver.channel()); // Stop the transceiver. @@ -77,7 +89,7 @@ EXPECT_EQ(&channel, transceiver.channel()); // Set the channel to `nullptr`. - transceiver.SetChannel(nullptr); + transceiver.SetChannel(nullptr, nullptr); EXPECT_EQ(nullptr, transceiver.channel()); } @@ -279,6 +291,7 @@ TEST_F(RtpTransceiverTestForHeaderExtensions, NoNegotiatedHdrExtsWithChannelWithoutNegotiation) { + const std::string content_name("my_mid"); EXPECT_CALL(*receiver_.get(), SetMediaChannel(_)); EXPECT_CALL(*receiver_.get(), StopAndEndTrack()); EXPECT_CALL(*sender_.get(), SetMediaChannel(_)); @@ -289,11 +302,16 @@ EXPECT_CALL(mock_channel, media_type()) .WillRepeatedly(Return(cricket::MediaType::MEDIA_TYPE_AUDIO)); EXPECT_CALL(mock_channel, media_channel()).WillRepeatedly(Return(nullptr)); - transceiver_.SetChannel(&mock_channel); + EXPECT_CALL(mock_channel, content_name()) + .WillRepeatedly(ReturnRef(content_name)); + EXPECT_CALL(mock_channel, SetRtpTransport(_)).WillRepeatedly(Return(true)); + transceiver_.SetChannel(&mock_channel, + [](const std::string&) { return nullptr; }); EXPECT_THAT(transceiver_.HeaderExtensionsNegotiated(), ElementsAre()); } TEST_F(RtpTransceiverTestForHeaderExtensions, ReturnsNegotiatedHdrExts) { + const std::string content_name("my_mid"); EXPECT_CALL(*receiver_.get(), SetMediaChannel(_)); EXPECT_CALL(*receiver_.get(), StopAndEndTrack()); EXPECT_CALL(*sender_.get(), SetMediaChannel(_)); @@ -305,6 +323,9 @@ EXPECT_CALL(mock_channel, media_type()) .WillRepeatedly(Return(cricket::MediaType::MEDIA_TYPE_AUDIO)); EXPECT_CALL(mock_channel, media_channel()).WillRepeatedly(Return(nullptr)); + EXPECT_CALL(mock_channel, content_name()) + .WillRepeatedly(ReturnRef(content_name)); + EXPECT_CALL(mock_channel, SetRtpTransport(_)).WillRepeatedly(Return(true)); cricket::RtpHeaderExtensions extensions = {webrtc::RtpExtension("uri1", 1), webrtc::RtpExtension("uri2", 2)}; @@ -312,7 +333,8 @@ description.set_rtp_header_extensions(extensions); transceiver_.OnNegotiationUpdate(SdpType::kAnswer, &description); - transceiver_.SetChannel(&mock_channel); + transceiver_.SetChannel(&mock_channel, + [](const std::string&) { return nullptr; }); EXPECT_THAT(transceiver_.HeaderExtensionsNegotiated(), ElementsAre(RtpHeaderExtensionCapability( "uri1", 1, RtpTransceiverDirection::kSendRecv),
diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc index 4ab336c..ea1c4c4 100644 --- a/pc/sdp_offer_answer.cc +++ b/pc/sdp_offer_answer.cc
@@ -3522,7 +3522,7 @@ cricket::ChannelInterface* channel = transceiver->internal()->channel(); if (content.rejected) { if (channel) { - transceiver->internal()->SetChannel(nullptr); + transceiver->internal()->SetChannel(nullptr, nullptr); DestroyChannelInterface(channel); } } else { @@ -3537,7 +3537,9 @@ return RTCError(RTCErrorType::INTERNAL_ERROR, "Failed to create channel for mid=" + content.name); } - transceiver->internal()->SetChannel(channel); + transceiver->internal()->SetChannel(channel, [&](const std::string& mid) { + return transport_controller()->GetRtpTransport(mid); + }); } } return RTCError::OK(); @@ -4714,7 +4716,10 @@ return RTCError(RTCErrorType::INTERNAL_ERROR, "Failed to create voice channel."); } - rtp_manager()->GetAudioTransceiver()->internal()->SetChannel(voice_channel); + rtp_manager()->GetAudioTransceiver()->internal()->SetChannel( + voice_channel, [&](const std::string& mid) { + return transport_controller()->GetRtpTransport(mid); + }); } const cricket::ContentInfo* video = cricket::GetFirstVideoContent(&desc); @@ -4725,7 +4730,10 @@ return RTCError(RTCErrorType::INTERNAL_ERROR, "Failed to create video channel."); } - rtp_manager()->GetVideoTransceiver()->internal()->SetChannel(video_channel); + rtp_manager()->GetVideoTransceiver()->internal()->SetChannel( + video_channel, [&](const std::string& mid) { + return transport_controller()->GetRtpTransport(mid); + }); } const cricket::ContentInfo* data = cricket::GetFirstDataContent(&desc); @@ -4748,18 +4756,13 @@ if (!channel_manager()->media_engine()) return nullptr; - // TODO(tommi): Avoid hop to network thread. - RtpTransportInternal* rtp_transport = pc_->GetRtpTransport(mid); - // TODO(bugs.webrtc.org/11992): CreateVoiceChannel internally switches to the // worker thread. We shouldn't be using the `call_ptr_` hack here but simply // be on the worker thread and use `call_` (update upstream code). - // TODO(tommi): This hops to the worker and from the worker to the network - // thread (blocking both signal and worker). return channel_manager()->CreateVoiceChannel( - pc_->call_ptr(), pc_->configuration()->media_config, rtp_transport, - signaling_thread(), mid, pc_->SrtpRequired(), pc_->GetCryptoOptions(), - &ssrc_generator_, audio_options()); + pc_->call_ptr(), pc_->configuration()->media_config, signaling_thread(), + mid, pc_->SrtpRequired(), pc_->GetCryptoOptions(), &ssrc_generator_, + audio_options()); } // TODO(steveanton): Perhaps this should be managed by the RtpTransceiver. @@ -4770,17 +4773,13 @@ if (!channel_manager()->media_engine()) return nullptr; - // NOTE: This involves a non-ideal hop (Invoke) over to the network thread. - RtpTransportInternal* rtp_transport = pc_->GetRtpTransport(mid); - // TODO(bugs.webrtc.org/11992): CreateVideoChannel internally switches to the // worker thread. We shouldn't be using the `call_ptr_` hack here but simply // be on the worker thread and use `call_` (update upstream code). return channel_manager()->CreateVideoChannel( - pc_->call_ptr(), pc_->configuration()->media_config, rtp_transport, - signaling_thread(), mid, pc_->SrtpRequired(), pc_->GetCryptoOptions(), - &ssrc_generator_, video_options(), - video_bitrate_allocator_factory_.get()); + pc_->call_ptr(), pc_->configuration()->media_config, signaling_thread(), + mid, pc_->SrtpRequired(), pc_->GetCryptoOptions(), &ssrc_generator_, + video_options(), video_bitrate_allocator_factory_.get()); } bool SdpOfferAnswerHandler::CreateDataChannel(const std::string& mid) { @@ -4825,7 +4824,7 @@ // so if ownership of the channel object lies with the transceiver, we could // un-set the channel pointer and uninitialize/destruct the channel object // at the same time, rather than in separate steps. - transceiver->internal()->SetChannel(nullptr); + transceiver->internal()->SetChannel(nullptr, nullptr); // TODO(tommi): All channel objects end up getting deleted on the // worker thread (ideally should be on the network thread but the // MediaChannel objects are tied to the worker. Can the teardown be done
diff --git a/pc/test/fake_peer_connection_for_stats.h b/pc/test/fake_peer_connection_for_stats.h index 5a9bc5d..877949d 100644 --- a/pc/test/fake_peer_connection_for_stats.h +++ b/pc/test/fake_peer_connection_for_stats.h
@@ -208,7 +208,8 @@ webrtc::CryptoOptions(), &ssrc_generator_, transport_name); GetOrCreateFirstTransceiverOfType(cricket::MEDIA_TYPE_AUDIO) ->internal() - ->SetChannel(voice_channel_.get()); + ->SetChannel(voice_channel_.get(), + [](const std::string&) { return nullptr; }); return voice_media_channel_ptr; } @@ -225,7 +226,8 @@ webrtc::CryptoOptions(), &ssrc_generator_, transport_name); GetOrCreateFirstTransceiverOfType(cricket::MEDIA_TYPE_VIDEO) ->internal() - ->SetChannel(video_channel_.get()); + ->SetChannel(video_channel_.get(), + [](const std::string&) { return nullptr; }); return video_media_channel_ptr; }