Don't inject worker queue into send streams.
This prepares for making AudioSendStream use its own task queue. In the
future more of the functionality that depends on running on the task
queue is planned to be moved directly into RtpTransportControllerSend.
They should instead get it from the transport controller. This affects
the media transport tests which previously assumed that the transport
controller could be missing. However, this is not something that is used
in production, so this is an improvement of the tests as they will
behave more like production code.
Bug: webrtc:9883
Change-Id: Ie32f4c2f6433ec37ac16a08d531ceb690ea9c0b5
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/126000
Reviewed-by: Oskar Sundbom <ossu@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27010}
diff --git a/audio/BUILD.gn b/audio/BUILD.gn
index 3a1d123..9e0bb9f 100644
--- a/audio/BUILD.gn
+++ b/audio/BUILD.gn
@@ -47,6 +47,7 @@
"../api/audio:audio_frame_api",
"../api/audio:audio_mixer_api",
"../api/audio_codecs:audio_codecs_api",
+ "../api/task_queue",
"../call:bitrate_allocator",
"../call:call_interfaces",
"../call:rtp_interfaces",
@@ -128,12 +129,14 @@
"../api/audio_codecs:audio_codecs_api",
"../api/audio_codecs/opus:audio_decoder_opus",
"../api/audio_codecs/opus:audio_encoder_opus",
+ "../api/task_queue:global_task_queue_factory",
"../api/units:time_delta",
"../call:mock_bitrate_allocator",
"../call:mock_call_interfaces",
"../call:mock_rtp_interfaces",
"../call:rtp_interfaces",
"../call:rtp_receiver",
+ "../call:rtp_sender",
"../common_audio",
"../logging:mocks",
"../logging:rtc_event_log_api",
diff --git a/audio/audio_send_stream.cc b/audio/audio_send_stream.cc
index 280c915..68819a7 100644
--- a/audio/audio_send_stream.cc
+++ b/audio/audio_send_stream.cc
@@ -85,7 +85,6 @@
Clock* clock,
const webrtc::AudioSendStream::Config& config,
const rtc::scoped_refptr<webrtc::AudioState>& audio_state,
- rtc::TaskQueue* worker_queue,
ProcessThread* module_process_thread,
RtpTransportControllerSendInterface* rtp_transport,
BitrateAllocatorInterface* bitrate_allocator,
@@ -95,14 +94,13 @@
: AudioSendStream(clock,
config,
audio_state,
- worker_queue,
rtp_transport,
bitrate_allocator,
event_log,
rtcp_rtt_stats,
suspended_rtp_state,
voe::CreateChannelSend(clock,
- worker_queue,
+ rtp_transport->GetWorkerQueue(),
module_process_thread,
config.media_transport,
/*overhead_observer=*/this,
@@ -118,7 +116,6 @@
Clock* clock,
const webrtc::AudioSendStream::Config& config,
const rtc::scoped_refptr<webrtc::AudioState>& audio_state,
- rtc::TaskQueue* worker_queue,
RtpTransportControllerSendInterface* rtp_transport,
BitrateAllocatorInterface* bitrate_allocator,
RtcEventLog* event_log,
@@ -126,7 +123,7 @@
const absl::optional<RtpState>& suspended_rtp_state,
std::unique_ptr<voe::ChannelSendInterface> channel_send)
: clock_(clock),
- worker_queue_(worker_queue),
+ worker_queue_(rtp_transport->GetWorkerQueue()),
config_(Config(/*send_transport=*/nullptr,
/*media_transport=*/nullptr)),
audio_state_(audio_state),
@@ -144,6 +141,9 @@
RTC_DCHECK(audio_state_);
RTC_DCHECK(channel_send_);
RTC_DCHECK(bitrate_allocator_);
+ // Currently we require the rtp transport even when media transport is used.
+ RTC_DCHECK(rtp_transport);
+
// TODO(nisse): Eventually, we should have only media_transport. But for the
// time being, we can have either. When media transport is injected, there
// should be no rtp_transport, and below check should be strengthened to XOR
diff --git a/audio/audio_send_stream.h b/audio/audio_send_stream.h
index afe9321..0d35562 100644
--- a/audio/audio_send_stream.h
+++ b/audio/audio_send_stream.h
@@ -42,7 +42,6 @@
AudioSendStream(Clock* clock,
const webrtc::AudioSendStream::Config& config,
const rtc::scoped_refptr<webrtc::AudioState>& audio_state,
- rtc::TaskQueue* worker_queue,
ProcessThread* module_process_thread,
RtpTransportControllerSendInterface* rtp_transport,
BitrateAllocatorInterface* bitrate_allocator,
@@ -53,7 +52,6 @@
AudioSendStream(Clock* clock,
const webrtc::AudioSendStream::Config& config,
const rtc::scoped_refptr<webrtc::AudioState>& audio_state,
- rtc::TaskQueue* worker_queue,
RtpTransportControllerSendInterface* rtp_transport,
BitrateAllocatorInterface* bitrate_allocator,
RtcEventLog* event_log,
diff --git a/audio/audio_send_stream_unittest.cc b/audio/audio_send_stream_unittest.cc
index df5ee30..80d5ad4 100644
--- a/audio/audio_send_stream_unittest.cc
+++ b/audio/audio_send_stream_unittest.cc
@@ -13,6 +13,7 @@
#include <vector>
#include "absl/memory/memory.h"
+#include "api/task_queue/global_task_queue_factory.h"
#include "api/test/mock_frame_encryptor.h"
#include "audio/audio_send_stream.h"
#include "audio/audio_state.h"
@@ -161,11 +162,13 @@
}
std::unique_ptr<internal::AudioSendStream> CreateAudioSendStream() {
+ EXPECT_CALL(rtp_transport_, GetWorkerQueue())
+ .WillRepeatedly(Return(&worker_queue_));
return std::unique_ptr<internal::AudioSendStream>(
new internal::AudioSendStream(
Clock::GetRealTimeClock(), stream_config_, audio_state_,
- &worker_queue_, &rtp_transport_, &bitrate_allocator_, &event_log_,
- &rtcp_rtt_stats_, absl::nullopt,
+ &rtp_transport_, &bitrate_allocator_, &event_log_, &rtcp_rtt_stats_,
+ absl::nullopt,
std::unique_ptr<voe::ChannelSendInterface>(channel_send_)));
}
diff --git a/audio/test/media_transport_test.cc b/audio/test/media_transport_test.cc
index 02e3f75..71a3fb8 100644
--- a/audio/test/media_transport_test.cc
+++ b/audio/test/media_transport_test.cc
@@ -13,10 +13,12 @@
#include "api/audio_codecs/audio_encoder_factory_template.h"
#include "api/audio_codecs/opus/audio_decoder_opus.h"
#include "api/audio_codecs/opus/audio_encoder_opus.h"
+#include "api/task_queue/global_task_queue_factory.h"
#include "api/test/loopback_media_transport.h"
#include "api/test/mock_audio_mixer.h"
#include "audio/audio_receive_stream.h"
#include "audio/audio_send_stream.h"
+#include "call/rtp_transport_controller_send.h"
#include "call/test/mock_bitrate_allocator.h"
#include "logging/rtc_event_log/rtc_event_log.h"
#include "modules/audio_device/include/test_audio_device.h"
@@ -32,6 +34,10 @@
namespace test {
namespace {
+using testing::_;
+using testing::NiceMock;
+using testing::Return;
+
constexpr int kPayloadTypeOpus = 17;
constexpr int kSamplingFrequency = 48000;
constexpr int kNumChannels = 2;
@@ -69,10 +75,10 @@
std::unique_ptr<rtc::Thread> transport_thread = rtc::Thread::Create();
transport_thread->Start();
MediaTransportPair transport_pair(transport_thread.get());
- MockTransport rtcp_send_transport;
- MockTransport send_transport;
+ NiceMock<MockTransport> rtcp_send_transport;
+ NiceMock<MockTransport> send_transport;
std::unique_ptr<RtcEventLog> null_event_log = RtcEventLog::CreateNull();
- MockBitrateAllocator bitrate_allocator;
+ NiceMock<MockBitrateAllocator> bitrate_allocator;
rtc::scoped_refptr<TestAudioDeviceModule> audio_device =
TestAudioDeviceModule::CreateTestAudioDeviceModule(
@@ -117,13 +123,16 @@
send_config.send_codec_spec =
AudioSendStream::Config::SendCodecSpec(kPayloadTypeOpus, audio_format);
send_config.encoder_factory = CreateAudioEncoderFactory<AudioEncoderOpus>();
- rtc::TaskQueue send_tq("audio send queue");
std::unique_ptr<ProcessThread> send_process_thread =
ProcessThread::Create("audio send thread");
+ RtpTransportControllerSend rtp_transport(
+ Clock::GetRealTimeClock(), null_event_log.get(), nullptr,
+ BitrateConstraints(), ProcessThread::Create("Pacer"),
+ &GlobalTaskQueueFactory());
webrtc::internal::AudioSendStream send_stream(
- Clock::GetRealTimeClock(), send_config, audio_state, &send_tq,
- send_process_thread.get(),
- /*transport=*/nullptr, &bitrate_allocator, null_event_log.get(),
+ Clock::GetRealTimeClock(), send_config, audio_state,
+ send_process_thread.get(), &rtp_transport, &bitrate_allocator,
+ null_event_log.get(),
/*rtcp_rtt_stats=*/nullptr, absl::optional<RtpState>());
audio_device->Init(); // Starts thread.
diff --git a/call/call.cc b/call/call.cc
index 08fdcd2..3000244 100644
--- a/call/call.cc
+++ b/call/call.cc
@@ -666,12 +666,8 @@
}
}
- // TODO(srte): AudioSendStream should call GetWorkerQueue directly rather than
- // having it injected.
-
AudioSendStream* send_stream = new AudioSendStream(
- clock_, config, config_.audio_state,
- transport_send_ptr_->GetWorkerQueue(), module_process_thread_.get(),
+ clock_, config, config_.audio_state, module_process_thread_.get(),
transport_send_ptr_, bitrate_allocator_.get(), event_log_,
call_stats_.get(), suspended_rtp_state);
{
@@ -802,11 +798,8 @@
// Copy ssrcs from |config| since |config| is moved.
std::vector<uint32_t> ssrcs = config.rtp.ssrcs;
- // TODO(srte): VideoSendStream should call GetWorkerQueue directly rather than
- // having it injected.
VideoSendStream* send_stream = new VideoSendStream(
- clock_, num_cpu_cores_, module_process_thread_.get(),
- transport_send_ptr_->GetWorkerQueue(), task_queue_factory_,
+ clock_, num_cpu_cores_, module_process_thread_.get(), task_queue_factory_,
call_stats_.get(), transport_send_ptr_, bitrate_allocator_.get(),
video_send_delay_stats_.get(), event_log_, std::move(config),
std::move(encoder_config), suspended_video_send_ssrcs_,
diff --git a/video/video_send_stream.cc b/video/video_send_stream.cc
index 3b6c4f5..edf3922 100644
--- a/video/video_send_stream.cc
+++ b/video/video_send_stream.cc
@@ -69,7 +69,6 @@
Clock* clock,
int num_cpu_cores,
ProcessThread* module_process_thread,
- rtc::TaskQueue* worker_queue,
TaskQueueFactory* task_queue_factory,
CallStats* call_stats,
RtpTransportControllerSendInterface* transport,
@@ -81,7 +80,7 @@
const std::map<uint32_t, RtpState>& suspended_ssrcs,
const std::map<uint32_t, RtpPayloadState>& suspended_payload_states,
std::unique_ptr<FecController> fec_controller)
- : worker_queue_(worker_queue),
+ : worker_queue_(transport->GetWorkerQueue()),
stats_proxy_(clock, config, encoder_config.content_type),
config_(std::move(config)),
content_type_(encoder_config.content_type) {
diff --git a/video/video_send_stream.h b/video/video_send_stream.h
index f1ac477..827f991 100644
--- a/video/video_send_stream.h
+++ b/video/video_send_stream.h
@@ -56,7 +56,6 @@
Clock* clock,
int num_cpu_cores,
ProcessThread* module_process_thread,
- rtc::TaskQueue* worker_queue,
TaskQueueFactory* task_queue_factory,
CallStats* call_stats,
RtpTransportControllerSendInterface* transport,