Use different RTX payload types for different H264 profiles

Use different RTX payload types for different H264 profiles

This CL is a quick fix to unblock H264 High Profile. This CL is supposed
to be superseded by a proper fix of
https://bugs.chromium.org/p/webrtc/issues/detail?id=6705 like
https://codereview.webrtc.org/2493133002/.

BUG=webrtc:6677
Committed: https://crrev.com/725e484e33ec92daf4d3c722a0487f8cb244b637
Cr-Commit-Position: refs/heads/master@{#15099}

[magjed_webrtc, 2016-11-13 15:38:53]

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

[commit-bot: I haz the power, 2016-11-13 15:38:56]

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, 2016-11-13 15:48:06]

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

[commit-bot: I haz the power, 2016-11-13 15:48:07]

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

[magjed_webrtc, 2016-11-14 08:47:09]

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

[commit-bot: I haz the power, 2016-11-14 08:47:16]

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

[magjed_webrtc, 2016-11-14 08:48:19]

Patchset #1 (id:1) has been deleted

[magjed_webrtc, 2016-11-14 08:59:09]

magjed@webrtc.org changed reviewers:
+ hta@webrtc.org

[magjed_webrtc, 2016-11-14 08:59:09]

Harald - please take a look.

[commit-bot: I haz the power, 2016-11-14 09:19:08]

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

[commit-bot: I haz the power, 2016-11-14 09:19:09]

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

[hta-webrtc, 2016-11-15 10:41:07]

Even for a quick fix, I can't let this go through without unit tests that
exercise all 3 cases (constrained baseline, constrained high and
none-of-the-above).

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

https://codereview.webrtc.org/2497773003/diff/20001/webrtc/media/engine/webrt...
webrtc/media/engine/webrtcvideoengine2.cc:413: // Only supported rtx for
constrained baseline and constrained high profile.
Grammar nit: "In H.264, we only support rtx for constrained baseline and
constrained high profile."

https://codereview.webrtc.org/2497773003/diff/20001/webrtc/media/engine/webrt...
webrtc/media/engine/webrtcvideoengine2.cc:418: }
"Else" missing - if this is neither of the two above, it will fall through to
the "codecs->push_back" at the bottom with rtx_payload_type uninitialized.

I'm assuming the "else" should be "return" like the others.

Please add an unit test covering a profile for which RTX is not supported.

[magjed_webrtc, 2016-11-15 17:47:57]

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

[commit-bot: I haz the power, 2016-11-15 17:48:00]

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

[magjed_webrtc, 2016-11-15 17:48:04]

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

https://codereview.webrtc.org/2497773003/diff/20001/webrtc/media/engine/webrt...
webrtc/media/engine/webrtcvideoengine2.cc:413: // Only supported rtx for
constrained baseline and constrained high profile.
On 2016/11/15 10:41:07, hta-webrtc wrote:
> Grammar nit: "In H.264, we only support rtx for constrained baseline and
> constrained high profile."

Done.

https://codereview.webrtc.org/2497773003/diff/20001/webrtc/media/engine/webrt...
webrtc/media/engine/webrtcvideoengine2.cc:418: }
On 2016/11/15 10:41:07, hta-webrtc wrote:
> "Else" missing - if this is neither of the two above, it will fall through to
> the "codecs->push_back" at the bottom with rtx_payload_type uninitialized.
> 
> I'm assuming the "else" should be "return" like the others.
> 
> Please add an unit test covering a profile for which RTX is not supported.

Done.

[commit-bot: I haz the power, 2016-11-15 17:54:50]

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

[commit-bot: I haz the power, 2016-11-15 17:54:51]

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

[hta-webrtc, 2016-11-16 00:47:53]

lgtm, happy!

[magjed_webrtc, 2016-11-16 08:35:29]

The CQ bit was checked by magjed@webrtc.org

[commit-bot: I haz the power, 2016-11-16 08:35:37]

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, 2016-11-16 08:48:15]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2016-11-16 08:48:30]

Description was changed from

==========
Use different RTX payload types for different H264 profiles

This CL is a quick fix to unblock H264 High Profile. This CL is supposed
to be superseded by a proper fix of
https://bugs.chromium.org/p/webrtc/issues/detail?id=6705 like
https://codereview.webrtc.org/2493133002/.

BUG=webrtc:6677
==========

to

==========
Use different RTX payload types for different H264 profiles

This CL is a quick fix to unblock H264 High Profile. This CL is supposed
to be superseded by a proper fix of
https://bugs.chromium.org/p/webrtc/issues/detail?id=6705 like
https://codereview.webrtc.org/2493133002/.

BUG=webrtc:6677

Committed: https://crrev.com/725e484e33ec92daf4d3c722a0487f8cb244b637
Cr-Commit-Position: refs/heads/master@{#15099}
==========

[commit-bot: I haz the power, 2016-11-16 08:48:33]

Patchset 2 (id:??) landed as
https://crrev.com/725e484e33ec92daf4d3c722a0487f8cb244b637
Cr-Commit-Position: refs/heads/master@{#15099}