From 36e7d704100d632799f32344fb125e2944172c78 Mon Sep 17 00:00:00 2001 From: brandtr Date: Fri, 13 Jan 2017 07:15:54 -0800 Subject: [PATCH] Explicitly only add transport-cc RTCP feedback param to default FlexFEC codec. Earlier, the FlexFEC codec would receive the same default RTCP feedback params as the media codecs. Since most of these are not used, there is no point negotiating them. BUG=webrtc:5654 Review-Url: https://codereview.webrtc.org/2623513002 Cr-Commit-Position: refs/heads/master@{#16057} --- webrtc/media/engine/internalencoderfactory.cc | 4 ++++ webrtc/media/engine/webrtcvideoengine2.cc | 5 +++-- webrtc/media/engine/webrtcvideoengine2_unittest.cc | 8 ++++++++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/webrtc/media/engine/internalencoderfactory.cc b/webrtc/media/engine/internalencoderfactory.cc index ec9837579b..ad3a4be0c6 100644 --- a/webrtc/media/engine/internalencoderfactory.cc +++ b/webrtc/media/engine/internalencoderfactory.cc @@ -53,6 +53,10 @@ InternalEncoderFactory::InternalEncoderFactory() { // we never use the actual value anywhere in our code however. // TODO(brandtr): Consider honouring this value in the sender and receiver. flexfec_codec.SetParam(kFlexfecFmtpRepairWindow, "10000000"); + flexfec_codec.AddFeedbackParam( + FeedbackParam(kRtcpFbParamTransportCc, kParamValueEmpty)); + flexfec_codec.AddFeedbackParam( + FeedbackParam(kRtcpFbParamRemb, kParamValueEmpty)); supported_codecs_.push_back(flexfec_codec); } } diff --git a/webrtc/media/engine/webrtcvideoengine2.cc b/webrtc/media/engine/webrtcvideoengine2.cc index ca65e6541e..de0e2d5078 100644 --- a/webrtc/media/engine/webrtcvideoengine2.cc +++ b/webrtc/media/engine/webrtcvideoengine2.cc @@ -571,7 +571,7 @@ static rtc::Optional NextFreePayloadType( // This is a helper function for GetSupportedCodecs below. It will append new // unique codecs from |input_codecs| to |unified_codecs|. It will add default // feedback params to the codecs and will also add an associated RTX codec for -// recognized codecs (VP8, VP9, H264, and Red). +// recognized codecs (VP8, VP9, H264, and RED). static void AppendVideoCodecs(const std::vector& input_codecs, std::vector* unified_codecs) { for (VideoCodec codec : input_codecs) { @@ -582,7 +582,8 @@ static void AppendVideoCodecs(const std::vector& input_codecs, codec.id = *payload_type; // TODO(magjed): Move the responsibility of setting these parameters to the // encoder factories instead. - if (codec.name != kRedCodecName && codec.name != kUlpfecCodecName) + if (codec.name != kRedCodecName && codec.name != kUlpfecCodecName && + codec.name != kFlexfecCodecName) AddDefaultFeedbackParams(&codec); // Don't add same codec twice. if (FindMatchingCodec(*unified_codecs, codec)) diff --git a/webrtc/media/engine/webrtcvideoengine2_unittest.cc b/webrtc/media/engine/webrtcvideoengine2_unittest.cc index 852eedd65c..7d80a0db63 100644 --- a/webrtc/media/engine/webrtcvideoengine2_unittest.cc +++ b/webrtc/media/engine/webrtcvideoengine2_unittest.cc @@ -2367,6 +2367,14 @@ class WebRtcVideoChannel2FlexfecTest : public WebRtcVideoChannel2Test { : WebRtcVideoChannel2Test("WebRTC-FlexFEC-03/Enabled/") {} }; +// TODO(brandtr): Merge into "non-field trial" test when FlexFEC is enabled +// by default. +TEST_F(WebRtcVideoChannel2FlexfecTest, + DefaultFlexfecCodecHasTransportCcAndRembFeedbackParam) { + EXPECT_TRUE(cricket::HasTransportCc(GetEngineCodec("flexfec-03"))); + EXPECT_TRUE(cricket::HasRemb(GetEngineCodec("flexfec-03"))); +} + // TODO(brandtr): Merge into "non-field trial" test when FlexFEC is enabled // by default. TEST_F(WebRtcVideoChannel2FlexfecTest, SetDefaultSendCodecsWithoutSsrc) {