Explicitly only add transport-cc RTCP feedback param to default FlexFEC codec.

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}
Committed: https://chromium.googlesource.com/external/webrtc/+/36e7d704100d632799f32344fb125e2944172c78

[brandtr, 2017-01-09 13:04:16]

Patchset #1 (id:1) has been deleted

[brandtr, 2017-01-09 13:13:21]

brandtr@webrtc.org changed reviewers:
+ magjed@webrtc.org, stefan@webrtc.org

[brandtr, 2017-01-09 13:13:22]

PTAL.

holmer: Verify the thinking.
magjed: Naming and structure in VideoEngine2.

[magjed_webrtc, 2017-01-10 10:46:57]

https://codereview.webrtc.org/2623513002/diff/20001/webrtc/media/engine/webrt...
File webrtc/media/engine/webrtcvideoengine2.cc (right):

https://codereview.webrtc.org/2623513002/diff/20001/webrtc/media/engine/webrt...
webrtc/media/engine/webrtcvideoengine2.cc:593:
AddDefaultFlexfecFeedbackParams(&codec);
Like the TODO above says, we eventually want this moved into the codec factories
instead. So I would prefer if you move the flexfec feedback params to the
InternalEncoderFactory instead, and only add 'codec.name != kFlexfecCodecName'
to the if-statement here.

[stefan-webrtc, 2017-01-10 11:38:56]

https://codereview.webrtc.org/2623513002/diff/20001/webrtc/media/engine/webrt...
File webrtc/media/engine/webrtcvideoengine2.cc (right):

https://codereview.webrtc.org/2623513002/diff/20001/webrtc/media/engine/webrt...
webrtc/media/engine/webrtcvideoengine2.cc:162:
FeedbackParam(kRtcpFbParamTransportCc, kParamValueEmpty));
I think you'd want to add remb here as well in case of receive-side bwe.

[brandtr, 2017-01-10 12:33:51]

https://codereview.webrtc.org/2623513002/diff/20001/webrtc/media/engine/webrt...
File webrtc/media/engine/webrtcvideoengine2.cc (right):

https://codereview.webrtc.org/2623513002/diff/20001/webrtc/media/engine/webrt...
webrtc/media/engine/webrtcvideoengine2.cc:162:
FeedbackParam(kRtcpFbParamTransportCc, kParamValueEmpty));
On 2017/01/10 11:38:56, stefan-webrtc wrote:
> I think you'd want to add remb here as well in case of receive-side bwe.

That's right. Done.

https://codereview.webrtc.org/2623513002/diff/20001/webrtc/media/engine/webrt...
webrtc/media/engine/webrtcvideoengine2.cc:593:
AddDefaultFlexfecFeedbackParams(&codec);
On 2017/01/10 10:46:57, magjed_webrtc wrote:
> Like the TODO above says, we eventually want this moved into the codec
factories
> instead. So I would prefer if you move the flexfec feedback params to the
> InternalEncoderFactory instead, and only add 'codec.name != kFlexfecCodecName'
> to the if-statement here.

Right, should have looked more carefully. Done.

[magjed_webrtc, 2017-01-10 13:08:27]

lgtm

[brandtr, 2017-01-12 15:05:56]

Rebase.

[stefan-webrtc, 2017-01-12 16:06:20]

lgtm

[brandtr, 2017-01-13 12:13:45]

The CQ bit was checked by brandtr@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-13 12:13:58]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-01-13 13:21:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-13 13:21:38]

Dry run: Try jobs failed on following builders:
  linux_asan on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_asan/builds/21033)

[brandtr, 2017-01-13 13:32:50]

The CQ bit was checked by brandtr@webrtc.org

[brandtr, 2017-01-13 13:32:51]

The patchset sent to the CQ was uploaded after l-g-t-m from magjed@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2623513002/#ps60001 (title:
"Rebase.")

[commit-bot: I haz the power, 2017-01-13 13:33:05]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-01-13 15:15:54]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1484314370969210,
"parent_rev": "bf0845201a3850a0aad9a11c974162e39ac897bc", "commit_rev":
"36e7d704100d632799f32344fb125e2944172c78"}

[commit-bot: I haz the power, 2017-01-13 15:15:57]

Description was changed from

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

to

==========
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}
Committed:
https://chromium.googlesource.com/external/webrtc/+/36e7d704100d632799f32344f...
==========

[commit-bot: I haz the power, 2017-01-13 15:15:58]

Committed patchset #3 (id:60001) as
https://chromium.googlesource.com/external/webrtc/+/36e7d704100d632799f32344f...