Reland "Added field trial WebRTC-GenericDescriptor for the new generic descriptor."
This reverts commit 6f68324adbf52b247e10b33a4e83a586e66cc6df.
Reason for revert: Removed full stack tests that cause timeout.
Original change's description:
> Revert "Added field trial WebRTC-GenericDescriptor for the new generic descriptor."
>
> This reverts commit 3f4a4fad8cd661309ff5d9a631e89518f32e7c5e.
>
> Reason for revert: Breaking internal tests
>
> Original change's description:
> > Added field trial WebRTC-GenericDescriptor for the new generic descriptor.
> >
> > Also parameterized tests to test the new generic descriptor and
> > added --generic_descriptor flag to loopback tests.
> >
> > Bug: webrtc:9361
> > Change-Id: I2b76889606fe2e81249ecdebb0b7b61151682485
> > Reviewed-on: https://webrtc-review.googlesource.com/101900
> > Commit-Queue: Philip Eliasson <philipel@webrtc.org>
> > Reviewed-by: Erik Språng <sprang@webrtc.org>
> > Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
> > Reviewed-by: Stefan Holmer <stefan@webrtc.org>
> > Cr-Commit-Position: refs/heads/master@{#24835}
>
> TBR=danilchap@webrtc.org,sprang@webrtc.org,stefan@webrtc.org,philipel@webrtc.org
>
> Change-Id: I4d4714a9f4ab0e95adf0f4130bc1a932efc448fa
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: webrtc:9361
> Reviewed-on: https://webrtc-review.googlesource.com/101940
> Reviewed-by: Lu Liu <lliuu@webrtc.org>
> Commit-Queue: Lu Liu <lliuu@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#24839}
TBR=danilchap@webrtc.org,sprang@webrtc.org,stefan@webrtc.org,philipel@webrtc.org,lliuu@webrtc.org
Change-Id: Ibcf0a1d3aa947b84e3b891b1975d0fc2c730f2ae
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:9361
Reviewed-on: https://webrtc-review.googlesource.com/102064
Commit-Queue: Philip Eliasson <philipel@webrtc.org>
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#24845}diff --git a/api/test/video_quality_test_fixture.h b/api/test/video_quality_test_fixture.h
index 5c0e8ce..99451f9 100644
--- a/api/test/video_quality_test_fixture.h
+++ b/api/test/video_quality_test_fixture.h
@@ -34,6 +34,7 @@
~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 0c8b307..ffbda1a 100644
--- a/call/rtp_payload_params.cc
+++ b/call/rtp_payload_params.cc
@@ -116,7 +116,9 @@
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 @@
: 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 664ac17..f6829ca 100644
--- a/call/rtp_payload_params.h
+++ b/call/rtp_payload_params.h
@@ -60,6 +60,7 @@
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 c125c4e..a3cb379 100644
--- a/call/rtp_payload_params_unittest.cc
+++ b/call/rtp_payload_params_unittest.cc
@@ -300,7 +300,10 @@
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 @@
}
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 d59a9b0..2dd9dd4 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 @@
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<const uint8_t> data,
RtpGenericFrameDescriptor* descriptor);
diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc
index 90ad9e9..62ac066 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 @@
CreateExtensionSize<VideoContentTypeExtension>(),
CreateExtensionSize<VideoTimingExtension>(),
{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 f7828a4..89ac329 100644
--- a/test/call_test.cc
+++ b/test/call_test.cc
@@ -238,6 +238,8 @@
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 0835777..12ae8e8 100644
--- a/test/constants.cc
+++ b/test/constants.cc
@@ -20,6 +20,7 @@
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 85e8c18..3cee828 100644
--- a/test/constants.h
+++ b/test/constants.h
@@ -18,5 +18,6 @@
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 31742ee..5c52555 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<std::string> {
public:
- CodecEndToEndTest() = default;
+ CodecEndToEndTest() : field_trial_(GetParam()) {}
+
+ private:
+ test::ScopedFieldTrials field_trial_;
};
class CodecObserver : public test::EndToEndTest,
@@ -89,7 +93,12 @@
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 @@
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 @@
}
#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 @@
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 @@
}
// 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 @@
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 804fbcc..ec1d59e 100644
--- a/video/screenshare_loopback.cc
+++ b/video/screenshare_loopback.cc
@@ -220,6 +220,8 @@
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 @@
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 0555fb3..25bfa01 100644
--- a/video/sv_loopback.cc
+++ b/video/sv_loopback.cc
@@ -419,6 +419,8 @@
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 @@
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 52f0a65..71a86f7 100644
--- a/video/video_loopback.cc
+++ b/video/video_loopback.cc
@@ -235,6 +235,8 @@
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 @@
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 7f7c873..cd4c5f7 100644
--- a/video/video_quality_test.cc
+++ b/video/video_quality_test.cc
@@ -214,7 +214,7 @@
}
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 @@
}
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;
