Use SingleThreadedTaskQueue in DirectTransport
DirectTransport has so far used its own thread, which led to a different threading-model for in the unit-tests than is used in actual WebRTC. Because of that, some critical-sections that weren't truly necessary in WebRTC could not be replaced with thread-checks, because those checks failed in unit-tests.
This CL introduces SingleThreadedTaskQueue - a TaskQueue which guarantees to run all of its tasks on the same thread (rtc::TaskQueue doesn't guarantee that on Mac) - and uses that for DirectTransport. CLs based on top of this will uncomment thread-checks which had to be commented out before, and remove unnecessary critical-sections.
Future work would probably replace the thread-checkers by more sophisticated serialized-access checks, allowing us to move from the SingleThreadedTaskQueue to a normal TaskQueue.
Related implementation notes:
* This CL has made DirectTransport::StopSending() superfluous, and so it was deleted.
BUG=webrtc:8113, webrtc:7405, webrtc:8056, webrtc:8116
Review-Url: https://codereview.webrtc.org/2998923002
Cr-Commit-Position: refs/heads/master@{#19445}
diff --git a/webrtc/test/call_test.cc b/webrtc/test/call_test.cc
index 51816d8..3efc022 100644
--- a/webrtc/test/call_test.cc
+++ b/webrtc/test/call_test.cc
@@ -18,8 +18,11 @@
#include "webrtc/config.h"
#include "webrtc/modules/audio_mixer/audio_mixer_impl.h"
#include "webrtc/rtc_base/checks.h"
+#include "webrtc/rtc_base/event.h"
+#include "webrtc/rtc_base/ptr_util.h"
#include "webrtc/test/testsupport/fileutils.h"
#include "webrtc/voice_engine/include/voe_base.h"
+
namespace webrtc {
namespace test {
@@ -41,112 +44,124 @@
num_flexfec_streams_(0),
decoder_factory_(CreateBuiltinAudioDecoderFactory()),
encoder_factory_(CreateBuiltinAudioEncoderFactory()),
+ task_queue_("CallTestTaskQueue"),
fake_send_audio_device_(nullptr),
fake_recv_audio_device_(nullptr) {}
CallTest::~CallTest() {
+ task_queue_.SendTask([this]() {
+ fake_send_audio_device_.reset();
+ fake_recv_audio_device_.reset();
+ frame_generator_capturer_.reset();
+ });
}
void CallTest::RunBaseTest(BaseTest* test) {
- num_video_streams_ = test->GetNumVideoStreams();
- num_audio_streams_ = test->GetNumAudioStreams();
- num_flexfec_streams_ = test->GetNumFlexfecStreams();
- RTC_DCHECK(num_video_streams_ > 0 || num_audio_streams_ > 0);
- Call::Config send_config(test->GetSenderCallConfig());
- if (num_audio_streams_ > 0) {
- CreateFakeAudioDevices(test->CreateCapturer(), test->CreateRenderer());
- test->OnFakeAudioDevicesCreated(fake_send_audio_device_.get(),
- fake_recv_audio_device_.get());
- apm_send_ = AudioProcessing::Create();
- apm_recv_ = AudioProcessing::Create();
- CreateVoiceEngines();
- AudioState::Config audio_state_config;
- audio_state_config.voice_engine = voe_send_.voice_engine;
- audio_state_config.audio_mixer = AudioMixerImpl::Create();
- audio_state_config.audio_processing = apm_send_;
- send_config.audio_state = AudioState::Create(audio_state_config);
- }
- CreateSenderCall(send_config);
- if (sender_call_transport_controller_ != nullptr) {
- test->OnRtpTransportControllerSendCreated(
- sender_call_transport_controller_);
- }
- if (test->ShouldCreateReceivers()) {
- Call::Config recv_config(test->GetReceiverCallConfig());
+ task_queue_.SendTask([this, test]() {
+ num_video_streams_ = test->GetNumVideoStreams();
+ num_audio_streams_ = test->GetNumAudioStreams();
+ num_flexfec_streams_ = test->GetNumFlexfecStreams();
+ RTC_DCHECK(num_video_streams_ > 0 || num_audio_streams_ > 0);
+ Call::Config send_config(test->GetSenderCallConfig());
if (num_audio_streams_ > 0) {
+ CreateFakeAudioDevices(test->CreateCapturer(), test->CreateRenderer());
+ test->OnFakeAudioDevicesCreated(fake_send_audio_device_.get(),
+ fake_recv_audio_device_.get());
+ apm_send_ = AudioProcessing::Create();
+ apm_recv_ = AudioProcessing::Create();
+ CreateVoiceEngines();
AudioState::Config audio_state_config;
- audio_state_config.voice_engine = voe_recv_.voice_engine;
+ audio_state_config.voice_engine = voe_send_.voice_engine;
audio_state_config.audio_mixer = AudioMixerImpl::Create();
- audio_state_config.audio_processing = apm_recv_;
- recv_config.audio_state = AudioState::Create(audio_state_config);
+ audio_state_config.audio_processing = apm_send_;
+ send_config.audio_state = AudioState::Create(audio_state_config);
}
- CreateReceiverCall(recv_config);
- }
- test->OnCallsCreated(sender_call_.get(), receiver_call_.get());
- receive_transport_.reset(test->CreateReceiveTransport());
- send_transport_.reset(test->CreateSendTransport(sender_call_.get()));
+ CreateSenderCall(send_config);
+ if (sender_call_transport_controller_ != nullptr) {
+ test->OnRtpTransportControllerSendCreated(
+ sender_call_transport_controller_);
+ }
+ if (test->ShouldCreateReceivers()) {
+ Call::Config recv_config(test->GetReceiverCallConfig());
+ if (num_audio_streams_ > 0) {
+ AudioState::Config audio_state_config;
+ audio_state_config.voice_engine = voe_recv_.voice_engine;
+ audio_state_config.audio_mixer = AudioMixerImpl::Create();
+ audio_state_config.audio_processing = apm_recv_;
+ recv_config.audio_state = AudioState::Create(audio_state_config);
+ }
+ CreateReceiverCall(recv_config);
+ }
+ test->OnCallsCreated(sender_call_.get(), receiver_call_.get());
+ receive_transport_.reset(test->CreateReceiveTransport(&task_queue_));
+ send_transport_.reset(
+ test->CreateSendTransport(&task_queue_, sender_call_.get()));
- if (test->ShouldCreateReceivers()) {
- send_transport_->SetReceiver(receiver_call_->Receiver());
- receive_transport_->SetReceiver(sender_call_->Receiver());
- if (num_video_streams_ > 0)
- receiver_call_->SignalChannelNetworkState(MediaType::VIDEO, kNetworkUp);
- if (num_audio_streams_ > 0)
- receiver_call_->SignalChannelNetworkState(MediaType::AUDIO, kNetworkUp);
- } else {
- // Sender-only call delivers to itself.
- send_transport_->SetReceiver(sender_call_->Receiver());
- receive_transport_->SetReceiver(nullptr);
- }
+ if (test->ShouldCreateReceivers()) {
+ send_transport_->SetReceiver(receiver_call_->Receiver());
+ receive_transport_->SetReceiver(sender_call_->Receiver());
+ if (num_video_streams_ > 0)
+ receiver_call_->SignalChannelNetworkState(MediaType::VIDEO, kNetworkUp);
+ if (num_audio_streams_ > 0)
+ receiver_call_->SignalChannelNetworkState(MediaType::AUDIO, kNetworkUp);
+ } else {
+ // Sender-only call delivers to itself.
+ send_transport_->SetReceiver(sender_call_->Receiver());
+ receive_transport_->SetReceiver(nullptr);
+ }
- CreateSendConfig(num_video_streams_, num_audio_streams_, num_flexfec_streams_,
- send_transport_.get());
- if (test->ShouldCreateReceivers()) {
- CreateMatchingReceiveConfigs(receive_transport_.get());
- }
- if (num_video_streams_ > 0) {
- test->ModifyVideoConfigs(&video_send_config_, &video_receive_configs_,
- &video_encoder_config_);
- }
- if (num_audio_streams_ > 0) {
- test->ModifyAudioConfigs(&audio_send_config_, &audio_receive_configs_);
- }
- if (num_flexfec_streams_ > 0) {
- test->ModifyFlexfecConfigs(&flexfec_receive_configs_);
- }
+ CreateSendConfig(num_video_streams_, num_audio_streams_,
+ num_flexfec_streams_, send_transport_.get());
+ if (test->ShouldCreateReceivers()) {
+ CreateMatchingReceiveConfigs(receive_transport_.get());
+ }
+ if (num_video_streams_ > 0) {
+ test->ModifyVideoConfigs(&video_send_config_, &video_receive_configs_,
+ &video_encoder_config_);
+ }
+ if (num_audio_streams_ > 0) {
+ test->ModifyAudioConfigs(&audio_send_config_, &audio_receive_configs_);
+ }
+ if (num_flexfec_streams_ > 0) {
+ test->ModifyFlexfecConfigs(&flexfec_receive_configs_);
+ }
- if (num_flexfec_streams_ > 0) {
- CreateFlexfecStreams();
- test->OnFlexfecStreamsCreated(flexfec_receive_streams_);
- }
- if (num_video_streams_ > 0) {
- CreateVideoStreams();
- test->OnVideoStreamsCreated(video_send_stream_, video_receive_streams_);
- }
- if (num_audio_streams_ > 0) {
- CreateAudioStreams();
- test->OnAudioStreamsCreated(audio_send_stream_, audio_receive_streams_);
- }
+ if (num_flexfec_streams_ > 0) {
+ CreateFlexfecStreams();
+ test->OnFlexfecStreamsCreated(flexfec_receive_streams_);
+ }
+ if (num_video_streams_ > 0) {
+ CreateVideoStreams();
+ test->OnVideoStreamsCreated(video_send_stream_, video_receive_streams_);
+ }
+ if (num_audio_streams_ > 0) {
+ CreateAudioStreams();
+ test->OnAudioStreamsCreated(audio_send_stream_, audio_receive_streams_);
+ }
- if (num_video_streams_ > 0) {
- int width = kDefaultWidth;
- int height = kDefaultHeight;
- int frame_rate = kDefaultFramerate;
- test->ModifyVideoCaptureStartResolution(&width, &height, &frame_rate);
- CreateFrameGeneratorCapturer(frame_rate, width, height);
- test->OnFrameGeneratorCapturerCreated(frame_generator_capturer_.get());
- }
+ if (num_video_streams_ > 0) {
+ int width = kDefaultWidth;
+ int height = kDefaultHeight;
+ int frame_rate = kDefaultFramerate;
+ test->ModifyVideoCaptureStartResolution(&width, &height, &frame_rate);
+ CreateFrameGeneratorCapturer(frame_rate, width, height);
+ test->OnFrameGeneratorCapturerCreated(frame_generator_capturer_.get());
+ }
- Start();
+ Start();
+ });
+
test->PerformTest();
- send_transport_->StopSending();
- receive_transport_->StopSending();
- Stop();
- DestroyStreams();
- DestroyCalls();
- if (num_audio_streams_ > 0)
- DestroyVoiceEngines();
+ task_queue_.SendTask([this]() {
+ Stop();
+ DestroyStreams();
+ send_transport_.reset();
+ receive_transport_.reset();
+ DestroyCalls();
+ if (num_audio_streams_ > 0)
+ DestroyVoiceEngines();
+ });
test->OnTestFinished();
}
@@ -517,16 +532,19 @@
void BaseTest::OnCallsCreated(Call* sender_call, Call* receiver_call) {
}
-test::PacketTransport* BaseTest::CreateSendTransport(Call* sender_call) {
- return new PacketTransport(sender_call, this, test::PacketTransport::kSender,
- CallTest::payload_type_map_,
- FakeNetworkPipe::Config());
+test::PacketTransport* BaseTest::CreateSendTransport(
+ SingleThreadedTaskQueueForTesting* task_queue,
+ Call* sender_call) {
+ return new PacketTransport(
+ task_queue, sender_call, this, test::PacketTransport::kSender,
+ CallTest::payload_type_map_, FakeNetworkPipe::Config());
}
-test::PacketTransport* BaseTest::CreateReceiveTransport() {
- return new PacketTransport(nullptr, this, test::PacketTransport::kReceiver,
- CallTest::payload_type_map_,
- FakeNetworkPipe::Config());
+test::PacketTransport* BaseTest::CreateReceiveTransport(
+ SingleThreadedTaskQueueForTesting* task_queue) {
+ return new PacketTransport(
+ task_queue, nullptr, this, test::PacketTransport::kReceiver,
+ CallTest::payload_type_map_, FakeNetworkPipe::Config());
}
size_t BaseTest::GetNumVideoStreams() const {
