Move ownership of objects from channel_manager to connection_context

This is a preparatory step in deleting the ChannelManager class.

Also delete some declarations whose implementation was previously removed.

Bug: webrtc:13931
Change-Id: I8764c00fa696932e79fcfe17550ef2490d6a1ed1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/262804
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36923}
This commit is contained in:
Harald Alvestrand
2022-05-18 07:51:34 +00:00
committed by WebRTC LUCI CQ
parent 0359ba2225
commit 0ac50b9dfd
8 changed files with 87 additions and 64 deletions

View File

@ -27,7 +27,8 @@ namespace cricket {
// static
std::unique_ptr<ChannelManager> ChannelManager::Create(
std::unique_ptr<MediaEngineInterface> media_engine,
MediaEngineInterface* media_engine,
rtc::UniqueRandomIdGenerator* ssrc_generator,
bool enable_rtx,
rtc::Thread* worker_thread,
rtc::Thread* network_thread) {
@ -35,15 +36,16 @@ std::unique_ptr<ChannelManager> ChannelManager::Create(
RTC_DCHECK(worker_thread);
return absl::WrapUnique(new ChannelManager(
std::move(media_engine), enable_rtx, worker_thread, network_thread));
media_engine, ssrc_generator, enable_rtx, worker_thread, network_thread));
}
ChannelManager::ChannelManager(
std::unique_ptr<MediaEngineInterface> media_engine,
bool enable_rtx,
rtc::Thread* worker_thread,
rtc::Thread* network_thread)
: media_engine_(std::move(media_engine)),
ChannelManager::ChannelManager(MediaEngineInterface* media_engine,
rtc::UniqueRandomIdGenerator* ssrc_generator,
bool enable_rtx,
rtc::Thread* worker_thread,
rtc::Thread* network_thread)
: media_engine_(media_engine),
ssrc_generator_(ssrc_generator),
signaling_thread_(rtc::Thread::Current()),
worker_thread_(worker_thread),
network_thread_(network_thread),
@ -61,14 +63,6 @@ ChannelManager::ChannelManager(
ChannelManager::~ChannelManager() {
RTC_DCHECK_RUN_ON(signaling_thread_);
worker_thread_->Invoke<void>(RTC_FROM_HERE, [&] {
RTC_DCHECK_RUN_ON(worker_thread_);
// While `media_engine_` is const throughout the ChannelManager's lifetime,
// it requires destruction to happen on the worker thread. Instead of
// marking the pointer as non-const, we live with this const_cast<> in the
// destructor.
const_cast<std::unique_ptr<MediaEngineInterface>&>(media_engine_).reset();
});
}
void ChannelManager::GetSupportedAudioSendCodecs(
@ -152,7 +146,7 @@ std::unique_ptr<VoiceChannel> ChannelManager::CreateVoiceChannel(
auto voice_channel = std::make_unique<VoiceChannel>(
worker_thread_, network_thread_, signaling_thread_,
absl::WrapUnique(media_channel), mid, srtp_required, crypto_options,
&ssrc_generator_);
ssrc_generator_);
return voice_channel;
}
@ -191,7 +185,7 @@ std::unique_ptr<VideoChannel> ChannelManager::CreateVideoChannel(
auto video_channel = std::make_unique<VideoChannel>(
worker_thread_, network_thread_, signaling_thread_,
absl::WrapUnique(media_channel), mid, srtp_required, crypto_options,
&ssrc_generator_);
ssrc_generator_);
return video_channel;
}

View File

