admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Enable FlexFEC as a receiver video codec by default

Browse Source 
- 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}
This commit is contained in:
Harsh Maniar 2020-11-17 12:51:38 -08:00 committed by Commit Bot
parent b06aa5a4ce
commit f08db1be94
4 changed files with 49 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
media/engine/webrtc_video_engine.cc
View File

@ -67,6 +67,11 @@ bool IsEnabled(const webrtc::WebRtcKeyValueConfig& trials,
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) {
return (value > 0) && ((value & (value - 1)) == 0);
}
@ -107,7 +112,8 @@ void AddDefaultFeedbackParams(VideoCodec* codec,
// default feedback params to the codecs.
std::vector<VideoCodec> AssignPayloadTypesAndDefaultCodecs(
std::vector<webrtc::SdpVideoFormat> input_formats,
const webrtc::WebRtcKeyValueConfig& trials) {
const webrtc::WebRtcKeyValueConfig& trials,
bool is_decoder_factory) {
if (input_formats.empty())
return std::vector<VideoCodec>();
static const int kFirstDynamicPayloadType = 96;
@ -117,7 +123,13 @@ std::vector<VideoCodec> AssignPayloadTypesAndDefaultCodecs(
input_formats.push_back(webrtc::SdpVideoFormat(kRedCodecName));
input_formats.push_back(webrtc::SdpVideoFormat(kUlpfecCodecName));
if (IsEnabled(trials, "WebRTC-FlexFEC-03-Advertised")) {
// flexfec-03 is supported as
// - 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);
// This value is currently arbitrarily set to 10 seconds. (The unit
// is microseconds.) This parameter MUST be present in the SDP, but
@ -160,7 +172,9 @@ std::vector<VideoCodec> AssignPayloadTypesAndDefaultCodecs(
// is_decoder_factory is needed to keep track of the implict assumption that any
// H264 decoder also supports constrained base line profile.
// TODO(kron): Perhaps it better to move the implcit knowledge to the place
// Also, is_decoder_factory is used to decide whether FlexFEC video format
// should be advertised as supported.
// TODO(kron): Perhaps it is better to move the implicit knowledge to the place
// where codecs are negotiated.
template <class T>
std::vector<VideoCodec> GetPayloadTypesAndDefaultCodecs(
@ -178,7 +192,7 @@ std::vector<VideoCodec> GetPayloadTypesAndDefaultCodecs(
}
return AssignPayloadTypesAndDefaultCodecs(std::move(supported_formats),
trials);
trials, is_decoder_factory);
}
bool IsTemporalLayersSupported(const std::string& codec_name) {
@ -1499,7 +1513,7 @@ void WebRtcVideoChannel::ConfigureReceiverRtp(
// TODO(brandtr): Generalize when we add support for multistream protection.
flexfec_config->payload_type = recv_flexfec_payload_type_;
if (IsEnabled(call_->trials(), "WebRTC-FlexFEC-03-Advertised") &&
if (!IsDisabled(call_->trials(), "WebRTC-FlexFEC-03-Advertised") &&
sp.GetFecFrSsrc(ssrc, &flexfec_config->remote_ssrc)) {
flexfec_config->protected_media_ssrcs = {ssrc};
flexfec_config->local_ssrc = config->rtp.local_ssrc;

30
media/engine/webrtc_video_engine_unittest.cc
View File

@ -951,26 +951,36 @@ TEST_F(WebRtcVideoEngineTest, SimulcastEnabledForH264BehindFieldTrial) {
EXPECT_TRUE(channel->SetVideoSend(ssrcs[0], nullptr, nullptr));
}
// Test that the FlexFEC field trial properly alters the output of
// WebRtcVideoEngine::codecs(), for an existing |engine_| object.
//
// TODO(brandtr): Remove this test, when the FlexFEC field trial is gone.
TEST_F(WebRtcVideoEngineTest,
Flexfec03SupportedAsInternalCodecBehindFieldTrial) {
// Test that FlexFEC is not supported as a send video codec by default.
// Only enabling field trial should allow advertising FlexFEC send codec.
TEST_F(WebRtcVideoEngineTest, Flexfec03SendCodecEnablesWithFieldTrial) {
encoder_factory_->AddSupportedVideoCodecType("VP8");
auto flexfec = Field("name", &VideoCodec::name, "flexfec-03");
// FlexFEC is not active without field trial.
EXPECT_THAT(engine_.send_codecs(), Not(Contains(flexfec)));
// FlexFEC is active with field trial.
RTC_DCHECK(!override_field_trials_);
override_field_trials_ = std::make_unique<webrtc::test::ScopedFieldTrials>(
"WebRTC-FlexFEC-03-Advertised/Enabled/");
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_F(WebRtcVideoEngineTest, ReportSupportedCodecs) {
encoder_factory_->AddSupportedVideoCodecType("VP8");
@ -4017,13 +4027,13 @@ TEST_F(WebRtcVideoChannelTest, FlexfecRecvCodecWithoutSsrcNotExposedByDefault) {
EXPECT_TRUE(streams.empty());
}
TEST_F(WebRtcVideoChannelTest, FlexfecRecvCodecWithSsrcNotExposedByDefault) {
TEST_F(WebRtcVideoChannelTest, FlexfecRecvCodecWithSsrcExposedByDefault) {
AddRecvStream(
CreatePrimaryWithFecFrStreamParams("cname", kSsrcs1[0], kFlexfecSsrc));
const std::vector<FakeFlexfecReceiveStream*>& streams =
fake_call_->GetFlexfecReceiveStreams();
EXPECT_TRUE(streams.empty());
EXPECT_EQ(1U, streams.size());
}
// TODO(brandtr): When FlexFEC is no longer behind a field trial, merge all

6
pc/peer_connection_end_to_end_unittest.cc
View File

@ -21,6 +21,7 @@
#include "media/sctp/sctp_transport_internal.h"
#include "rtc_base/gunit.h"
#include "rtc_base/logging.h"
#include "test/field_trial.h"
#ifdef WEBRTC_ANDROID
#include "pc/test/android_test_initializer.h"
@ -428,6 +429,11 @@ TEST_P(PeerConnectionEndToEndTest, CallWithCustomCodec) {
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,
decoder_id2;
CreatePcs(rtc::scoped_refptr<webrtc::AudioEncoderFactory>(

4
pc/peer_connection_integrationtest.cc
View File

@ -2021,6 +2021,10 @@ TEST_P(PeerConnectionIntegrationTest, EndToEndCallWithSendOnlyVideo) {
// Tests that receive only works without the caller having an encoder factory
// and the callee having a decoder factory.
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(
CreateOneDirectionalPeerConnectionWrappers(/*caller_to_callee=*/false));
ConnectFakeSignaling();