Add task queue to RtpRtcpInterface::Configuration.
Let ModuleRtpRtcpImpl2 use the configured value instead of
TaskQueueBase::Current().
Intention is to allow construction of RtpRtcpImpl2 on any thread.
If a task queue is provided (required for periodic rtt updates), the
destruction of the object must be done on that same task queue.
Also, delete ModuleRtpRtcpImpl2::Create, callers updated to use std::make_unique.
Bug: None
Change-Id: I412b7b1e1ce24722ffd23d16aa6c48a7214c9bcd
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/199968
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Reviewed-by: Sam Zackrisson <saza@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32949}
diff --git a/audio/channel_receive.cc b/audio/channel_receive.cc
index 2788dac..e9a7f18 100644
--- a/audio/channel_receive.cc
+++ b/audio/channel_receive.cc
@@ -508,7 +508,7 @@
if (frame_transformer)
InitFrameTransformerDelegate(std::move(frame_transformer));
- rtp_rtcp_ = ModuleRtpRtcpImpl2::Create(configuration);
+ rtp_rtcp_ = std::make_unique<ModuleRtpRtcpImpl2>(configuration);
rtp_rtcp_->SetSendingMediaStatus(false);
rtp_rtcp_->SetRemoteSSRC(remote_ssrc_);
diff --git a/audio/channel_send.cc b/audio/channel_send.cc
index 80e7ab2..5700c13 100644
--- a/audio/channel_send.cc
+++ b/audio/channel_send.cc
@@ -492,10 +492,11 @@
retransmission_rate_limiter_.get();
configuration.extmap_allow_mixed = extmap_allow_mixed;
configuration.rtcp_report_interval_ms = rtcp_report_interval_ms;
+ configuration.task_queue = TaskQueueBase::Current();
configuration.local_media_ssrc = ssrc;
- rtp_rtcp_ = ModuleRtpRtcpImpl2::Create(configuration);
+ rtp_rtcp_ = std::make_unique<ModuleRtpRtcpImpl2>(configuration);
rtp_rtcp_->SetSendingMediaStatus(false);
rtp_sender_audio_ = std::make_unique<RTPSenderAudio>(configuration.clock,
diff --git a/audio/voip/audio_channel.cc b/audio/voip/audio_channel.cc
index dc53acf..cc1e4a4 100644
--- a/audio/voip/audio_channel.cc
+++ b/audio/voip/audio_channel.cc
@@ -50,8 +50,7 @@
rtp_config.rtcp_report_interval_ms = kRtcpReportIntervalMs;
rtp_config.outgoing_transport = transport;
rtp_config.local_media_ssrc = local_ssrc;
-
- rtp_rtcp_ = ModuleRtpRtcpImpl2::Create(rtp_config);
+ rtp_rtcp_ = std::make_unique<ModuleRtpRtcpImpl2>(rtp_config);
rtp_rtcp_->SetSendingMediaStatus(false);
rtp_rtcp_->SetRTCPStatus(RtcpMode::kCompound);
diff --git a/audio/voip/test/audio_egress_unittest.cc b/audio/voip/test/audio_egress_unittest.cc
index 0692ef2..e07df38 100644
--- a/audio/voip/test/audio_egress_unittest.cc
+++ b/audio/voip/test/audio_egress_unittest.cc
@@ -37,7 +37,7 @@
rtp_config.rtcp_report_interval_ms = 5000;
rtp_config.outgoing_transport = transport;
rtp_config.local_media_ssrc = remote_ssrc;
- auto rtp_rtcp = ModuleRtpRtcpImpl2::Create(rtp_config);
+ auto rtp_rtcp = std::make_unique<ModuleRtpRtcpImpl2>(rtp_config);
rtp_rtcp->SetSendingMediaStatus(false);
rtp_rtcp->SetRTCPStatus(RtcpMode::kCompound);
return rtp_rtcp;
diff --git a/audio/voip/test/audio_ingress_unittest.cc b/audio/voip/test/audio_ingress_unittest.cc
index 55ecfec..fa0038e 100644
--- a/audio/voip/test/audio_ingress_unittest.cc
+++ b/audio/voip/test/audio_ingress_unittest.cc
@@ -46,7 +46,7 @@
rtp_config.rtcp_report_interval_ms = 5000;
rtp_config.outgoing_transport = &transport_;
rtp_config.local_media_ssrc = 0xdeadc0de;
- rtp_rtcp_ = ModuleRtpRtcpImpl2::Create(rtp_config);
+ rtp_rtcp_ = std::make_unique<ModuleRtpRtcpImpl2>(rtp_config);
rtp_rtcp_->SetSendingMediaStatus(false);
rtp_rtcp_->SetRTCPStatus(RtcpMode::kCompound);
diff --git a/call/flexfec_receive_stream_impl.cc b/call/flexfec_receive_stream_impl.cc
index e629bca..3bcee60 100644
--- a/call/flexfec_receive_stream_impl.cc
+++ b/call/flexfec_receive_stream_impl.cc
@@ -130,8 +130,9 @@
configuration.receive_statistics = receive_statistics;
configuration.outgoing_transport = config.rtcp_send_transport;
configuration.rtt_stats = rtt_stats;
+ configuration.task_queue = TaskQueueBase::Current();
configuration.local_media_ssrc = config.local_ssrc;
- return ModuleRtpRtcpImpl2::Create(configuration);
+ return std::make_unique<ModuleRtpRtcpImpl2>(configuration);
}
} // namespace
diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc
index 041427a..088f90e 100644
--- a/call/rtp_video_sender.cc
+++ b/call/rtp_video_sender.cc
@@ -232,6 +232,7 @@
configuration.extmap_allow_mixed = rtp_config.extmap_allow_mixed;
configuration.rtcp_report_interval_ms = rtcp_report_interval_ms;
configuration.field_trials = &trials;
+ configuration.task_queue = TaskQueueBase::Current();
std::vector<RtpStreamSender> rtp_streams;
@@ -252,8 +253,7 @@
configuration.need_rtp_packet_infos = rtp_config.lntf.enabled;
- std::unique_ptr<ModuleRtpRtcpImpl2> rtp_rtcp(
- ModuleRtpRtcpImpl2::Create(configuration));
+ auto rtp_rtcp = std::make_unique<ModuleRtpRtcpImpl2>(configuration);
rtp_rtcp->SetSendingStatus(false);
rtp_rtcp->SetSendingMediaStatus(false);
rtp_rtcp->SetRTCPStatus(RtcpMode::kCompound);
diff --git a/modules/rtp_rtcp/source/nack_rtx_unittest.cc b/modules/rtp_rtcp/source/nack_rtx_unittest.cc
index 8afaf3e..0f8d37b 100644
--- a/modules/rtp_rtcp/source/nack_rtx_unittest.cc
+++ b/modules/rtp_rtcp/source/nack_rtx_unittest.cc
@@ -136,7 +136,7 @@
configuration.retransmission_rate_limiter = &retransmission_rate_limiter_;
configuration.local_media_ssrc = kTestSsrc;
configuration.rtx_send_ssrc = kTestRtxSsrc;
- rtp_rtcp_module_ = ModuleRtpRtcpImpl2::Create(configuration);
+ rtp_rtcp_module_ = std::make_unique<ModuleRtpRtcpImpl2>(configuration);
FieldTrialBasedConfig field_trials;
RTPSenderVideo::Config video_config;
video_config.clock = &fake_clock;
diff --git a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc
index 4c8038f..d25d10d 100644
--- a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc
+++ b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc
@@ -92,7 +92,7 @@
receive_statistics_(ReceiveStatistics::Create(&clock_)),
retransmission_rate_limiter_(&clock_, 1000) {
RtpRtcpInterface::Configuration configuration = GetDefaultConfig();
- rtp_rtcp_impl_.reset(new ModuleRtpRtcpImpl2(configuration));
+ rtp_rtcp_impl_ = std::make_unique<ModuleRtpRtcpImpl2>(configuration);
}
RtpRtcpInterface::Configuration GetDefaultConfig() {
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc
index 94dc297..3d70ee8 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc
@@ -53,7 +53,7 @@
}
ModuleRtpRtcpImpl2::ModuleRtpRtcpImpl2(const Configuration& configuration)
- : worker_queue_(TaskQueueBase::Current()),
+ : worker_queue_(configuration.task_queue),
rtcp_sender_(configuration),
rtcp_receiver_(configuration, this),
clock_(configuration.clock),
@@ -66,7 +66,6 @@
remote_bitrate_(configuration.remote_bitrate_estimator),
rtt_stats_(configuration.rtt_stats),
rtt_ms_(0) {
- RTC_DCHECK(worker_queue_);
process_thread_checker_.Detach();
if (!configuration.receiver_only) {
rtp_sender_ = std::make_unique<RtpSenderContext>(configuration);
@@ -82,6 +81,7 @@
SetMaxRtpPacketSize(IP_PACKET_SIZE - kTcpOverIpv4HeaderSize);
if (rtt_stats_) {
+ RTC_DCHECK(worker_queue_);
rtt_update_task_ = RepeatingTaskHandle::DelayedStart(
worker_queue_, kRttUpdateInterval, [this]() {
PeriodicUpdate();
@@ -91,16 +91,10 @@
}
ModuleRtpRtcpImpl2::~ModuleRtpRtcpImpl2() {
- RTC_DCHECK_RUN_ON(worker_queue_);
- rtt_update_task_.Stop();
-}
-
-// static
-std::unique_ptr<ModuleRtpRtcpImpl2> ModuleRtpRtcpImpl2::Create(
- const Configuration& configuration) {
- RTC_DCHECK(configuration.clock);
- RTC_DCHECK(TaskQueueBase::Current());
- return std::make_unique<ModuleRtpRtcpImpl2>(configuration);
+ if (worker_queue_) {
+ RTC_DCHECK_RUN_ON(worker_queue_);
+ rtt_update_task_.Stop();
+ }
}
// Returns the number of milliseconds until the module want a worker thread
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2.h b/modules/rtp_rtcp/source/rtp_rtcp_impl2.h
index 9431e75..6cdd149 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl2.h
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2.h
@@ -55,13 +55,6 @@
const RtpRtcpInterface::Configuration& configuration);
~ModuleRtpRtcpImpl2() override;
- // This method is provided to easy with migrating away from the
- // RtpRtcp::Create factory method. Since this is an internal implementation
- // detail though, creating an instance of ModuleRtpRtcpImpl2 directly should
- // be fine.
- static std::unique_ptr<ModuleRtpRtcpImpl2> Create(
- const Configuration& configuration);
-
// Returns the number of milliseconds until the module want a worker thread to
// call Process.
int64_t TimeUntilNextProcess() override;
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc
index 3b66642..e107ebb 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc
@@ -161,8 +161,9 @@
config.local_media_ssrc = is_sender_ ? kSenderSsrc : kReceiverSsrc;
config.need_rtp_packet_infos = true;
config.non_sender_rtt_measurement = true;
+ config.task_queue = TaskQueueBase::Current();
- impl_.reset(new ModuleRtpRtcpImpl2(config));
+ impl_ = std::make_unique<ModuleRtpRtcpImpl2>(config);
impl_->SetRemoteSSRC(is_sender_ ? kReceiverSsrc : kSenderSsrc);
impl_->SetRTCPStatus(RtcpMode::kCompound);
}
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_interface.h b/modules/rtp_rtcp/source/rtp_rtcp_interface.h
index 5bb3eb5..30b66ef 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_interface.h
+++ b/modules/rtp_rtcp/source/rtp_rtcp_interface.h
@@ -18,6 +18,7 @@
#include "absl/types/optional.h"
#include "api/frame_transformer_interface.h"
#include "api/scoped_refptr.h"
+#include "api/task_queue/task_queue_base.h"
#include "api/transport/webrtc_key_value_config.h"
#include "api/video/video_bitrate_allocation.h"
#include "modules/rtp_rtcp/include/receive_statistics.h"
@@ -86,6 +87,11 @@
RtcpCnameCallback* rtcp_cname_callback = nullptr;
ReportBlockDataObserver* report_block_data_observer = nullptr;
+ // For RtpRtcpImpl2 only. Used for periodic RTT updates, when rtt_stats
+ // (above) is non-null. If provided, destruction of the RtpRtcp object must
+ // occur on this task queue, while construction is allowed on any thread.
+ TaskQueueBase* task_queue = nullptr;
+
// Estimates the bandwidth available for a set of streams from the same
// client.
RemoteBitrateEstimator* remote_bitrate_estimator = nullptr;
diff --git a/modules/rtp_rtcp/source/rtp_sender_audio_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_audio_unittest.cc
index d75f4e8..07849fa 100644
--- a/modules/rtp_rtcp/source/rtp_sender_audio_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_audio_unittest.cc
@@ -69,7 +69,7 @@
public:
RtpSenderAudioTest()
: fake_clock_(kStartTime),
- rtp_module_(ModuleRtpRtcpImpl2::Create([&] {
+ rtp_module_(std::make_unique<ModuleRtpRtcpImpl2>([&] {
RtpRtcpInterface::Configuration config;
config.audio = true;
config.clock = &fake_clock_;
diff --git a/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc
index 55bafdc..9feacf0 100644
--- a/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc
@@ -176,7 +176,7 @@
: field_trials_(GetParam()),
fake_clock_(kStartTime),
retransmission_rate_limiter_(&fake_clock_, 1000),
- rtp_module_(ModuleRtpRtcpImpl2::Create([&] {
+ rtp_module_(std::make_unique<ModuleRtpRtcpImpl2>([&] {
RtpRtcpInterface::Configuration config;
config.clock = &fake_clock_;
config.outgoing_transport = &transport_;
@@ -1167,7 +1167,7 @@
RtpSenderVideoWithFrameTransformerTest()
: fake_clock_(kStartTime),
retransmission_rate_limiter_(&fake_clock_, 1000),
- rtp_module_(ModuleRtpRtcpImpl2::Create([&] {
+ rtp_module_(std::make_unique<ModuleRtpRtcpImpl2>([&] {
RtpRtcpInterface::Configuration config;
config.clock = &fake_clock_;
config.outgoing_transport = &transport_;
diff --git a/video/end_to_end_tests/bandwidth_tests.cc b/video/end_to_end_tests/bandwidth_tests.cc
index 7217383..ac2fd57 100644
--- a/video/end_to_end_tests/bandwidth_tests.cc
+++ b/video/end_to_end_tests/bandwidth_tests.cc
@@ -245,7 +245,7 @@
config.outgoing_transport = receive_transport_;
config.retransmission_rate_limiter = &retransmission_rate_limiter_;
config.local_media_ssrc = (*receive_configs)[0].rtp.local_ssrc;
- rtp_rtcp_ = ModuleRtpRtcpImpl2::Create(config);
+ rtp_rtcp_ = std::make_unique<ModuleRtpRtcpImpl2>(config);
rtp_rtcp_->SetRemoteSSRC((*receive_configs)[0].rtp.remote_ssrc);
rtp_rtcp_->SetRTCPStatus(RtcpMode::kReducedSize);
}
diff --git a/video/rtp_video_stream_receiver2.cc b/video/rtp_video_stream_receiver2.cc
index 63d8c38..2cda8c7 100644
--- a/video/rtp_video_stream_receiver2.cc
+++ b/video/rtp_video_stream_receiver2.cc
@@ -97,9 +97,8 @@
configuration.rtcp_cname_callback = rtcp_cname_callback;
configuration.local_media_ssrc = local_ssrc;
configuration.non_sender_rtt_measurement = non_sender_rtt_measurement;
-
- std::unique_ptr<ModuleRtpRtcpImpl2> rtp_rtcp =
- ModuleRtpRtcpImpl2::Create(configuration);
+ configuration.task_queue = TaskQueueBase::Current();
+ auto rtp_rtcp = std::make_unique<ModuleRtpRtcpImpl2>(configuration);
rtp_rtcp->SetRTCPStatus(RtcpMode::kCompound);
return rtp_rtcp;
diff --git a/video/video_send_stream_tests.cc b/video/video_send_stream_tests.cc
index 52e4ddb..7346374 100644
--- a/video/video_send_stream_tests.cc
+++ b/video/video_send_stream_tests.cc
@@ -1503,7 +1503,7 @@
config.clock = Clock::GetRealTimeClock();
config.outgoing_transport = feedback_transport_.get();
config.retransmission_rate_limiter = &retranmission_rate_limiter_;
- rtp_rtcp_ = ModuleRtpRtcpImpl2::Create(config);
+ rtp_rtcp_ = std::make_unique<ModuleRtpRtcpImpl2>(config);
rtp_rtcp_->SetRTCPStatus(RtcpMode::kReducedSize);
}
