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}
This commit is contained in:
Niels Moller 2021-01-12 15:54:16 +00:00 committed by Commit Bot
parent b942d45110
commit 2accc7d6e0
18 changed files with 39 additions and 33 deletions

View File

@ -508,7 +508,7 @@ ChannelReceive::ChannelReceive(
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_);

View File

@ -492,11 +492,10 @@ ChannelSend::ChannelSend(
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,

View File

@ -50,7 +50,8 @@ AudioChannel::AudioChannel(
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);

View File

@ -37,7 +37,7 @@ std::unique_ptr<ModuleRtpRtcpImpl2> CreateRtpStack(Clock* clock,
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;

View File

@ -46,7 +46,7 @@ class AudioIngressTest : public ::testing::Test {
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);

View File

@ -130,9 +130,8 @@ std::unique_ptr<ModuleRtpRtcpImpl2> CreateRtpRtcpModule(
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

View File

@ -232,7 +232,6 @@ std::vector<RtpStreamSender> CreateRtpStreamSenders(
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 @@ std::vector<RtpStreamSender> CreateRtpStreamSenders(
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);

View File

@ -136,7 +136,7 @@ class RtpRtcpRtxNackTest : public ::testing::Test {
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;

View File

@ -92,7 +92,7 @@ class RtcpSenderTest : public ::testing::Test {
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() {

View File

@ -53,7 +53,7 @@ void ModuleRtpRtcpImpl2::RtpSenderContext::AssignSequenceNumber(
}
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 @@ ModuleRtpRtcpImpl2::ModuleRtpRtcpImpl2(const Configuration& configuration)
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 @@ ModuleRtpRtcpImpl2::ModuleRtpRtcpImpl2(const Configuration& configuration)
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(const Configuration& configuration)
}
ModuleRtpRtcpImpl2::~ModuleRtpRtcpImpl2() {
if (worker_queue_) {
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

View File

@ -55,6 +55,13 @@ class ModuleRtpRtcpImpl2 final : public RtpRtcpInterface,
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;

View File

@ -161,9 +161,8 @@ class RtpRtcpModule : public RtcpPacketTypeCounterObserver {
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);
}

View File

@ -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 @@ class RtpRtcpInterface : public RtcpFeedbackSenderInterface {
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;

View File

@ -69,7 +69,7 @@ class RtpSenderAudioTest : public ::testing::Test {
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_;

View File

@ -176,7 +176,7 @@ class RtpSenderVideoTest : public ::testing::TestWithParam<bool> {
: 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 @@ class RtpSenderVideoWithFrameTransformerTest : public ::testing::Test {
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_;

View File

@ -245,7 +245,7 @@ TEST_F(BandwidthEndToEndTest, RembWithSendSideBwe) {
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);
}

View File

@ -97,8 +97,9 @@ std::unique_ptr<ModuleRtpRtcpImpl2> CreateRtpRtcpModule(
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;

View File

@ -1503,7 +1503,7 @@ TEST_F(VideoSendStreamTest, MinTransmitBitrateRespectsRemb) {
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);
}