Revert "Enable FlexFEC as a receiver video codec by default"
This reverts commit f08db1be94e760c201acdc3a121e67453960c970. Reason for revert: It looks like this breaks Chromium FYI Windows bots. See https://ci.chromium.org/p/chromium/builders/webrtc.fyi/WebRTC%20Chromium%20FYI%20Win10%20Tester/6988. If this is not the culprit I will reland. Original change's description: > Enable FlexFEC as a receiver video codec by default > > - Add Flex FEC format as default supported receive codec > - Disallow advertising FlexFEC as video sender codec by default until implementation is complete > - Toggle field trial "WebRTC-FlexFEC-03-Advertised"s behavior for receiver to use as kill-switch to prevent codec advertising > > Bug: webrtc:8151 > Change-Id: Iff367119263496fb335500e96641669654b45834 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/191947 > Commit-Queue: Christoffer Rodbro <crodbro@webrtc.org> > Reviewed-by: Ying Wang <yinwa@webrtc.org> > Reviewed-by: Christoffer Rodbro <crodbro@webrtc.org> > Reviewed-by: Stefan Holmer <stefan@webrtc.org> > Cr-Commit-Position: refs/heads/master@{#32639} TBR=brandtr@webrtc.org,tommi@webrtc.org,stefan@webrtc.org,crodbro@webrtc.org,crodbro@google.com,yinwa@webrtc.org,philipp.hancke@googlemail.com,hmaniar@nvidia.com # Not skipping CQ checks because original CL landed > 1 day ago. Bug: webrtc:8151 Change-Id: Ia1788a1cf34e0fc9500a081552f6ed03d0995d5b Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/194334 Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org> Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org> Cr-Commit-Position: refs/heads/master@{#32657}
This commit is contained in:
parent
b223cb60e9
commit
ce4be1e640
@ -67,11 +67,6 @@ bool IsEnabled(const webrtc::WebRtcKeyValueConfig& trials,
|
|||||||
return absl::StartsWith(trials.Lookup(name), "Enabled");
|
return absl::StartsWith(trials.Lookup(name), "Enabled");
|
||||||
}
|
}
|
||||||
|
|
||||||
bool IsDisabled(const webrtc::WebRtcKeyValueConfig& trials,
|
|
||||||
absl::string_view name) {
|
|
||||||
return absl::StartsWith(trials.Lookup(name), "Disabled");
|
|
||||||
}
|
|
||||||
|
|
||||||
bool PowerOfTwo(int value) {
|
bool PowerOfTwo(int value) {
|
||||||
return (value > 0) && ((value & (value - 1)) == 0);
|
return (value > 0) && ((value & (value - 1)) == 0);
|
||||||
}
|
}
|
||||||
@ -112,8 +107,7 @@ void AddDefaultFeedbackParams(VideoCodec* codec,
|
|||||||
// default feedback params to the codecs.
|
// default feedback params to the codecs.
|
||||||
std::vector<VideoCodec> AssignPayloadTypesAndDefaultCodecs(
|
std::vector<VideoCodec> AssignPayloadTypesAndDefaultCodecs(
|
||||||
std::vector<webrtc::SdpVideoFormat> input_formats,
|
std::vector<webrtc::SdpVideoFormat> input_formats,
|
||||||
const webrtc::WebRtcKeyValueConfig& trials,
|
const webrtc::WebRtcKeyValueConfig& trials) {
|
||||||
bool is_decoder_factory) {
|
|
||||||
if (input_formats.empty())
|
if (input_formats.empty())
|
||||||
return std::vector<VideoCodec>();
|
return std::vector<VideoCodec>();
|
||||||
static const int kFirstDynamicPayloadType = 96;
|
static const int kFirstDynamicPayloadType = 96;
|
||||||
@ -123,13 +117,7 @@ std::vector<VideoCodec> AssignPayloadTypesAndDefaultCodecs(
|
|||||||
input_formats.push_back(webrtc::SdpVideoFormat(kRedCodecName));
|
input_formats.push_back(webrtc::SdpVideoFormat(kRedCodecName));
|
||||||
input_formats.push_back(webrtc::SdpVideoFormat(kUlpfecCodecName));
|
input_formats.push_back(webrtc::SdpVideoFormat(kUlpfecCodecName));
|
||||||
|
|
||||||
// flexfec-03 is supported as
|
if (IsEnabled(trials, "WebRTC-FlexFEC-03-Advertised")) {
|
||||||
// - receive codec unless WebRTC-FlexFEC-03-Advertised is disabled
|
|
||||||
// - send codec if WebRTC-FlexFEC-03-Advertised is enabled
|
|
||||||
if ((is_decoder_factory &&
|
|
||||||
!IsDisabled(trials, "WebRTC-FlexFEC-03-Advertised")) ||
|
|
||||||
(!is_decoder_factory &&
|
|
||||||
IsEnabled(trials, "WebRTC-FlexFEC-03-Advertised"))) {
|
|
||||||
webrtc::SdpVideoFormat flexfec_format(kFlexfecCodecName);
|
webrtc::SdpVideoFormat flexfec_format(kFlexfecCodecName);
|
||||||
// This value is currently arbitrarily set to 10 seconds. (The unit
|
// This value is currently arbitrarily set to 10 seconds. (The unit
|
||||||
// is microseconds.) This parameter MUST be present in the SDP, but
|
// is microseconds.) This parameter MUST be present in the SDP, but
|
||||||
@ -172,9 +160,7 @@ std::vector<VideoCodec> AssignPayloadTypesAndDefaultCodecs(
|
|||||||
|
|
||||||
// is_decoder_factory is needed to keep track of the implict assumption that any
|
// is_decoder_factory is needed to keep track of the implict assumption that any
|
||||||
// H264 decoder also supports constrained base line profile.
|
// H264 decoder also supports constrained base line profile.
|
||||||
// Also, is_decoder_factory is used to decide whether FlexFEC video format
|
// TODO(kron): Perhaps it better to move the implcit knowledge to the place
|
||||||
// should be advertised as supported.
|
|
||||||
// TODO(kron): Perhaps it is better to move the implicit knowledge to the place
|
|
||||||
// where codecs are negotiated.
|
// where codecs are negotiated.
|
||||||
template <class T>
|
template <class T>
|
||||||
std::vector<VideoCodec> GetPayloadTypesAndDefaultCodecs(
|
std::vector<VideoCodec> GetPayloadTypesAndDefaultCodecs(
|
||||||
@ -192,7 +178,7 @@ std::vector<VideoCodec> GetPayloadTypesAndDefaultCodecs(
|
|||||||
}
|
}
|
||||||
|
|
||||||
return AssignPayloadTypesAndDefaultCodecs(std::move(supported_formats),
|
return AssignPayloadTypesAndDefaultCodecs(std::move(supported_formats),
|
||||||
trials, is_decoder_factory);
|
trials);
|
||||||
}
|
}
|
||||||
|
|
||||||
bool IsTemporalLayersSupported(const std::string& codec_name) {
|
bool IsTemporalLayersSupported(const std::string& codec_name) {
|
||||||
@ -1513,7 +1499,7 @@ void WebRtcVideoChannel::ConfigureReceiverRtp(
|
|||||||
|
|
||||||
// TODO(brandtr): Generalize when we add support for multistream protection.
|
// TODO(brandtr): Generalize when we add support for multistream protection.
|
||||||
flexfec_config->payload_type = recv_flexfec_payload_type_;
|
flexfec_config->payload_type = recv_flexfec_payload_type_;
|
||||||
if (!IsDisabled(call_->trials(), "WebRTC-FlexFEC-03-Advertised") &&
|
if (IsEnabled(call_->trials(), "WebRTC-FlexFEC-03-Advertised") &&
|
||||||
sp.GetFecFrSsrc(ssrc, &flexfec_config->remote_ssrc)) {
|
sp.GetFecFrSsrc(ssrc, &flexfec_config->remote_ssrc)) {
|
||||||
flexfec_config->protected_media_ssrcs = {ssrc};
|
flexfec_config->protected_media_ssrcs = {ssrc};
|
||||||
flexfec_config->local_ssrc = config->rtp.local_ssrc;
|
flexfec_config->local_ssrc = config->rtp.local_ssrc;
|
||||||
|
|||||||
@ -951,36 +951,26 @@ TEST_F(WebRtcVideoEngineTest, SimulcastEnabledForH264BehindFieldTrial) {
|
|||||||
EXPECT_TRUE(channel->SetVideoSend(ssrcs[0], nullptr, nullptr));
|
EXPECT_TRUE(channel->SetVideoSend(ssrcs[0], nullptr, nullptr));
|
||||||
}
|
}
|
||||||
|
|
||||||
// Test that FlexFEC is not supported as a send video codec by default.
|
// Test that the FlexFEC field trial properly alters the output of
|
||||||
// Only enabling field trial should allow advertising FlexFEC send codec.
|
// WebRtcVideoEngine::codecs(), for an existing |engine_| object.
|
||||||
TEST_F(WebRtcVideoEngineTest, Flexfec03SendCodecEnablesWithFieldTrial) {
|
//
|
||||||
|
// TODO(brandtr): Remove this test, when the FlexFEC field trial is gone.
|
||||||
|
TEST_F(WebRtcVideoEngineTest,
|
||||||
|
Flexfec03SupportedAsInternalCodecBehindFieldTrial) {
|
||||||
encoder_factory_->AddSupportedVideoCodecType("VP8");
|
encoder_factory_->AddSupportedVideoCodecType("VP8");
|
||||||
|
|
||||||
auto flexfec = Field("name", &VideoCodec::name, "flexfec-03");
|
auto flexfec = Field("name", &VideoCodec::name, "flexfec-03");
|
||||||
|
|
||||||
|
// FlexFEC is not active without field trial.
|
||||||
EXPECT_THAT(engine_.send_codecs(), Not(Contains(flexfec)));
|
EXPECT_THAT(engine_.send_codecs(), Not(Contains(flexfec)));
|
||||||
|
|
||||||
|
// FlexFEC is active with field trial.
|
||||||
RTC_DCHECK(!override_field_trials_);
|
RTC_DCHECK(!override_field_trials_);
|
||||||
override_field_trials_ = std::make_unique<webrtc::test::ScopedFieldTrials>(
|
override_field_trials_ = std::make_unique<webrtc::test::ScopedFieldTrials>(
|
||||||
"WebRTC-FlexFEC-03-Advertised/Enabled/");
|
"WebRTC-FlexFEC-03-Advertised/Enabled/");
|
||||||
EXPECT_THAT(engine_.send_codecs(), Contains(flexfec));
|
EXPECT_THAT(engine_.send_codecs(), Contains(flexfec));
|
||||||
}
|
}
|
||||||
|
|
||||||
// Test that FlexFEC is supported as a receive video codec by default.
|
|
||||||
// Disabling field trial should prevent advertising FlexFEC receive codec.
|
|
||||||
TEST_F(WebRtcVideoEngineTest, Flexfec03ReceiveCodecDisablesWithFieldTrial) {
|
|
||||||
decoder_factory_->AddSupportedVideoCodecType("VP8");
|
|
||||||
|
|
||||||
auto flexfec = Field("name", &VideoCodec::name, "flexfec-03");
|
|
||||||
|
|
||||||
EXPECT_THAT(engine_.recv_codecs(), Contains(flexfec));
|
|
||||||
|
|
||||||
RTC_DCHECK(!override_field_trials_);
|
|
||||||
override_field_trials_ = std::make_unique<webrtc::test::ScopedFieldTrials>(
|
|
||||||
"WebRTC-FlexFEC-03-Advertised/Disabled/");
|
|
||||||
EXPECT_THAT(engine_.recv_codecs(), Not(Contains(flexfec)));
|
|
||||||
}
|
|
||||||
|
|
||||||
// Test that codecs are added in the order they are reported from the factory.
|
// Test that codecs are added in the order they are reported from the factory.
|
||||||
TEST_F(WebRtcVideoEngineTest, ReportSupportedCodecs) {
|
TEST_F(WebRtcVideoEngineTest, ReportSupportedCodecs) {
|
||||||
encoder_factory_->AddSupportedVideoCodecType("VP8");
|
encoder_factory_->AddSupportedVideoCodecType("VP8");
|
||||||
@ -4027,13 +4017,13 @@ TEST_F(WebRtcVideoChannelTest, FlexfecRecvCodecWithoutSsrcNotExposedByDefault) {
|
|||||||
EXPECT_TRUE(streams.empty());
|
EXPECT_TRUE(streams.empty());
|
||||||
}
|
}
|
||||||
|
|
||||||
TEST_F(WebRtcVideoChannelTest, FlexfecRecvCodecWithSsrcExposedByDefault) {
|
TEST_F(WebRtcVideoChannelTest, FlexfecRecvCodecWithSsrcNotExposedByDefault) {
|
||||||
AddRecvStream(
|
AddRecvStream(
|
||||||
CreatePrimaryWithFecFrStreamParams("cname", kSsrcs1[0], kFlexfecSsrc));
|
CreatePrimaryWithFecFrStreamParams("cname", kSsrcs1[0], kFlexfecSsrc));
|
||||||
|
|
||||||
const std::vector<FakeFlexfecReceiveStream*>& streams =
|
const std::vector<FakeFlexfecReceiveStream*>& streams =
|
||||||
fake_call_->GetFlexfecReceiveStreams();
|
fake_call_->GetFlexfecReceiveStreams();
|
||||||
EXPECT_EQ(1U, streams.size());
|
EXPECT_TRUE(streams.empty());
|
||||||
}
|
}
|
||||||
|
|
||||||
// TODO(brandtr): When FlexFEC is no longer behind a field trial, merge all
|
// TODO(brandtr): When FlexFEC is no longer behind a field trial, merge all
|
||||||
|
|||||||
@ -21,7 +21,6 @@
|
|||||||
#include "media/sctp/sctp_transport_internal.h"
|
#include "media/sctp/sctp_transport_internal.h"
|
||||||
#include "rtc_base/gunit.h"
|
#include "rtc_base/gunit.h"
|
||||||
#include "rtc_base/logging.h"
|
#include "rtc_base/logging.h"
|
||||||
#include "test/field_trial.h"
|
|
||||||
|
|
||||||
#ifdef WEBRTC_ANDROID
|
#ifdef WEBRTC_ANDROID
|
||||||
#include "pc/test/android_test_initializer.h"
|
#include "pc/test/android_test_initializer.h"
|
||||||
@ -429,11 +428,6 @@ TEST_P(PeerConnectionEndToEndTest, CallWithCustomCodec) {
|
|||||||
std::vector<webrtc::AudioCodecPairId>* const codec_ids_;
|
std::vector<webrtc::AudioCodecPairId>* const codec_ids_;
|
||||||
};
|
};
|
||||||
|
|
||||||
// Disable advertising FlexFEC as receive codec to avoid running out of unique
|
|
||||||
// payload types. See bugs.webrtc.org/12194
|
|
||||||
webrtc::test::ScopedFieldTrials field_trials(
|
|
||||||
"WebRTC-FlexFEC-03-Advertised/Disabled/");
|
|
||||||
|
|
||||||
std::vector<webrtc::AudioCodecPairId> encoder_id1, encoder_id2, decoder_id1,
|
std::vector<webrtc::AudioCodecPairId> encoder_id1, encoder_id2, decoder_id1,
|
||||||
decoder_id2;
|
decoder_id2;
|
||||||
CreatePcs(rtc::scoped_refptr<webrtc::AudioEncoderFactory>(
|
CreatePcs(rtc::scoped_refptr<webrtc::AudioEncoderFactory>(
|
||||||
|
|||||||
@ -2021,10 +2021,6 @@ TEST_P(PeerConnectionIntegrationTest, EndToEndCallWithSendOnlyVideo) {
|
|||||||
// Tests that receive only works without the caller having an encoder factory
|
// Tests that receive only works without the caller having an encoder factory
|
||||||
// and the callee having a decoder factory.
|
// and the callee having a decoder factory.
|
||||||
TEST_P(PeerConnectionIntegrationTest, EndToEndCallWithReceiveOnlyVideo) {
|
TEST_P(PeerConnectionIntegrationTest, EndToEndCallWithReceiveOnlyVideo) {
|
||||||
// Disable advertising FlexFEC as receive codec to avoid running out of unique
|
|
||||||
// payload types. See bugs.webrtc.org/12194
|
|
||||||
webrtc::test::ScopedFieldTrials field_trials(
|
|
||||||
"WebRTC-FlexFEC-03-Advertised/Disabled/");
|
|
||||||
ASSERT_TRUE(
|
ASSERT_TRUE(
|
||||||
CreateOneDirectionalPeerConnectionWrappers(/*caller_to_callee=*/false));
|
CreateOneDirectionalPeerConnectionWrappers(/*caller_to_callee=*/false));
|
||||||
ConnectFakeSignaling();
|
ConnectFakeSignaling();
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user