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}
This commit is contained in:
philipel 2018-09-26 12:25:31 +02:00 committed by Commit Bot
parent d5bc865f13
commit 569397fec7
14 changed files with 72 additions and 25 deletions

View File

@ -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.

View File

@ -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;
}

View File

@ -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_

View File

@ -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_;
};

View File

@ -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<const uint8_t> data,
RtpGenericFrameDescriptor* descriptor);

View File

@ -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<VideoContentTypeExtension>(),
CreateExtensionSize<VideoTimingExtension>(),
{RtpMid::kId, RtpMid::kMaxValueSizeBytes},
{RtpGenericFrameDescriptorExtension::kId,
RtpGenericFrameDescriptorExtension::kMaxSizeBytes},
};
const char* FrameTypeToString(FrameType frame_type) {

View File

@ -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,

View File

@ -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

View File

@ -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

View File

@ -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 @@ 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(

View File

@ -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(),

View File

@ -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,

View File

@ -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(),

View File

@ -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;