From e269cb4fe27b843f91a502b6f54ad61a025ad424 Mon Sep 17 00:00:00 2001 From: Artem Titov Date: Wed, 29 Aug 2018 09:59:23 +0200 Subject: [PATCH] Add support of overriding network simulation in video quality tests. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add ability to provide custom implementation of NetworkSimulatedInterface for sender and receiver network in VideoQualityTestFixtureInterface, passing them to the factory method. Also unite this mechanism with FecControllerFactoryInterface injection. Bug: webrtc:9630 Change-Id: I79259113e0fc00d933b73ca299afa836a4cd19d2 Reviewed-on: https://webrtc-review.googlesource.com/96280 Commit-Queue: Artem Titov Reviewed-by: Patrik Höglund Reviewed-by: Erik Språng Cr-Commit-Position: refs/heads/master@{#24476} --- api/test/create_video_quality_test_fixture.cc | 11 ++- api/test/create_video_quality_test_fixture.h | 5 +- api/test/video_quality_test_fixture.h | 21 ++++- video/video_quality_test.cc | 81 +++++++++++++------ video/video_quality_test.h | 5 +- 5 files changed, 92 insertions(+), 31 deletions(-) diff --git a/api/test/create_video_quality_test_fixture.cc b/api/test/create_video_quality_test_fixture.cc index 994401b65b..f317ed709e 100644 --- a/api/test/create_video_quality_test_fixture.cc +++ b/api/test/create_video_quality_test_fixture.cc @@ -26,7 +26,16 @@ CreateVideoQualityTestFixture() { std::unique_ptr CreateVideoQualityTestFixture( std::unique_ptr fec_controller_factory) { - return absl::make_unique(std::move(fec_controller_factory)); + auto components = absl::make_unique< + VideoQualityTestFixtureInterface::InjectionComponents>(); + components->fec_controller_factory = std::move(fec_controller_factory); + return absl::make_unique(std::move(components)); +} + +std::unique_ptr CreateVideoQualityTestFixture( + std::unique_ptr + components) { + return absl::make_unique(std::move(components)); } } // namespace webrtc diff --git a/api/test/create_video_quality_test_fixture.h b/api/test/create_video_quality_test_fixture.h index 07c222f16a..135819903c 100644 --- a/api/test/create_video_quality_test_fixture.h +++ b/api/test/create_video_quality_test_fixture.h @@ -24,6 +24,9 @@ std::unique_ptr CreateVideoQualityTestFixture( std::unique_ptr fec_controller_factory); -} +std::unique_ptr CreateVideoQualityTestFixture( + std::unique_ptr + components); +} // namespace webrtc #endif // API_TEST_CREATE_VIDEO_QUALITY_TEST_FIXTURE_H_ diff --git a/api/test/video_quality_test_fixture.h b/api/test/video_quality_test_fixture.h index ed54076ca3..5c0e8ce169 100644 --- a/api/test/video_quality_test_fixture.h +++ b/api/test/video_quality_test_fixture.h @@ -84,8 +84,10 @@ class VideoQualityTestFixtureInterface { // it is just configuration, that will be passed to default implementation // of simulation layer. DefaultNetworkSimulationConfig pipe; - // Config for default simulation implementation. May be nullopt in that - // case, a default config will be used. + // Config for default simulation implementation. Must be nullopt if + // `sender_network` and `receiver_network` in InjectionComponents are + // non-null. May be nullopt even if `sender_network` and `receiver_network` + // are null; in that case, a default config will be used. absl::optional config; struct SS { // Spatial scalability. std::vector streams; // If empty, one stream is assumed. @@ -105,6 +107,21 @@ class VideoQualityTestFixtureInterface { } logging; }; + // Contains objects, that will be injected on different layers of test + // framework to override the behavior of system parts. + struct InjectionComponents { + InjectionComponents(); + ~InjectionComponents(); + + // Simulations of sender and receiver networks. They must either both be + // null (in which case `config` from Params is used), or both be non-null + // (in which case `config` from Params must be nullopt). + std::unique_ptr sender_network; + std::unique_ptr receiver_network; + + std::unique_ptr fec_controller_factory; + }; + virtual ~VideoQualityTestFixtureInterface() = default; virtual void RunWithAnalyzer(const Params& params) = 0; diff --git a/video/video_quality_test.cc b/video/video_quality_test.cc index 434919d7a2..03749a7016 100644 --- a/video/video_quality_test.cc +++ b/video/video_quality_test.cc @@ -87,13 +87,18 @@ std::unique_ptr VideoQualityTest::CreateVideoEncoder( } VideoQualityTest::VideoQualityTest( - std::unique_ptr fec_controller_factory) + std::unique_ptr injection_components) : clock_(Clock::GetRealTimeClock()), video_encoder_factory_([this](const SdpVideoFormat& format) { return this->CreateVideoEncoder(format); }), receive_logs_(0), - send_logs_(0) { + send_logs_(0), + injection_components_(std::move(injection_components)) { + if (injection_components_ == nullptr) { + injection_components_ = absl::make_unique(); + } + payload_type_map_ = test::CallTest::payload_type_map_; RTC_DCHECK(payload_type_map_.find(kPayloadTypeH264) == payload_type_map_.end()); @@ -105,7 +110,8 @@ VideoQualityTest::VideoQualityTest( payload_type_map_[kPayloadTypeVP8] = webrtc::MediaType::VIDEO; payload_type_map_[kPayloadTypeVP9] = webrtc::MediaType::VIDEO; - fec_controller_factory_ = std::move(fec_controller_factory); + fec_controller_factory_ = + std::move(injection_components_->fec_controller_factory); } VideoQualityTest::Params::Params() @@ -127,6 +133,10 @@ VideoQualityTest::Params::Params() VideoQualityTest::Params::~Params() = default; +VideoQualityTest::InjectionComponents::InjectionComponents() = default; + +VideoQualityTest::InjectionComponents::~InjectionComponents() = default; + void VideoQualityTest::TestBody() {} std::string VideoQualityTest::GenerateGraphTitle() const { @@ -144,12 +154,21 @@ std::string VideoQualityTest::GenerateGraphTitle() const { return ss.str(); } -void VideoQualityTest::CheckParams() { - if (!params_.config) { +void VideoQualityTest::CheckParamsAndInjectionComponents() { + if (injection_components_ == nullptr) { + injection_components_ = absl::make_unique(); + } + if (!params_.config && injection_components_->sender_network == nullptr && + injection_components_->receiver_network == nullptr) { // TODO(titovartem) replace with default config creation when removing // pipe. params_.config = params_.pipe; } + RTC_CHECK( + (params_.config && injection_components_->sender_network == nullptr && + injection_components_->receiver_network == nullptr) || + (!params_.config && injection_components_->sender_network != nullptr && + injection_components_->receiver_network != nullptr)); for (size_t video_idx = 0; video_idx < num_video_streams_; ++video_idx) { // Iterate over primary and secondary video streams. if (!params_.video[video_idx].enabled) @@ -161,17 +180,19 @@ void VideoQualityTest::CheckParams() { if (params_.ss[video_idx].num_spatial_layers == 0) params_.ss[video_idx].num_spatial_layers = 1; - if (params_.config->loss_percent != 0 || - params_.config->queue_length_packets != 0) { - // Since LayerFilteringTransport changes the sequence numbers, we can't - // use that feature with pack loss, since the NACK request would end up - // retransmitting the wrong packets. - RTC_CHECK(params_.ss[video_idx].selected_sl == -1 || - params_.ss[video_idx].selected_sl == - params_.ss[video_idx].num_spatial_layers - 1); - RTC_CHECK(params_.video[video_idx].selected_tl == -1 || - params_.video[video_idx].selected_tl == - params_.video[video_idx].num_temporal_layers - 1); + if (params_.config) { + if (params_.config->loss_percent != 0 || + params_.config->queue_length_packets != 0) { + // Since LayerFilteringTransport changes the sequence numbers, we can't + // use that feature with pack loss, since the NACK request would end up + // retransmitting the wrong packets. + RTC_CHECK(params_.ss[video_idx].selected_sl == -1 || + params_.ss[video_idx].selected_sl == + params_.ss[video_idx].num_spatial_layers - 1); + RTC_CHECK(params_.video[video_idx].selected_tl == -1 || + params_.video[video_idx].selected_tl == + params_.video[video_idx].num_temporal_layers - 1); + } } // TODO(ivica): Should max_bitrate_bps == -1 represent inf max bitrate, as @@ -312,7 +333,7 @@ void VideoQualityTest::FillScalabilitySettings( } else { // Read VideoStream and SpatialLayer elements from a list of comma separated // lists. To use a default value for an element, use -1 or leave empty. - // Validity checks performed in CheckParams. + // Validity checks performed in CheckParamsAndInjectionComponents. RTC_CHECK(params->ss[video_idx].streams.empty()); for (auto descriptor : stream_descriptors) { if (descriptor.empty()) @@ -804,11 +825,16 @@ void VideoQualityTest::StopThumbnails() { std::unique_ptr VideoQualityTest::CreateSendTransport() { + std::unique_ptr simulated_network = nullptr; + if (injection_components_->sender_network == nullptr) { + simulated_network = absl::make_unique(*params_.config); + } else { + simulated_network = std::move(injection_components_->sender_network); + } return absl::make_unique( &task_queue_, - absl::make_unique( - Clock::GetRealTimeClock(), - absl::make_unique(*params_.config)), + absl::make_unique(Clock::GetRealTimeClock(), + std::move(simulated_network)), sender_call_.get(), kPayloadTypeVP8, kPayloadTypeVP9, params_.video[0].selected_tl, params_.ss[0].selected_sl, payload_type_map_, kVideoSendSsrcs[0], @@ -818,11 +844,16 @@ VideoQualityTest::CreateSendTransport() { std::unique_ptr VideoQualityTest::CreateReceiveTransport() { + std::unique_ptr simulated_network = nullptr; + if (injection_components_->receiver_network == nullptr) { + simulated_network = absl::make_unique(*params_.config); + } else { + simulated_network = std::move(injection_components_->receiver_network); + } return absl::make_unique( &task_queue_, - absl::make_unique( - Clock::GetRealTimeClock(), - absl::make_unique(*params_.config)), + absl::make_unique(Clock::GetRealTimeClock(), + std::move(simulated_network)), receiver_call_.get(), payload_type_map_); } @@ -836,7 +867,7 @@ void VideoQualityTest::RunWithAnalyzer(const Params& params) { params_ = params; // TODO(ivica): Merge with RunWithRenderer and use a flag / argument to // differentiate between the analyzer and the renderer case. - CheckParams(); + CheckParamsAndInjectionComponents(); if (!params_.analyzer.graph_data_output_filename.empty()) { graph_data_output_file = @@ -1074,7 +1105,7 @@ void VideoQualityTest::RunWithRenderers(const Params& params) { task_queue_.SendTask([&]() { params_ = params; - CheckParams(); + CheckParamsAndInjectionComponents(); // TODO(ivica): Remove bitrate_config and use the default Call::Config(), to // match the full stack tests. diff --git a/video/video_quality_test.h b/video/video_quality_test.h index 1bc55ab177..7fc38ee4f9 100644 --- a/video/video_quality_test.h +++ b/video/video_quality_test.h @@ -32,7 +32,7 @@ class VideoQualityTest : public test::CallTest, public VideoQualityTestFixtureInterface { public: explicit VideoQualityTest( - std::unique_ptr fec_controller_factory); + std::unique_ptr injection_components); void RunWithAnalyzer(const Params& params) override; void RunWithRenderers(const Params& params) override; @@ -66,7 +66,7 @@ class VideoQualityTest : // Helper methods accessing only params_. std::string GenerateGraphTitle() const; - void CheckParams(); + void CheckParamsAndInjectionComponents(); // Helper methods for setting up the call. void CreateCapturers(); @@ -110,6 +110,7 @@ class VideoQualityTest : int send_logs_; Params params_; + std::unique_ptr injection_components_; // Note: not same as similarly named member in CallTest. This is the number of // separate send streams, the one in CallTest is the number of substreams for