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;
}
