Revert "Add task queue to RtpRtcpInterface::Configuration."
This reverts commit f23e2144e86400e2d68097345d4b3dc7a4b7f8a4.
Reason for revert: Need further discussion on appropriate thread/tq requirements.
Original change's description:
> 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}
TBR=danilchap@webrtc.org,ilnik@webrtc.org,saza@webrtc.org,nisse@webrtc.org,srte@webrtc.org
Change-Id: I7e5007f524a39a6552973ec9744cd04c13162432
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: None
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/201420
Reviewed-by: Niels Moller <nisse@webrtc.org>
Reviewed-by: Sam Zackrisson <saza@webrtc.org>
Reviewed-by: Tommi <tommi@webrtc.org>
Commit-Queue: Tommi <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32953}
diff --git a/audio/channel_receive.cc b/audio/channel_receive.cc
index e9a7f18..2788dac 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_ = std::make_unique<ModuleRtpRtcpImpl2>(configuration);
+ rtp_rtcp_ = ModuleRtpRtcpImpl2::Create(configuration);
rtp_rtcp_->SetSendingMediaStatus(false);
rtp_rtcp_->SetRemoteSSRC(remote_ssrc_);
diff --git a/audio/channel_send.cc b/audio/channel_send.cc
index 5700c13..80e7ab2 100644
--- a/audio/channel_send.cc
+++ b/audio/channel_send.cc
@@ -492,11 +492,10 @@
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_ = std::make_unique<ModuleRtpRtcpImpl2>(configuration);
+ rtp_rtcp_ = ModuleRtpRtcpImpl2::Create(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 cc1e4a4..dc53acf 100644
--- a/audio/voip/audio_channel.cc
+++ b/audio/voip/audio_channel.cc
@@ -50,7 +50,8 @@
rtp_config.rtcp_report_interval_ms = kRtcpReportIntervalMs;
rtp_config.outgoing_transport = transport;
rtp_config.local_media_ssrc = local_ssrc;
- rtp_rtcp_ = std::make_unique<ModuleRtpRtcpImpl2>(rtp_config);
+
+ rtp_rtcp_ = ModuleRtpRtcpImpl2::Create(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 e07df38..0692ef2 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 = std::make_unique<ModuleRtpRtcpImpl2>(rtp_config);
+ auto rtp_rtcp = ModuleRtpRtcpImpl2::Create(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 fa0038e..55ecfec 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_ = std::make_unique<ModuleRtpRtcpImpl2>(rtp_config);
+ rtp_rtcp_ = ModuleRtpRtcpImpl2::Create(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 3bcee60..e629bca 100644
--- a/call/flexfec_receive_stream_impl.cc
+++ b/call/flexfec_receive_stream_impl.cc
@@ -130,9 +130,8 @@
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 std::make_unique<ModuleRtpRtcpImpl2>(configuration);
+ return ModuleRtpRtcpImpl2::Create(configuration);
}
} // namespace
diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc
index 088f90e..041427a 100644
--- a/call/rtp_video_sender.cc
+++ b/call/rtp_video_sender.cc
@@ -232,7 +232,6 @@
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;
@@ -253,7 +252,8 @@
configuration.need_rtp_packet_infos = rtp_config.lntf.enabled;
- auto rtp_rtcp = std::make_unique<ModuleRtpRtcpImpl2>(configuration);
+ std::unique_ptr<ModuleRtpRtcpImpl2> rtp_rtcp(
+ ModuleRtpRtcpImpl2::Create(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 0f8d37b..8afaf3e 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_ = std::make_unique<ModuleRtpRtcpImpl2>(configuration);
+ rtp_rtcp_module_ = ModuleRtpRtcpImpl2::Create(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 d25d10d..4c8038f 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_ = std::make_unique<ModuleRtpRtcpImpl2>(configuration);
+ rtp_rtcp_impl_.reset(new 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 3d70ee8..94dc297 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_(configuration.task_queue),
+ : worker_queue_(TaskQueueBase::Current()),
rtcp_sender_(configuration),
rtcp_receiver_(configuration, this),
clock_(configuration.clock),
@@ -66,6 +66,7 @@
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);
@@ -81,7 +82,6 @@
SetMaxRtpPacketSize(IP_PACKET_SIZE - kTcpOverIpv4HeaderSize);
if (rtt_stats_) {
- RTC_DCHECK(worker_queue_);
rtt_update_task_ = RepeatingTaskHandle::DelayedStart(
worker_queue_, kRttUpdateInterval, [this]() {
PeriodicUpdate();
@@ -91,10 +91,16 @@
}
ModuleRtpRtcpImpl2::~ModuleRtpRtcpImpl2() {
- if (worker_queue_) {
- RTC_DCHECK_RUN_ON(worker_queue_);
- rtt_update_task_.Stop();
- }
+ 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);
}
// 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 6cdd149..9431e75 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl2.h
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2.h
@@ -55,6 +55,13 @@
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 e107ebb..3b66642 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc
@@ -161,9 +161,8 @@
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_ = std::make_unique<ModuleRtpRtcpImpl2>(config);
+ impl_.reset(new 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 30b66ef..5bb3eb5 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_interface.h
+++ b/modules/rtp_rtcp/source/rtp_rtcp_interface.h
@@ -18,7 +18,6 @@
#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"
@@ -87,11 +86,6 @@
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 07849fa..d75f4e8 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_(std::make_unique<ModuleRtpRtcpImpl2>([&] {
+ rtp_module_(ModuleRtpRtcpImpl2::Create([&] {
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 9feacf0..55bafdc 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_(std::make_unique<ModuleRtpRtcpImpl2>([&] {
+ rtp_module_(ModuleRtpRtcpImpl2::Create([&] {
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_(std::make_unique<ModuleRtpRtcpImpl2>([&] {
+ rtp_module_(ModuleRtpRtcpImpl2::Create([&] {
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 ac2fd57..7217383 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_ = std::make_unique<ModuleRtpRtcpImpl2>(config);
+ rtp_rtcp_ = ModuleRtpRtcpImpl2::Create(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 2cda8c7..63d8c38 100644
--- a/video/rtp_video_stream_receiver2.cc
+++ b/video/rtp_video_stream_receiver2.cc
@@ -97,8 +97,9 @@
configuration.rtcp_cname_callback = rtcp_cname_callback;
configuration.local_media_ssrc = local_ssrc;
configuration.non_sender_rtt_measurement = non_sender_rtt_measurement;
- configuration.task_queue = TaskQueueBase::Current();
- auto rtp_rtcp = std::make_unique<ModuleRtpRtcpImpl2>(configuration);
+
+ std::unique_ptr<ModuleRtpRtcpImpl2> rtp_rtcp =
+ ModuleRtpRtcpImpl2::Create(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 7346374..52e4ddb 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_ = std::make_unique<ModuleRtpRtcpImpl2>(config);
+ rtp_rtcp_ = ModuleRtpRtcpImpl2::Create(config);
rtp_rtcp_->SetRTCPStatus(RtcpMode::kReducedSize);
}
