From 636865e05d7cd7c30ecb4f4338d982054dffe3cb Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Wed, 3 Jun 2020 14:11:26 +0200 Subject: [PATCH] Delete field trial WebRTC-GenericDescriptor this trial is by default on for three months since https://webrtc-review.googlesource.com/c/src/+/168661 Bug: webrtc:11503 Change-Id: I8f2e0996fd1c77113715628198a409f12a525d51 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/176242 Reviewed-by: Sebastian Jansson Reviewed-by: Ilya Nikolaevskiy Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/master@{#31424} --- call/rtp_payload_params.cc | 10 +-- call/rtp_payload_params.h | 1 - call/rtp_payload_params_unittest.cc | 14 +--- call/rtp_video_sender_unittest.cc | 4 - video/end_to_end_tests/codec_tests.cc | 30 +++---- video/full_stack_tests.cc | 109 +++++++++++--------------- video/pc_full_stack_tests.cc | 72 ++++++----------- video/video_quality_test.cc | 5 -- 8 files changed, 83 insertions(+), 162 deletions(-) diff --git a/call/rtp_payload_params.cc b/call/rtp_payload_params.cc index 635ed52c1a..8cff22ad21 100644 --- a/call/rtp_payload_params.cc +++ b/call/rtp_payload_params.cc @@ -139,10 +139,7 @@ RtpPayloadParams::RtpPayloadParams(const uint32_t ssrc, : ssrc_(ssrc), generic_picture_id_experiment_( absl::StartsWith(trials.Lookup("WebRTC-GenericPictureId"), - "Enabled")), - generic_descriptor_experiment_( - !absl::StartsWith(trials.Lookup("WebRTC-GenericDescriptor"), - "Disabled")) { + "Enabled")) { for (auto& spatial_layer : last_shared_frame_id_) spatial_layer.fill(-1); @@ -186,9 +183,8 @@ RTPVideoHeader RtpPayloadParams::GetRtpVideoHeader( SetCodecSpecific(&rtp_video_header, first_frame_in_picture); - if (generic_descriptor_experiment_) - SetGeneric(codec_specific_info, shared_frame_id, is_keyframe, - &rtp_video_header); + SetGeneric(codec_specific_info, shared_frame_id, is_keyframe, + &rtp_video_header); return rtp_video_header; } diff --git a/call/rtp_payload_params.h b/call/rtp_payload_params.h index fa51efd7cc..2e0faeb5c9 100644 --- a/call/rtp_payload_params.h +++ b/call/rtp_payload_params.h @@ -114,7 +114,6 @@ class RtpPayloadParams final { RtpPayloadState state_; const bool generic_picture_id_experiment_; - const bool generic_descriptor_experiment_; }; } // namespace webrtc #endif // CALL_RTP_PAYLOAD_PARAMS_H_ diff --git a/call/rtp_payload_params_unittest.cc b/call/rtp_payload_params_unittest.cc index 75860a806e..98c1e524d9 100644 --- a/call/rtp_payload_params_unittest.cc +++ b/call/rtp_payload_params_unittest.cc @@ -350,8 +350,6 @@ TEST(RtpPayloadParamsTest, PictureIdForOldGenericFormat) { } TEST(RtpPayloadParamsTest, GenericDescriptorForGenericCodec) { - test::ScopedFieldTrials generic_picture_id( - "WebRTC-GenericDescriptor/Enabled/"); RtpPayloadState state{}; EncodedImage encoded_image; @@ -376,8 +374,6 @@ TEST(RtpPayloadParamsTest, GenericDescriptorForGenericCodec) { } TEST(RtpPayloadParamsTest, SetsGenericFromGenericFrameInfo) { - test::ScopedFieldTrials generic_picture_id( - "WebRTC-GenericDescriptor/Enabled/"); RtpPayloadState state; EncodedImage encoded_image; CodecSpecificInfo codec_info; @@ -426,9 +422,7 @@ class RtpPayloadParamsVp8ToGenericTest : public ::testing::Test { enum LayerSync { kNoSync, kSync }; RtpPayloadParamsVp8ToGenericTest() - : generic_descriptor_field_trial_("WebRTC-GenericDescriptor/Enabled/"), - state_(), - params_(123, &state_, trials_config_) {} + : state_(), params_(123, &state_, trials_config_) {} void ConvertAndCheck(int temporal_index, int64_t shared_frame_id, @@ -464,7 +458,6 @@ class RtpPayloadParamsVp8ToGenericTest : public ::testing::Test { } protected: - test::ScopedFieldTrials generic_descriptor_field_trial_; FieldTrialBasedConfig trials_config_; RtpPayloadState state_; RtpPayloadParams params_; @@ -523,9 +516,7 @@ class RtpPayloadParamsH264ToGenericTest : public ::testing::Test { enum LayerSync { kNoSync, kSync }; RtpPayloadParamsH264ToGenericTest() - : generic_descriptor_field_trial_("WebRTC-GenericDescriptor/Enabled/"), - state_(), - params_(123, &state_, trials_config_) {} + : state_(), params_(123, &state_, trials_config_) {} void ConvertAndCheck(int temporal_index, int64_t shared_frame_id, @@ -561,7 +552,6 @@ class RtpPayloadParamsH264ToGenericTest : public ::testing::Test { } protected: - test::ScopedFieldTrials generic_descriptor_field_trial_; FieldTrialBasedConfig trials_config_; RtpPayloadState state_; RtpPayloadParams params_; diff --git a/call/rtp_video_sender_unittest.cc b/call/rtp_video_sender_unittest.cc index 048fe1a7ec..6051a359f2 100644 --- a/call/rtp_video_sender_unittest.cc +++ b/call/rtp_video_sender_unittest.cc @@ -678,8 +678,6 @@ TEST(RtpVideoSenderTest, EarlyRetransmits) { } TEST(RtpVideoSenderTest, SupportsDependencyDescriptor) { - test::ScopedFieldTrials trials("WebRTC-GenericDescriptor/Enabled/"); - RtpVideoSenderTestFixture test({kSsrc1}, {}, kPayloadType, {}); test.router()->SetActive(true); @@ -744,8 +742,6 @@ TEST(RtpVideoSenderTest, SupportsDependencyDescriptor) { } TEST(RtpVideoSenderTest, SupportsStoppingUsingDependencyDescriptor) { - test::ScopedFieldTrials trials("WebRTC-GenericDescriptor/Enabled/"); - RtpVideoSenderTestFixture test({kSsrc1}, {}, kPayloadType, {}); test.router()->SetActive(true); diff --git a/video/end_to_end_tests/codec_tests.cc b/video/end_to_end_tests/codec_tests.cc index b73b289ec8..d10e08daf1 100644 --- a/video/end_to_end_tests/codec_tests.cc +++ b/video/end_to_end_tests/codec_tests.cc @@ -34,18 +34,14 @@ enum : int { // The first valid value is 1. }; } // namespace -class CodecEndToEndTest : public test::CallTest, - public ::testing::WithParamInterface { +class CodecEndToEndTest : public test::CallTest { public: - CodecEndToEndTest() : field_trial_(GetParam()) { + CodecEndToEndTest() { RegisterRtpExtension( RtpExtension(RtpExtension::kColorSpaceUri, kColorSpaceExtensionId)); RegisterRtpExtension(RtpExtension(RtpExtension::kVideoRotationUri, kVideoRotationExtensionId)); } - - private: - test::ScopedFieldTrials field_trial_; }; class CodecObserver : public test::EndToEndTest, @@ -121,13 +117,7 @@ class CodecObserver : public test::EndToEndTest, int frame_counter_; }; -INSTANTIATE_TEST_SUITE_P( - GenericDescriptor, - CodecEndToEndTest, - ::testing::Values("WebRTC-GenericDescriptor/Disabled/", - "WebRTC-GenericDescriptor/Enabled/")); - -TEST_P(CodecEndToEndTest, SendsAndReceivesVP8) { +TEST_F(CodecEndToEndTest, SendsAndReceivesVP8) { test::FunctionVideoEncoderFactory encoder_factory( []() { return VP8Encoder::Create(); }); test::FunctionVideoDecoderFactory decoder_factory( @@ -137,7 +127,7 @@ TEST_P(CodecEndToEndTest, SendsAndReceivesVP8) { RunBaseTest(&test); } -TEST_P(CodecEndToEndTest, SendsAndReceivesVP8Rotation90) { +TEST_F(CodecEndToEndTest, SendsAndReceivesVP8Rotation90) { test::FunctionVideoEncoderFactory encoder_factory( []() { return VP8Encoder::Create(); }); test::FunctionVideoDecoderFactory decoder_factory( @@ -148,7 +138,7 @@ TEST_P(CodecEndToEndTest, SendsAndReceivesVP8Rotation90) { } #if defined(RTC_ENABLE_VP9) -TEST_P(CodecEndToEndTest, SendsAndReceivesVP9) { +TEST_F(CodecEndToEndTest, SendsAndReceivesVP9) { test::FunctionVideoEncoderFactory encoder_factory( []() { return VP9Encoder::Create(); }); test::FunctionVideoDecoderFactory decoder_factory( @@ -158,7 +148,7 @@ TEST_P(CodecEndToEndTest, SendsAndReceivesVP9) { RunBaseTest(&test); } -TEST_P(CodecEndToEndTest, SendsAndReceivesVP9VideoRotation90) { +TEST_F(CodecEndToEndTest, SendsAndReceivesVP9VideoRotation90) { test::FunctionVideoEncoderFactory encoder_factory( []() { return VP9Encoder::Create(); }); test::FunctionVideoDecoderFactory decoder_factory( @@ -168,7 +158,7 @@ TEST_P(CodecEndToEndTest, SendsAndReceivesVP9VideoRotation90) { RunBaseTest(&test); } -TEST_P(CodecEndToEndTest, SendsAndReceivesVP9ExplicitColorSpace) { +TEST_F(CodecEndToEndTest, SendsAndReceivesVP9ExplicitColorSpace) { test::FunctionVideoEncoderFactory encoder_factory( []() { return VP9Encoder::Create(); }); test::FunctionVideoDecoderFactory decoder_factory( @@ -179,7 +169,7 @@ TEST_P(CodecEndToEndTest, SendsAndReceivesVP9ExplicitColorSpace) { RunBaseTest(&test); } -TEST_P(CodecEndToEndTest, +TEST_F(CodecEndToEndTest, SendsAndReceivesVP9ExplicitColorSpaceWithHdrMetadata) { test::FunctionVideoEncoderFactory encoder_factory( []() { return VP9Encoder::Create(); }); @@ -192,7 +182,7 @@ TEST_P(CodecEndToEndTest, } // Mutiplex tests are using VP9 as the underlying implementation. -TEST_P(CodecEndToEndTest, SendsAndReceivesMultiplex) { +TEST_F(CodecEndToEndTest, SendsAndReceivesMultiplex) { InternalEncoderFactory internal_encoder_factory; InternalDecoderFactory internal_decoder_factory; test::FunctionVideoEncoderFactory encoder_factory( @@ -211,7 +201,7 @@ TEST_P(CodecEndToEndTest, SendsAndReceivesMultiplex) { RunBaseTest(&test); } -TEST_P(CodecEndToEndTest, SendsAndReceivesMultiplexVideoRotation90) { +TEST_F(CodecEndToEndTest, SendsAndReceivesMultiplexVideoRotation90) { InternalEncoderFactory internal_encoder_factory; InternalDecoderFactory internal_decoder_factory; test::FunctionVideoEncoderFactory encoder_factory( diff --git a/video/full_stack_tests.cc b/video/full_stack_tests.cc index 7307b462b7..a12b334230 100644 --- a/video/full_stack_tests.cc +++ b/video/full_stack_tests.cc @@ -90,26 +90,6 @@ std::string ClipNameToClipPath(const char* clip_name) { // logs // bool // }; -class GenericDescriptorTest : public ::testing::TestWithParam { - public: - GenericDescriptorTest() - : field_trial_(AppendFieldTrials(GetParam())), - generic_descriptor_enabled_( - field_trial::IsEnabled("WebRTC-GenericDescriptor")) {} - - std::string GetTestName(std::string base) { - if (generic_descriptor_enabled_) - base += "_generic_descriptor"; - return base; - } - - bool GenericDescriptorEnabled() const { return generic_descriptor_enabled_; } - - private: - test::ScopedFieldTrials field_trial_; - bool generic_descriptor_enabled_; -}; - #if defined(RTC_ENABLE_VP9) TEST(FullStackTest, ForemanCifWithoutPacketLossVp9) { auto fixture = CreateVideoQualityTestFixture(); @@ -125,7 +105,7 @@ TEST(FullStackTest, ForemanCifWithoutPacketLossVp9) { fixture->RunWithAnalyzer(foreman_cif); } -TEST_P(GenericDescriptorTest, ForemanCifPlr5Vp9) { +TEST(GenericDescriptorTest, ForemanCifPlr5Vp9) { auto fixture = CreateVideoQualityTestFixture(); ParamsWithLogging foreman_cif; foreman_cif.call.send_side_bwe = true; @@ -134,11 +114,11 @@ TEST_P(GenericDescriptorTest, ForemanCifPlr5Vp9) { 30000, 500000, 2000000, false, "VP9", 1, 0, 0, false, false, true, ClipNameToClipPath("foreman_cif")}; - foreman_cif.analyzer = {GetTestName("foreman_cif_delay_50_0_plr_5_VP9"), 0.0, - 0.0, kFullStackTestDurationSecs}; + foreman_cif.analyzer = {"foreman_cif_delay_50_0_plr_5_VP9_generic_descriptor", + 0.0, 0.0, kFullStackTestDurationSecs}; foreman_cif.config->loss_percent = 5; foreman_cif.config->queue_delay_ms = 50; - foreman_cif.call.generic_descriptor = GenericDescriptorEnabled(); + foreman_cif.call.generic_descriptor = true; fixture->RunWithAnalyzer(foreman_cif); } @@ -217,7 +197,7 @@ TEST(FullStackTest, MAYBE_ParisQcifWithoutPacketLoss) { fixture->RunWithAnalyzer(paris_qcif); } -TEST_P(GenericDescriptorTest, ForemanCifWithoutPacketLoss) { +TEST(GenericDescriptorTest, ForemanCifWithoutPacketLoss) { auto fixture = CreateVideoQualityTestFixture(); // TODO(pbos): Decide on psnr/ssim thresholds for foreman_cif. ParamsWithLogging foreman_cif; @@ -227,13 +207,13 @@ TEST_P(GenericDescriptorTest, ForemanCifWithoutPacketLoss) { 700000, 700000, 700000, false, "VP8", 1, 0, 0, false, false, true, ClipNameToClipPath("foreman_cif")}; - foreman_cif.analyzer = {GetTestName("foreman_cif_net_delay_0_0_plr_0"), 0.0, - 0.0, kFullStackTestDurationSecs}; - foreman_cif.call.generic_descriptor = GenericDescriptorEnabled(); + foreman_cif.analyzer = {"foreman_cif_net_delay_0_0_plr_0_generic_descriptor", + 0.0, 0.0, kFullStackTestDurationSecs}; + foreman_cif.call.generic_descriptor = true; fixture->RunWithAnalyzer(foreman_cif); } -TEST_P(GenericDescriptorTest, ForemanCif30kbpsWithoutPacketLoss) { +TEST(GenericDescriptorTest, ForemanCif30kbpsWithoutPacketLoss) { auto fixture = CreateVideoQualityTestFixture(); ParamsWithLogging foreman_cif; foreman_cif.call.send_side_bwe = true; @@ -242,15 +222,16 @@ TEST_P(GenericDescriptorTest, ForemanCif30kbpsWithoutPacketLoss) { 30000, 30000, 30000, false, "VP8", 1, 0, 0, false, false, true, ClipNameToClipPath("foreman_cif")}; - foreman_cif.analyzer = {GetTestName("foreman_cif_30kbps_net_delay_0_0_plr_0"), - 0.0, 0.0, kFullStackTestDurationSecs}; - foreman_cif.call.generic_descriptor = GenericDescriptorEnabled(); + foreman_cif.analyzer = { + "foreman_cif_30kbps_net_delay_0_0_plr_0_generic_descriptor", 0.0, 0.0, + kFullStackTestDurationSecs}; + foreman_cif.call.generic_descriptor = true; fixture->RunWithAnalyzer(foreman_cif); } // TODO(webrtc:9722): Remove when experiment is cleaned up. -TEST_P(GenericDescriptorTest, - ForemanCif30kbpsWithoutPacketLossTrustedRateControl) { +TEST(GenericDescriptorTest, + ForemanCif30kbpsWithoutPacketLossTrustedRateControl) { test::ScopedFieldTrials override_field_trials( AppendFieldTrials(kVp8TrustedRateControllerFieldTrial)); auto fixture = CreateVideoQualityTestFixture(); @@ -263,9 +244,10 @@ TEST_P(GenericDescriptorTest, "VP8", 1, 0, 0, false, false, true, ClipNameToClipPath("foreman_cif")}; foreman_cif.analyzer = { - GetTestName("foreman_cif_30kbps_net_delay_0_0_plr_0_trusted_rate_ctrl"), + "foreman_cif_30kbps_net_delay_0_0_plr_0_trusted_rate_ctrl_generic_" + "descriptor", 0.0, 0.0, kFullStackTestDurationSecs}; - foreman_cif.call.generic_descriptor = GenericDescriptorEnabled(); + foreman_cif.call.generic_descriptor = true; fixture->RunWithAnalyzer(foreman_cif); } @@ -328,7 +310,7 @@ TEST(FullStackTest, ForemanCifMediaCapacitySmallLossAndQueue) { fixture->RunWithAnalyzer(foreman_cif); } -TEST_P(GenericDescriptorTest, ForemanCifPlr5) { +TEST(GenericDescriptorTest, ForemanCifPlr5) { auto fixture = CreateVideoQualityTestFixture(); ParamsWithLogging foreman_cif; foreman_cif.call.send_side_bwe = true; @@ -337,15 +319,15 @@ TEST_P(GenericDescriptorTest, ForemanCifPlr5) { 30000, 500000, 2000000, false, "VP8", 1, 0, 0, false, false, true, ClipNameToClipPath("foreman_cif")}; - foreman_cif.analyzer = {GetTestName("foreman_cif_delay_50_0_plr_5"), 0.0, 0.0, - kFullStackTestDurationSecs}; + foreman_cif.analyzer = {"foreman_cif_delay_50_0_plr_5_generic_descriptor", + 0.0, 0.0, kFullStackTestDurationSecs}; foreman_cif.config->loss_percent = 5; foreman_cif.config->queue_delay_ms = 50; - foreman_cif.call.generic_descriptor = GenericDescriptorEnabled(); + foreman_cif.call.generic_descriptor = true; fixture->RunWithAnalyzer(foreman_cif); } -TEST_P(GenericDescriptorTest, ForemanCifPlr5Ulpfec) { +TEST(GenericDescriptorTest, ForemanCifPlr5Ulpfec) { auto fixture = CreateVideoQualityTestFixture(); ParamsWithLogging foreman_cif; foreman_cif.call.send_side_bwe = true; @@ -354,11 +336,12 @@ TEST_P(GenericDescriptorTest, ForemanCifPlr5Ulpfec) { 30000, 500000, 2000000, false, "VP8", 1, 0, 0, true, false, true, ClipNameToClipPath("foreman_cif")}; - foreman_cif.analyzer = {GetTestName("foreman_cif_delay_50_0_plr_5_ulpfec"), - 0.0, 0.0, kFullStackTestDurationSecs}; + foreman_cif.analyzer = { + "foreman_cif_delay_50_0_plr_5_ulpfec_generic_descriptor", 0.0, 0.0, + kFullStackTestDurationSecs}; foreman_cif.config->loss_percent = 5; foreman_cif.config->queue_delay_ms = 50; - foreman_cif.call.generic_descriptor = GenericDescriptorEnabled(); + foreman_cif.call.generic_descriptor = true; fixture->RunWithAnalyzer(foreman_cif); } @@ -442,7 +425,7 @@ TEST(FullStackTest, ForemanCif30kbpsWithoutPacketlossH264) { fixture->RunWithAnalyzer(foreman_cif); } -TEST_P(GenericDescriptorTest, ForemanCifPlr5H264) { +TEST(GenericDescriptorTest, ForemanCifPlr5H264) { auto fixture = CreateVideoQualityTestFixture(); ParamsWithLogging foreman_cif; foreman_cif.call.send_side_bwe = true; @@ -451,11 +434,12 @@ TEST_P(GenericDescriptorTest, ForemanCifPlr5H264) { 30000, 500000, 2000000, false, "H264", 1, 0, 0, false, false, true, ClipNameToClipPath("foreman_cif")}; - foreman_cif.analyzer = {GetTestName("foreman_cif_delay_50_0_plr_5_H264"), 0.0, - 0.0, kFullStackTestDurationSecs}; + foreman_cif.analyzer = { + "foreman_cif_delay_50_0_plr_5_H264_generic_descriptor", 0.0, 0.0, + kFullStackTestDurationSecs}; foreman_cif.config->loss_percent = 5; foreman_cif.config->queue_delay_ms = 50; - foreman_cif.call.generic_descriptor = GenericDescriptorEnabled(); + foreman_cif.call.generic_descriptor = true; fixture->RunWithAnalyzer(foreman_cif); } @@ -565,7 +549,7 @@ TEST(FullStackTest, ForemanCif500kbps100ms) { fixture->RunWithAnalyzer(foreman_cif); } -TEST_P(GenericDescriptorTest, ForemanCif500kbps100msLimitedQueue) { +TEST(GenericDescriptorTest, ForemanCif500kbps100msLimitedQueue) { auto fixture = CreateVideoQualityTestFixture(); ParamsWithLogging foreman_cif; foreman_cif.call.send_side_bwe = true; @@ -574,12 +558,13 @@ TEST_P(GenericDescriptorTest, ForemanCif500kbps100msLimitedQueue) { 30000, 500000, 2000000, false, "VP8", 1, 0, 0, false, false, true, ClipNameToClipPath("foreman_cif")}; - foreman_cif.analyzer = {GetTestName("foreman_cif_500kbps_100ms_32pkts_queue"), - 0.0, 0.0, kFullStackTestDurationSecs}; + foreman_cif.analyzer = { + "foreman_cif_500kbps_100ms_32pkts_queue_generic_descriptor", 0.0, 0.0, + kFullStackTestDurationSecs}; foreman_cif.config->queue_length_packets = 32; foreman_cif.config->queue_delay_ms = 100; foreman_cif.config->link_capacity_kbps = 500; - foreman_cif.call.generic_descriptor = GenericDescriptorEnabled(); + foreman_cif.call.generic_descriptor = true; fixture->RunWithAnalyzer(foreman_cif); } @@ -666,7 +651,7 @@ TEST(FullStackTest, ConferenceMotionHd1TLModerateLimitsWhitelistVp8) { fixture->RunWithAnalyzer(conf_motion_hd); } -TEST_P(GenericDescriptorTest, ConferenceMotionHd2TLModerateLimits) { +TEST(GenericDescriptorTest, ConferenceMotionHd2TLModerateLimits) { auto fixture = CreateVideoQualityTestFixture(); ParamsWithLogging conf_motion_hd; conf_motion_hd.call.send_side_bwe = true; @@ -680,13 +665,13 @@ TEST_P(GenericDescriptorTest, ConferenceMotionHd2TLModerateLimits) { false, false, false, ClipNameToClipPath("ConferenceMotion_1280_720_50")}; conf_motion_hd.analyzer = { - GetTestName("conference_motion_hd_2tl_moderate_limits"), 0.0, 0.0, + "conference_motion_hd_2tl_moderate_limits_generic_descriptor", 0.0, 0.0, kFullStackTestDurationSecs}; conf_motion_hd.config->queue_length_packets = 50; conf_motion_hd.config->loss_percent = 3; conf_motion_hd.config->queue_delay_ms = 100; conf_motion_hd.config->link_capacity_kbps = 2000; - conf_motion_hd.call.generic_descriptor = GenericDescriptorEnabled(); + conf_motion_hd.call.generic_descriptor = true; fixture->RunWithAnalyzer(conf_motion_hd); } @@ -867,7 +852,7 @@ TEST(FullStackTest, ScreenshareSlidesVP8_2TL_Scroll) { fixture->RunWithAnalyzer(config); } -TEST_P(GenericDescriptorTest, ScreenshareSlidesVP8_2TL_LossyNet) { +TEST(GenericDescriptorTest, ScreenshareSlidesVP8_2TL_LossyNet) { auto fixture = CreateVideoQualityTestFixture(); ParamsWithLogging screenshare; screenshare.call.send_side_bwe = true; @@ -875,12 +860,12 @@ TEST_P(GenericDescriptorTest, ScreenshareSlidesVP8_2TL_LossyNet) { 1000000, false, "VP8", 2, 1, 400000, false, false, false, ""}; screenshare.screenshare[0] = {true, false, 10}; - screenshare.analyzer = {GetTestName("screenshare_slides_lossy_net"), 0.0, 0.0, - kFullStackTestDurationSecs}; + screenshare.analyzer = {"screenshare_slides_lossy_net_generic_descriptor", + 0.0, 0.0, kFullStackTestDurationSecs}; screenshare.config->loss_percent = 5; screenshare.config->queue_delay_ms = 200; screenshare.config->link_capacity_kbps = 500; - screenshare.call.generic_descriptor = GenericDescriptorEnabled(); + screenshare.call.generic_descriptor = true; fixture->RunWithAnalyzer(screenshare); } @@ -1246,10 +1231,4 @@ TEST(FullStackTest, MAYBE_LargeRoomVP8_50thumb) { fixture->RunWithAnalyzer(large_room); } -INSTANTIATE_TEST_SUITE_P( - FullStackTest, - GenericDescriptorTest, - ::testing::Values("WebRTC-GenericDescriptor/Disabled/", - "WebRTC-GenericDescriptor/Enabled/")); - } // namespace webrtc diff --git a/video/pc_full_stack_tests.cc b/video/pc_full_stack_tests.cc index 4ec382ef54..bcc92a348d 100644 --- a/video/pc_full_stack_tests.cc +++ b/video/pc_full_stack_tests.cc @@ -106,24 +106,6 @@ std::string ClipNameToClipPath(const char* clip_name) { } // namespace -class PCGenericDescriptorTest : public ::testing::TestWithParam { - public: - PCGenericDescriptorTest() - : field_trial_(AppendFieldTrials(GetParam())), - generic_descriptor_enabled_( - field_trial::IsEnabled("WebRTC-GenericDescriptor")) {} - - std::string GetTestName(std::string base) { - if (generic_descriptor_enabled_) - base += "_generic_descriptor"; - return base; - } - - private: - test::ScopedFieldTrials field_trial_; - bool generic_descriptor_enabled_; -}; - #if defined(RTC_ENABLE_VP9) TEST(PCFullStackTest, ForemanCifWithoutPacketLossVp9) { std::unique_ptr network_emulation_manager = @@ -149,14 +131,14 @@ TEST(PCFullStackTest, ForemanCifWithoutPacketLossVp9) { fixture->Run(std::move(run_params)); } -TEST_P(PCGenericDescriptorTest, ForemanCifPlr5Vp9) { +TEST(PCGenericDescriptorTest, ForemanCifPlr5Vp9) { std::unique_ptr network_emulation_manager = CreateNetworkEmulationManager(); BuiltInNetworkBehaviorConfig config; config.loss_percent = 5; config.queue_delay_ms = 50; auto fixture = CreateTestFixture( - GetTestName("pc_foreman_cif_delay_50_0_plr_5_VP9"), + "pc_foreman_cif_delay_50_0_plr_5_VP9_generic_descriptor", CreateTwoNetworkLinks(network_emulation_manager.get(), config), [](PeerConfigurer* alice) { VideoConfig video(352, 288, 30); @@ -263,11 +245,11 @@ TEST(PCFullStackTest, ParisQcifWithoutPacketLoss) { fixture->Run(std::move(run_params)); } -TEST_P(PCGenericDescriptorTest, ForemanCifWithoutPacketLoss) { +TEST(PCGenericDescriptorTest, ForemanCifWithoutPacketLoss) { std::unique_ptr network_emulation_manager = CreateNetworkEmulationManager(); auto fixture = CreateTestFixture( - GetTestName("pc_foreman_cif_net_delay_0_0_plr_0"), + "pc_foreman_cif_net_delay_0_0_plr_0_generic_descriptor", CreateTwoNetworkLinks(network_emulation_manager.get(), BuiltInNetworkBehaviorConfig()), [](PeerConfigurer* alice) { @@ -285,12 +267,12 @@ TEST_P(PCGenericDescriptorTest, ForemanCifWithoutPacketLoss) { fixture->Run(std::move(run_params)); } -TEST_P(PCGenericDescriptorTest, ForemanCif30kbpsWithoutPacketLoss) { +TEST(PCGenericDescriptorTest, ForemanCif30kbpsWithoutPacketLoss) { std::unique_ptr network_emulation_manager = CreateNetworkEmulationManager(); BuiltInNetworkBehaviorConfig config; auto fixture = CreateTestFixture( - GetTestName("pc_foreman_cif_30kbps_net_delay_0_0_plr_0"), + "pc_foreman_cif_30kbps_net_delay_0_0_plr_0_generic_descriptor", CreateTwoNetworkLinks(network_emulation_manager.get(), config), [](PeerConfigurer* alice) { VideoConfig video(352, 288, 10); @@ -314,16 +296,16 @@ TEST_P(PCGenericDescriptorTest, ForemanCif30kbpsWithoutPacketLoss) { } // TODO(webrtc:9722): Remove when experiment is cleaned up. -TEST_P(PCGenericDescriptorTest, - ForemanCif30kbpsWithoutPacketLossTrustedRateControl) { +TEST(PCGenericDescriptorTest, + ForemanCif30kbpsWithoutPacketLossTrustedRateControl) { test::ScopedFieldTrials override_field_trials( AppendFieldTrials(kVp8TrustedRateControllerFieldTrial)); std::unique_ptr network_emulation_manager = CreateNetworkEmulationManager(); BuiltInNetworkBehaviorConfig config; auto fixture = CreateTestFixture( - GetTestName( - "pc_foreman_cif_30kbps_net_delay_0_0_plr_0_trusted_rate_ctrl"), + "pc_foreman_cif_30kbps_net_delay_0_0_plr_0_trusted_rate_ctrl_generic_" + "descriptor", CreateTwoNetworkLinks(network_emulation_manager.get(), config), [](PeerConfigurer* alice) { VideoConfig video(352, 288, 10); @@ -478,14 +460,14 @@ TEST(PCFullStackTest, ForemanCifMediaCapacitySmallLossAndQueue) { fixture->Run(std::move(run_params)); } -TEST_P(PCGenericDescriptorTest, ForemanCifPlr5) { +TEST(PCGenericDescriptorTest, ForemanCifPlr5) { std::unique_ptr network_emulation_manager = CreateNetworkEmulationManager(); BuiltInNetworkBehaviorConfig config; config.loss_percent = 5; config.queue_delay_ms = 50; auto fixture = CreateTestFixture( - GetTestName("pc_foreman_cif_delay_50_0_plr_5"), + "pc_foreman_cif_delay_50_0_plr_5_generic_descriptor", CreateTwoNetworkLinks(network_emulation_manager.get(), config), [](PeerConfigurer* alice) { VideoConfig video(352, 288, 30); @@ -502,14 +484,14 @@ TEST_P(PCGenericDescriptorTest, ForemanCifPlr5) { fixture->Run(std::move(run_params)); } -TEST_P(PCGenericDescriptorTest, ForemanCifPlr5Ulpfec) { +TEST(PCGenericDescriptorTest, ForemanCifPlr5Ulpfec) { std::unique_ptr network_emulation_manager = CreateNetworkEmulationManager(); BuiltInNetworkBehaviorConfig config; config.loss_percent = 5; config.queue_delay_ms = 50; auto fixture = CreateTestFixture( - GetTestName("pc_foreman_cif_delay_50_0_plr_5_ulpfec"), + "pc_foreman_cif_delay_50_0_plr_5_ulpfec_generic_descriptor", CreateTwoNetworkLinks(network_emulation_manager.get(), config), [](PeerConfigurer* alice) { VideoConfig video(352, 288, 30); @@ -651,14 +633,14 @@ TEST(PCFullStackTest, ForemanCif30kbpsWithoutPacketlossH264) { fixture->Run(std::move(run_params)); } -TEST_P(PCGenericDescriptorTest, ForemanCifPlr5H264) { +TEST(PCGenericDescriptorTest, ForemanCifPlr5H264) { std::unique_ptr network_emulation_manager = CreateNetworkEmulationManager(); BuiltInNetworkBehaviorConfig config; config.loss_percent = 5; config.queue_delay_ms = 50; auto fixture = CreateTestFixture( - GetTestName("pc_foreman_cif_delay_50_0_plr_5_H264"), + "pc_foreman_cif_delay_50_0_plr_5_H264_generic_descriptor", CreateTwoNetworkLinks(network_emulation_manager.get(), config), [](PeerConfigurer* alice) { VideoConfig video(352, 288, 30); @@ -828,7 +810,7 @@ TEST(PCFullStackTest, ForemanCif500kbps100ms) { fixture->Run(std::move(run_params)); } -TEST_P(PCGenericDescriptorTest, ForemanCif500kbps100msLimitedQueue) { +TEST(PCGenericDescriptorTest, ForemanCif500kbps100msLimitedQueue) { std::unique_ptr network_emulation_manager = CreateNetworkEmulationManager(); BuiltInNetworkBehaviorConfig config; @@ -836,7 +818,7 @@ TEST_P(PCGenericDescriptorTest, ForemanCif500kbps100msLimitedQueue) { config.queue_delay_ms = 100; config.link_capacity_kbps = 500; auto fixture = CreateTestFixture( - GetTestName("pc_foreman_cif_500kbps_100ms_32pkts_queue"), + "pc_foreman_cif_500kbps_100ms_32pkts_queue_generic_descriptor", CreateTwoNetworkLinks(network_emulation_manager.get(), config), [](PeerConfigurer* alice) { VideoConfig video(352, 288, 30); @@ -956,7 +938,7 @@ TEST(PCFullStackTest, ConferenceMotionHd1TLModerateLimitsWhitelistVp8) { /* // TODO(bugs.webrtc.org/10639) requires simulcast/SVC support in PC framework -TEST_P(PCGenericDescriptorTest, ConferenceMotionHd2TLModerateLimits) { +TEST(PCGenericDescriptorTest, ConferenceMotionHd2TLModerateLimits) { auto fixture = CreateVideoQualityTestFixture(); ParamsWithLogging conf_motion_hd; conf_motion_hd.call.send_side_bwe = true; @@ -970,7 +952,7 @@ TEST_P(PCGenericDescriptorTest, ConferenceMotionHd2TLModerateLimits) { false, false, false, ClipNameToClipPath("ConferenceMotion_1280_720_50")}; conf_motion_hd.analyzer = { - GetTestName("conference_motion_hd_2tl_moderate_limits"), 0.0, 0.0, + "conference_motion_hd_2tl_moderate_limits_generic_descriptor", 0.0, 0.0, kTestDurationSec}; conf_motion_hd.config->queue_length_packets = 50; conf_motion_hd.config->loss_percent = 3; @@ -1295,7 +1277,7 @@ TEST(PCFullStackTest, ScreenshareSlidesVP8_2TL_Scroll) { } // TODO(bugs.webrtc.org/10639) requires simulcast/SVC support in PC framework -TEST_P(PCGenericDescriptorTest, ScreenshareSlidesVP8_2TL_LossyNet) { +TEST(PCGenericDescriptorTest, ScreenshareSlidesVP8_2TL_LossyNet) { auto fixture = CreateVideoQualityTestFixture(); ParamsWithLogging screenshare; screenshare.call.send_side_bwe = true; @@ -1303,12 +1285,12 @@ TEST_P(PCGenericDescriptorTest, ScreenshareSlidesVP8_2TL_LossyNet) { 1000000, false, "VP8", 2, 1, 400000, false, false, false, ""}; screenshare.screenshare[0] = {true, false, 10}; - screenshare.analyzer = {GetTestName("screenshare_slides_lossy_net"), 0.0, 0.0, - kTestDurationSec}; + screenshare.analyzer = {"screenshare_slides_lossy_net_generic_descriptor", + 0.0, 0.0, kTestDurationSec}; screenshare.config->loss_percent = 5; screenshare.config->queue_delay_ms = 200; screenshare.config->link_capacity_kbps = 500; - screenshare.call.generic_descriptor = GenericDescriptorEnabled(); + screenshare.call.generic_descriptor = true; fixture->RunWithAnalyzer(screenshare); } @@ -1813,12 +1795,6 @@ TEST(PCFullStackTest, MAYBE_LargeRoomVP8_50thumb) { } */ -INSTANTIATE_TEST_SUITE_P( - PCFullStackTest, - PCGenericDescriptorTest, - ::testing::Values("WebRTC-GenericDescriptor/Disabled/", - "WebRTC-GenericDescriptor/Enabled/")); - class PCDualStreamsTest : public ::testing::TestWithParam {}; /* diff --git a/video/video_quality_test.cc b/video/video_quality_test.cc index 94ce268fa9..88270b4b2e 100644 --- a/video/video_quality_test.cc +++ b/video/video_quality_test.cc @@ -815,11 +815,6 @@ void VideoQualityTest::SetupVideo(Transport* send_transport, } if (params_.call.generic_descriptor) { - // The generic descriptor is currently behind a field trial, so it needs - // to be set for this flag to have any effect. - // TODO(philipel): Remove this check when the experiment is removed. - RTC_CHECK(field_trial::IsEnabled("WebRTC-GenericDescriptor")); - video_send_configs_[video_idx].rtp.extensions.emplace_back( RtpExtension::kGenericFrameDescriptorUri00, kGenericFrameDescriptorExtensionId00);