@ -50,7 +50,8 @@ class ChannelManager : public ChannelFactoryInterface {
// If media_engine is non-nullptr, then the returned ChannelManager instance
// will own that reference and media engine initialization
static std::unique_ptr<ChannelManager> Create(
std::unique_ptr<MediaEngineInterface> media_engine,
MediaEngineInterface* media_engine,
rtc::UniqueRandomIdGenerator* ssrc_generator,
bool enable_rtx,
rtc::Thread* worker_thread,
rtc::Thread* network_thread);
@ -60,8 +61,8 @@ class ChannelManager : public ChannelFactoryInterface {
rtc::Thread* worker_thread() const { return worker_thread_; }
rtc::Thread* network_thread() const { return network_thread_; }
MediaEngineInterface* media_engine() { return media_engine_.get(); }
rtc::UniqueRandomIdGenerator& ssrc_generator() { return ssrc_generator_; }
MediaEngineInterface* media_engine() { return media_engine_; }
rtc::UniqueRandomIdGenerator& ssrc_generator() { return *ssrc_generator_; }
// Retrieves the list of supported audio & video codec types.
// Can be called before starting the media engine.
@ -97,29 +98,19 @@ class ChannelManager : public ChannelFactoryInterface {
override;
protected:
ChannelManager(std::unique_ptr<MediaEngineInterface> media_engine,
ChannelManager(MediaEngineInterface* media_engine,
rtc::UniqueRandomIdGenerator* ssrc_generator_,
bool enable_rtx,
rtc::Thread* worker_thread,
rtc::Thread* network_thread);
// Destroys a voice channel created by CreateVoiceChannel.
void DestroyVoiceChannel(VoiceChannel* voice_channel);
// Destroys a video channel created by CreateVideoChannel.
void DestroyVideoChannel(VideoChannel* video_channel);
private:
const std::unique_ptr<MediaEngineInterface> media_engine_; // Nullable.
MediaEngineInterface* media_engine_; // Nullable.
rtc::UniqueRandomIdGenerator* ssrc_generator_;
rtc::Thread* const signaling_thread_;
rtc::Thread* const worker_thread_;
rtc::Thread* const network_thread_;
// This object should be used to generate any SSRC that is not explicitly
// specified by the user (or by the remote party).
// TODO(bugs.webrtc.org/12666): This variable is used from both the signaling
// and worker threads. See if we can't restrict usage to a single thread.
rtc::UniqueRandomIdGenerator ssrc_generator_;
const bool enable_rtx_;
};

View File

@ -58,7 +58,9 @@ class ChannelManagerTest : public ::testing::Test {
worker_(rtc::Thread::Current()),
video_bitrate_allocator_factory_(
webrtc::CreateBuiltinVideoBitrateAllocatorFactory()),
cm_(cricket::ChannelManager::Create(CreateFakeMediaEngine(),
media_engine_(CreateFakeMediaEngine()),
cm_(cricket::ChannelManager::Create(media_engine_.get(),
&ssrc_generator_,
false,
worker_,
network_.get())),
@ -89,6 +91,8 @@ class ChannelManagerTest : public ::testing::Test {
rtc::Thread* const worker_;
std::unique_ptr<webrtc::VideoBitrateAllocatorFactory>
video_bitrate_allocator_factory_;
std::unique_ptr<cricket::MediaEngineInterface> media_engine_;
rtc::UniqueRandomIdGenerator ssrc_generator_;
std::unique_ptr<cricket::ChannelManager> cm_;
cricket::FakeCall fake_call_;
webrtc::test::ScopedKeyValueConfig field_trials_;
@ -106,7 +110,7 @@ TEST_F(ChannelManagerTest, SetVideoRtxEnabled) {
EXPECT_FALSE(ContainsMatchingCodec(recv_codecs, rtx_codec, &field_trials_));
// Enable and check.
cm_ = cricket::ChannelManager::Create(CreateFakeMediaEngine(),
cm_ = cricket::ChannelManager::Create(media_engine_.get(), &ssrc_generator_,
true, worker_, network_.get());
cm_->GetSupportedVideoSendCodecs(&send_codecs);
EXPECT_TRUE(ContainsMatchingCodec(send_codecs, rtx_codec, &field_trials_));
@ -114,7 +118,7 @@ TEST_F(ChannelManagerTest, SetVideoRtxEnabled) {
EXPECT_TRUE(ContainsMatchingCodec(recv_codecs, rtx_codec, &field_trials_));
// Disable and check.
cm_ = cricket::ChannelManager::Create(CreateFakeMediaEngine(),
cm_ = cricket::ChannelManager::Create(media_engine_.get(), &ssrc_generator_,
false, worker_, network_.get());
cm_->GetSupportedVideoSendCodecs(&send_codecs);
EXPECT_FALSE(ContainsMatchingCodec(send_codecs, rtx_codec, &field_trials_));

View File

@ -99,6 +99,7 @@ ConnectionContext::ConnectionContext(
wraps_current_thread_)),
trials_(dependencies->trials ? std::move(dependencies->trials)
: std::make_unique<FieldTrialBasedConfig>()),
media_engine_(std::move(dependencies->media_engine)),
network_monitor_factory_(
std::move(dependencies->network_monitor_factory)),
call_factory_(std::move(dependencies->call_factory)),
@ -144,8 +145,9 @@ ConnectionContext::ConnectionContext(
default_socket_factory_ =
std::make_unique<rtc::BasicPacketSocketFactory>(socket_factory);
// TODO(bugs.webrtc.org/13931): Delete ChannelManager when functions gone.
channel_manager_ = cricket::ChannelManager::Create(
std::move(dependencies->media_engine),
media_engine_.get(), &ssrc_generator_,
/*enable_rtx=*/true, worker_thread(), network_thread());
// Set warning levels on the threads, to give warnings when response
@ -161,6 +163,15 @@ ConnectionContext::ConnectionContext(
ConnectionContext::~ConnectionContext() {
RTC_DCHECK_RUN_ON(signaling_thread_);
channel_manager_.reset(nullptr);
worker_thread_->Invoke<void>(RTC_FROM_HERE, [&] {
RTC_DCHECK_RUN_ON(worker_thread());
// While `media_engine_` is const throughout the ChannelManager's lifetime,
// it requires destruction to happen on the worker thread. Instead of
// marking the pointer as non-const, we live with this const_cast<> in the
// destructor.
const_cast<std::unique_ptr<cricket::MediaEngineInterface>&>(media_engine_)
.reset();
});
// Make sure `worker_thread()` and `signaling_thread()` outlive
// `default_socket_factory_` and `default_network_manager_`.

View File

@ -70,6 +70,7 @@ class ConnectionContext final
}
cricket::ChannelManager* channel_manager() const;
cricket::MediaEngineInterface* media_engine() const;
rtc::Thread* signaling_thread() { return signaling_thread_; }
const rtc::Thread* signaling_thread() const { return signaling_thread_; }
@ -121,6 +122,13 @@ class ConnectionContext final
// channel_manager is accessed both on signaling thread and worker thread.
// Const after construction, explicitly cleared in destructor.
std::unique_ptr<cricket::ChannelManager> channel_manager_;
const std::unique_ptr<cricket::MediaEngineInterface> media_engine_;
// This object should be used to generate any SSRC that is not explicitly
// specified by the user (or by the remote party).
// TODO(bugs.webrtc.org/12666): This variable is used from both the signaling
// and worker threads. See if we can't restrict usage to a single thread.
rtc::UniqueRandomIdGenerator ssrc_generator_;
std::unique_ptr<rtc::NetworkMonitorFactory> const network_monitor_factory_
RTC_GUARDED_BY(signaling_thread_);
std::unique_ptr<rtc::BasicNetworkManager> default_network_manager_

View File

@ -114,12 +114,12 @@ class RtpSenderReceiverTest
webrtc::CreateBuiltinVideoBitrateAllocatorFactory()),
// Create fake media engine/etc. so we can create channels to use to
// test RtpSenders/RtpReceivers.
media_engine_(new cricket::FakeMediaEngine()),
media_engine_(std::make_unique<cricket::FakeMediaEngine>()),
fake_call_(worker_thread_, network_thread_),
local_stream_(MediaStream::Create(kStreamId1)) {
worker_thread_->Invoke<void>(RTC_FROM_HERE, [&]() {
channel_manager_ = cricket::ChannelManager::Create(
absl::WrapUnique(media_engine_), false, worker_thread_,
media_engine_.get(), &ssrc_generator_, false, worker_thread_,
network_thread_);
});
@ -526,8 +526,8 @@ class RtpSenderReceiverTest
std::unique_ptr<webrtc::RtpTransportInternal> rtp_transport_;
std::unique_ptr<webrtc::VideoBitrateAllocatorFactory>
video_bitrate_allocator_factory_;
// `media_engine_` is actually owned by `channel_manager_`.
cricket::FakeMediaEngine* media_engine_;
std::unique_ptr<cricket::FakeMediaEngine> media_engine_;
rtc::UniqueRandomIdGenerator ssrc_generator_;
std::unique_ptr<cricket::ChannelManager> channel_manager_;
cricket::FakeCall fake_call_;
std::unique_ptr<cricket::VoiceChannel> voice_channel_;

View File

@ -38,22 +38,31 @@ using ::testing::ReturnRef;
namespace webrtc {
namespace {
class ChannelManagerForTest : public cricket::ChannelManager {
class RtpTransceiverTest : public testing::Test {
public:
ChannelManagerForTest()
: cricket::ChannelManager(std::make_unique<cricket::FakeMediaEngine>(),
true,
rtc::Thread::Current(),
rtc::Thread::Current()) {}
RtpTransceiverTest()
: channel_manager_(
cricket::ChannelManager::Create(&fake_media_engine_,
&ssrc_generator_,
true,
rtc::Thread::Current(),
rtc::Thread::Current())) {}
protected:
cricket::ChannelManager* channel_manager() { return channel_manager_.get(); }
private:
cricket::FakeMediaEngine fake_media_engine_;
rtc::UniqueRandomIdGenerator ssrc_generator_;
std::unique_ptr<cricket::ChannelManager> channel_manager_;
};
} // namespace
// Checks that a channel cannot be set on a stopped `RtpTransceiver`.
TEST(RtpTransceiverTest, CannotSetChannelOnStoppedTransceiver) {
ChannelManagerForTest cm;
TEST_F(RtpTransceiverTest, CannotSetChannelOnStoppedTransceiver) {
const std::string content_name("my_mid");
auto transceiver = rtc::make_ref_counted<RtpTransceiver>(
cricket::MediaType::MEDIA_TYPE_AUDIO, &cm);
cricket::MediaType::MEDIA_TYPE_AUDIO, channel_manager());
auto channel1 = std::make_unique<cricket::MockChannelInterface>();
EXPECT_CALL(*channel1, media_type())
.WillRepeatedly(Return(cricket::MediaType::MEDIA_TYPE_AUDIO));
@ -85,11 +94,10 @@ TEST(RtpTransceiverTest, CannotSetChannelOnStoppedTransceiver) {
}
// Checks that a channel can be unset on a stopped `RtpTransceiver`
TEST(RtpTransceiverTest, CanUnsetChannelOnStoppedTransceiver) {
ChannelManagerForTest cm;
TEST_F(RtpTransceiverTest, CanUnsetChannelOnStoppedTransceiver) {
const std::string content_name("my_mid");
auto transceiver = rtc::make_ref_counted<RtpTransceiver>(
cricket::MediaType::MEDIA_TYPE_VIDEO, &cm);
cricket::MediaType::MEDIA_TYPE_VIDEO, channel_manager());
auto channel = std::make_unique<cricket::MockChannelInterface>();
EXPECT_CALL(*channel, media_type())
.WillRepeatedly(Return(cricket::MediaType::MEDIA_TYPE_VIDEO));
@ -114,7 +122,7 @@ TEST(RtpTransceiverTest, CanUnsetChannelOnStoppedTransceiver) {
EXPECT_EQ(nullptr, transceiver->channel());
}
class RtpTransceiverUnifiedPlanTest : public ::testing::Test {
class RtpTransceiverUnifiedPlanTest : public RtpTransceiverTest {
public:
RtpTransceiverUnifiedPlanTest()
: transceiver_(rtc::make_ref_counted<RtpTransceiver>(
@ -125,8 +133,8 @@ class RtpTransceiverUnifiedPlanTest : public ::testing::Test {
rtc::Thread::Current(),
rtc::Thread::Current(),
receiver_),
&channel_manager_,
channel_manager_.media_engine()->voice().GetRtpHeaderExtensions(),
channel_manager(),
channel_manager()->media_engine()->voice().GetRtpHeaderExtensions(),
/* on_negotiation_needed= */ [] {})) {}
static rtc::scoped_refptr<MockRtpReceiverInternal> MockReceiver() {
@ -145,7 +153,6 @@ class RtpTransceiverUnifiedPlanTest : public ::testing::Test {
rtc::scoped_refptr<MockRtpReceiverInternal> receiver_ = MockReceiver();
rtc::scoped_refptr<MockRtpSenderInternal> sender_ = MockSender();
ChannelManagerForTest channel_manager_;
rtc::scoped_refptr<RtpTransceiver> transceiver_;
};
@ -168,7 +175,7 @@ TEST_F(RtpTransceiverUnifiedPlanTest, StopSetsDirection) {
*transceiver_->current_direction());
}
class RtpTransceiverTestForHeaderExtensions : public ::testing::Test {
class RtpTransceiverTestForHeaderExtensions : public RtpTransceiverTest {
public:
RtpTransceiverTestForHeaderExtensions()
: extensions_(
@ -192,7 +199,7 @@ class RtpTransceiverTestForHeaderExtensions : public ::testing::Test {
rtc::Thread::Current(),
rtc::Thread::Current(),
receiver_),
&channel_manager_,
channel_manager(),
extensions_,
/* on_negotiation_needed= */ [] {})) {}
@ -218,7 +225,6 @@ class RtpTransceiverTestForHeaderExtensions : public ::testing::Test {
rtc::scoped_refptr<MockRtpReceiverInternal> receiver_ = MockReceiver();
rtc::scoped_refptr<MockRtpSenderInternal> sender_ = MockSender();
ChannelManagerForTest channel_manager_;
std::vector<RtpHeaderExtensionCapability> extensions_;
rtc::scoped_refptr<RtpTransceiver> transceiver_;
};
@ -404,4 +410,6 @@ TEST_F(RtpTransceiverTestForHeaderExtensions,
"uri5", 6, RtpTransceiverDirection::kSendRecv)));
}
} // namespace
} // namespace webrtc

View File

@ -415,7 +415,14 @@ class FakePeerConnectionForStats : public FakePeerConnectionBase {
class TestChannelManager : public cricket::ChannelManager {
public:
TestChannelManager(rtc::Thread* worker, rtc::Thread* network)
: cricket::ChannelManager(nullptr, true, worker, network) {}
: cricket::ChannelManager(nullptr,
&ssrc_generator_,
true,
worker,
network) {}
private:
rtc::UniqueRandomIdGenerator ssrc_generator_;
};
rtc::Thread* const network_thread_;