diff --git a/api/test/video_quality_test_fixture.h b/api/test/video_quality_test_fixture.h index 5c0e8ce169..99451f9ee8 100644 --- a/api/test/video_quality_test_fixture.h +++ b/api/test/video_quality_test_fixture.h @@ -34,6 +34,7 @@ class VideoQualityTestFixtureInterface { ~Params(); struct CallConfig { bool send_side_bwe; + bool generic_descriptor; BitrateConstraints call_bitrate_config; int num_thumbnails; // Indicates if secondary_(video|ss|screenshare) structures are used. diff --git a/call/rtp_payload_params.cc b/call/rtp_payload_params.cc index 0c8b30755b..ffbda1a2db 100644 --- a/call/rtp_payload_params.cc +++ b/call/rtp_payload_params.cc @@ -116,7 +116,9 @@ RtpPayloadParams::RtpPayloadParams(const uint32_t ssrc, const RtpPayloadState* state) : ssrc_(ssrc), generic_picture_id_experiment_( - field_trial::IsEnabled("WebRTC-GenericPictureId")) { + field_trial::IsEnabled("WebRTC-GenericPictureId")), + generic_descriptor_experiment_( + field_trial::IsEnabled("WebRTC-GenericDescriptor")) { for (auto& spatial_layer : last_shared_frame_id_) spatial_layer.fill(-1); @@ -152,7 +154,9 @@ RTPVideoHeader RtpPayloadParams::GetRtpVideoHeader( : true; SetCodecSpecific(&rtp_video_header, first_frame_in_picture); - SetGeneric(shared_frame_id, is_keyframe, &rtp_video_header); + + if (generic_descriptor_experiment_) + SetGeneric(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 664ac17346..f6829cace5 100644 --- a/call/rtp_payload_params.h +++ b/call/rtp_payload_params.h @@ -60,6 +60,7 @@ 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 c125c4ee8f..a3cb379d18 100644 --- a/call/rtp_payload_params_unittest.cc +++ b/call/rtp_payload_params_unittest.cc @@ -300,7 +300,10 @@ class RtpPayloadParamsVp8ToGenericTest : public ::testing::Test { public: enum LayerSync { kNoSync, kSync }; - RtpPayloadParamsVp8ToGenericTest() : state_(), params_(123, &state_) {} + RtpPayloadParamsVp8ToGenericTest() + : generic_descriptor_field_trial_("WebRTC-GenericDescriptor/Enabled/"), + state_(), + params_(123, &state_) {} void ConvertAndCheck(int temporal_index, int64_t shared_frame_id, @@ -330,6 +333,7 @@ class RtpPayloadParamsVp8ToGenericTest : public ::testing::Test { } protected: + test::ScopedFieldTrials generic_descriptor_field_trial_; RtpPayloadState state_; RtpPayloadParams params_; }; diff --git a/modules/rtp_rtcp/source/rtp_generic_frame_descriptor_extension.h b/modules/rtp_rtcp/source/rtp_generic_frame_descriptor_extension.h index d59a9b03a4..2dd9dd49eb 100644 --- a/modules/rtp_rtcp/source/rtp_generic_frame_descriptor_extension.h +++ b/modules/rtp_rtcp/source/rtp_generic_frame_descriptor_extension.h @@ -25,6 +25,7 @@ class RtpGenericFrameDescriptorExtension { static constexpr char kUri[] = "http://www.webrtc.org/experiments/rtp-hdrext/" "generic-frame-descriptor-00"; + static constexpr int kMaxSizeBytes = 16; static bool Parse(rtc::ArrayView data, RtpGenericFrameDescriptor* descriptor); diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc index 90ad9e9a9f..62ac066ffa 100644 --- a/modules/rtp_rtcp/source/rtp_sender.cc +++ b/modules/rtp_rtcp/source/rtp_sender.cc @@ -22,6 +22,7 @@ #include "modules/rtp_rtcp/include/rtp_cvo.h" #include "modules/rtp_rtcp/source/byte_io.h" #include "modules/rtp_rtcp/source/playout_delay_oracle.h" +#include "modules/rtp_rtcp/source/rtp_generic_frame_descriptor_extension.h" #include "modules/rtp_rtcp/source/rtp_header_extensions.h" #include "modules/rtp_rtcp/source/rtp_packet_to_send.h" #include "modules/rtp_rtcp/source/rtp_sender_audio.h" @@ -74,6 +75,8 @@ constexpr RtpExtensionSize kVideoExtensionSizes[] = { CreateExtensionSize(), CreateExtensionSize(), {RtpMid::kId, RtpMid::kMaxValueSizeBytes}, + {RtpGenericFrameDescriptorExtension::kId, + RtpGenericFrameDescriptorExtension::kMaxSizeBytes}, }; const char* FrameTypeToString(FrameType frame_type) { diff --git a/test/call_test.cc b/test/call_test.cc index f7828a4c3f..89ac329302 100644 --- a/test/call_test.cc +++ b/test/call_test.cc @@ -238,6 +238,8 @@ void CallTest::CreateVideoSendConfig(VideoSendStream::Config* video_config, kTransportSequenceNumberExtensionId)); video_config->rtp.extensions.push_back(RtpExtension( RtpExtension::kVideoContentTypeUri, kVideoContentTypeExtensionId)); + video_config->rtp.extensions.push_back(RtpExtension( + RtpExtension::kGenericFrameDescriptorUri, kGenericDescriptorExtensionId)); if (video_encoder_configs_.empty()) { video_encoder_configs_.emplace_back(); FillEncoderConfiguration(kVideoCodecGeneric, num_video_streams, diff --git a/test/constants.cc b/test/constants.cc index 0835777570..12ae8e8449 100644 --- a/test/constants.cc +++ b/test/constants.cc @@ -20,6 +20,7 @@ const int kTransportSequenceNumberExtensionId = 8; const int kVideoRotationExtensionId = 9; const int kVideoContentTypeExtensionId = 10; const int kVideoTimingExtensionId = 11; +const int kGenericDescriptorExtensionId = 12; } // namespace test } // namespace webrtc diff --git a/test/constants.h b/test/constants.h index 85e8c18d53..3cee828b20 100644 --- a/test/constants.h +++ b/test/constants.h @@ -18,5 +18,6 @@ extern const int kTransportSequenceNumberExtensionId; extern const int kVideoRotationExtensionId; extern const int kVideoContentTypeExtensionId; extern const int kVideoTimingExtensionId; +extern const int kGenericDescriptorExtensionId; } // namespace test } // namespace webrtc diff --git a/video/end_to_end_tests/codec_tests.cc b/video/end_to_end_tests/codec_tests.cc index 31742ee3cd..5c5255567a 100644 --- a/video/end_to_end_tests/codec_tests.cc +++ b/video/end_to_end_tests/codec_tests.cc @@ -23,9 +23,13 @@ namespace webrtc { -class CodecEndToEndTest : public test::CallTest { +class CodecEndToEndTest : public test::CallTest, + public testing::WithParamInterface { public: - CodecEndToEndTest() = default; + CodecEndToEndTest() : field_trial_(GetParam()) {} + + private: + test::ScopedFieldTrials field_trial_; }; class CodecObserver : public test::EndToEndTest, @@ -89,7 +93,12 @@ class CodecObserver : public test::EndToEndTest, int frame_counter_; }; -TEST_F(CodecEndToEndTest, SendsAndReceivesVP8) { +INSTANTIATE_TEST_CASE_P(GenericDescriptor, + CodecEndToEndTest, + ::testing::Values("WebRTC-GenericDescriptor/Disabled/", + "WebRTC-GenericDescriptor/Enabled/")); + +TEST_P(CodecEndToEndTest, SendsAndReceivesVP8) { test::FunctionVideoEncoderFactory encoder_factory( []() { return VP8Encoder::Create(); }); CodecObserver test(5, kVideoRotation_0, "VP8", &encoder_factory, @@ -97,7 +106,7 @@ TEST_F(CodecEndToEndTest, SendsAndReceivesVP8) { RunBaseTest(&test); } -TEST_F(CodecEndToEndTest, SendsAndReceivesVP8Rotation90) { +TEST_P(CodecEndToEndTest, SendsAndReceivesVP8Rotation90) { test::FunctionVideoEncoderFactory encoder_factory( []() { return VP8Encoder::Create(); }); CodecObserver test(5, kVideoRotation_90, "VP8", &encoder_factory, @@ -106,7 +115,7 @@ TEST_F(CodecEndToEndTest, SendsAndReceivesVP8Rotation90) { } #if !defined(RTC_DISABLE_VP9) -TEST_F(CodecEndToEndTest, SendsAndReceivesVP9) { +TEST_P(CodecEndToEndTest, SendsAndReceivesVP9) { test::FunctionVideoEncoderFactory encoder_factory( []() { return VP9Encoder::Create(); }); CodecObserver test(500, kVideoRotation_0, "VP9", &encoder_factory, @@ -114,7 +123,7 @@ TEST_F(CodecEndToEndTest, SendsAndReceivesVP9) { RunBaseTest(&test); } -TEST_F(CodecEndToEndTest, SendsAndReceivesVP9VideoRotation90) { +TEST_P(CodecEndToEndTest, SendsAndReceivesVP9VideoRotation90) { test::FunctionVideoEncoderFactory encoder_factory( []() { return VP9Encoder::Create(); }); CodecObserver test(5, kVideoRotation_90, "VP9", &encoder_factory, @@ -123,7 +132,7 @@ TEST_F(CodecEndToEndTest, SendsAndReceivesVP9VideoRotation90) { } // Mutiplex tests are using VP9 as the underlying implementation. -TEST_F(CodecEndToEndTest, SendsAndReceivesMultiplex) { +TEST_P(CodecEndToEndTest, SendsAndReceivesMultiplex) { InternalEncoderFactory internal_encoder_factory; InternalDecoderFactory decoder_factory; test::FunctionVideoEncoderFactory encoder_factory( @@ -138,7 +147,7 @@ TEST_F(CodecEndToEndTest, SendsAndReceivesMultiplex) { RunBaseTest(&test); } -TEST_F(CodecEndToEndTest, SendsAndReceivesMultiplexVideoRotation90) { +TEST_P(CodecEndToEndTest, SendsAndReceivesMultiplexVideoRotation90) { InternalEncoderFactory internal_encoder_factory; InternalDecoderFactory decoder_factory; test::FunctionVideoEncoderFactory encoder_factory( diff --git a/video/screenshare_loopback.cc b/video/screenshare_loopback.cc index 804fbccf25..ec1d59e6a4 100644 --- a/video/screenshare_loopback.cc +++ b/video/screenshare_loopback.cc @@ -220,6 +220,8 @@ DEFINE_bool(logs, false, "print logs to stderr"); DEFINE_bool(send_side_bwe, true, "Use send-side bandwidth estimation"); +DEFINE_bool(generic_descriptor, false, "Use the generic frame descriptor."); + DEFINE_bool(allow_reordering, false, "Allow packet reordering to occur"); DEFINE_string( @@ -288,7 +290,8 @@ void Loopback() { call_bitrate_config.max_bitrate_bps = -1; // Don't cap bandwidth estimate. VideoQualityTest::Params params; - params.call = {flags::FLAG_send_side_bwe, call_bitrate_config}; + params.call = {flags::FLAG_send_side_bwe, flags::FLAG_generic_descriptor, + call_bitrate_config}; params.video[0] = {true, flags::Width(), flags::Height(), diff --git a/video/sv_loopback.cc b/video/sv_loopback.cc index 0555fb3552..25bfa01dbc 100644 --- a/video/sv_loopback.cc +++ b/video/sv_loopback.cc @@ -419,6 +419,8 @@ DEFINE_bool(logs, false, "print logs to stderr"); DEFINE_bool(send_side_bwe, true, "Use send-side bandwidth estimation"); +DEFINE_bool(generic_descriptor, false, "Use the generic frame descriptor."); + DEFINE_bool(allow_reordering, false, "Allow packet reordering to occur"); DEFINE_bool(use_ulpfec, false, "Use RED+ULPFEC forward error correction."); @@ -492,7 +494,8 @@ void Loopback() { 1000; VideoQualityTest::Params params, camera_params, screenshare_params; - params.call = {flags::FLAG_send_side_bwe, call_bitrate_config, 0}; + params.call = {flags::FLAG_send_side_bwe, flags::FLAG_generic_descriptor, + call_bitrate_config, 0}; params.call.dual_video = true; params.video[screenshare_idx] = { true, diff --git a/video/video_loopback.cc b/video/video_loopback.cc index 52f0a65084..71a86f73d6 100644 --- a/video/video_loopback.cc +++ b/video/video_loopback.cc @@ -235,6 +235,8 @@ DEFINE_bool(logs, false, "print logs to stderr"); DEFINE_bool(send_side_bwe, true, "Use send-side bandwidth estimation"); +DEFINE_bool(generic_descriptor, false, "Use the generic frame descriptor."); + DEFINE_bool(allow_reordering, false, "Allow packet reordering to occur"); DEFINE_bool(use_ulpfec, false, "Use RED+ULPFEC forward error correction."); @@ -292,7 +294,8 @@ void Loopback() { call_bitrate_config.max_bitrate_bps = -1; // Don't cap bandwidth estimate. VideoQualityTest::Params params; - params.call = {flags::FLAG_send_side_bwe, call_bitrate_config, 0}; + params.call = {flags::FLAG_send_side_bwe, flags::FLAG_generic_descriptor, + call_bitrate_config, 0}; params.video[0] = {flags::FLAG_video, flags::Width(), flags::Height(), diff --git a/video/video_quality_test.cc b/video/video_quality_test.cc index 7f7c8737a1..cd4c5f7608 100644 --- a/video/video_quality_test.cc +++ b/video/video_quality_test.cc @@ -214,7 +214,7 @@ VideoQualityTest::VideoQualityTest( } VideoQualityTest::Params::Params() - : call({false, BitrateConstraints(), 0}), + : call({false, false, BitrateConstraints(), 0}), video{{false, 640, 480, 30, 50, 800, 800, false, "VP8", 1, -1, 0, false, false, false, ""}, {false, 640, 480, 30, 50, 800, 800, false, "VP8", 1, -1, 0, false, @@ -536,18 +536,29 @@ void VideoQualityTest::SetupVideo(Transport* send_transport, } video_send_configs_[video_idx].rtp.extensions.clear(); if (params_.call.send_side_bwe) { - video_send_configs_[video_idx].rtp.extensions.push_back( - RtpExtension(RtpExtension::kTransportSequenceNumberUri, - test::kTransportSequenceNumberExtensionId)); + video_send_configs_[video_idx].rtp.extensions.emplace_back( + RtpExtension::kTransportSequenceNumberUri, + test::kTransportSequenceNumberExtensionId); } else { - video_send_configs_[video_idx].rtp.extensions.push_back(RtpExtension( - RtpExtension::kAbsSendTimeUri, test::kAbsSendTimeExtensionId)); + video_send_configs_[video_idx].rtp.extensions.emplace_back( + RtpExtension::kAbsSendTimeUri, test::kAbsSendTimeExtensionId); } - video_send_configs_[video_idx].rtp.extensions.push_back( - RtpExtension(RtpExtension::kVideoContentTypeUri, - test::kVideoContentTypeExtensionId)); - video_send_configs_[video_idx].rtp.extensions.push_back(RtpExtension( - RtpExtension::kVideoTimingUri, test::kVideoTimingExtensionId)); + + 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::kGenericFrameDescriptorUri, + test::kGenericDescriptorExtensionId); + } + + video_send_configs_[video_idx].rtp.extensions.emplace_back( + RtpExtension::kVideoContentTypeUri, test::kVideoContentTypeExtensionId); + video_send_configs_[video_idx].rtp.extensions.emplace_back( + RtpExtension::kVideoTimingUri, test::kVideoTimingExtensionId); video_encoder_configs_[video_idx].video_format.name = params_.video[video_idx].codec;