Support multiple rtx codecs.

Support multiple rtx codecs.

Adds negotiation of rtx codecs for red and vp9. To keep backwards
compatibility with older Chrome versions, this change includes two
hacks:
1. Red packets will be retransmitted over the rtx codec associated with
   vp8 if no rtx codec is associated with red. This is how Chrome does
   it today and ensures that we still can send red over rtx to older
   versions.

2. If rtx packets associated with the media codec (vp8/vp9 etc) are
   received and red has been negotiated, we will assume that the sender
   incorrectly has packetized red inside the rtx header associated with
   media. We will therefore restore it with the red payload type
   instead, which ensures that we can still receive rtx associated with
   red from old versions.

Offering multiple rtx codecs to older versions should not be a problem
since old versions themselves only try to negotiate rtx for vp8.

R=pbos@webrtc.org
TBR=mflodman@webrtc.org
BUG=webrtc:4024
TEST=Verified by running apprtc and emulating packet loss between Chrome with and without the patch.

Committed: https://crrev.com/10880011d9f116a9aff87436de231151e355ac7d
Cr-Commit-Position: refs/heads/master@{#11472}

[stefan-webrtc, 2016-01-28 16:38:54]

stefan@webrtc.org changed reviewers:
+ mflodman@webrtc.org, pbos@webrtc.org

[stefan-webrtc, 2016-01-28 16:38:54]

PTAL

I'm still considering what tests I can add to make sure the interop doesn't
break.

[stefan-webrtc, 2016-01-28 18:56:44]

Remove some more code.

[stefan-webrtc, 2016-01-28 19:22:58]

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

[commit-bot: I haz the power, 2016-01-28 19:23:00]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1649493004/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1649493004/40001

[stefan-webrtc, 2016-01-28 19:47:21]

Description was changed from

==========
Support multiple rtx codecs.

Adds negotiation of rtx codecs for red and vp9. To keep backwards
compatibility with older Chrome versions, this change includes two
hacks:
1. Red packets will be retransmitted over the rtx codec associated with
   vp8 if no rtx codec is associated with red. This is how Chrome does
   it today and ensures that we still can send red over rtx to older
   versions.

2. If rtx packets associated with the media codec (vp8/vp9 etc) are
   received and red has been negotiated, we will assume that the sender
   incorrectly has packetized red inside the rtx header associated with
   media. We will therefore restore it with the red payload type
   instead, which ensures that we can still receive rtx associated with
   red from old versions.

Offering multiple rtx codecs to older versions should not be a problem
since old versions themselves only tries to negotiate rtx for vp8.

BUG=webrtc:4024
==========

to

==========
Support multiple rtx codecs.

Adds negotiation of rtx codecs for red and vp9. To keep backwards
compatibility with older Chrome versions, this change includes two
hacks:
1. Red packets will be retransmitted over the rtx codec associated with
   vp8 if no rtx codec is associated with red. This is how Chrome does
   it today and ensures that we still can send red over rtx to older
   versions.

2. If rtx packets associated with the media codec (vp8/vp9 etc) are
   received and red has been negotiated, we will assume that the sender
   incorrectly has packetized red inside the rtx header associated with
   media. We will therefore restore it with the red payload type
   instead, which ensures that we can still receive rtx associated with
   red from old versions.

Offering multiple rtx codecs to older versions should not be a problem
since old versions themselves only try to negotiate rtx for vp8.

BUG=webrtc:4024
==========

[commit-bot: I haz the power, 2016-01-28 20:36:16]

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

[commit-bot: I haz the power, 2016-01-28 20:36:16]

Dry run: This issue passed the CQ dry run.

[stefan-webrtc, 2016-02-01 13:15:29]

Description was changed from

==========
Support multiple rtx codecs.

Adds negotiation of rtx codecs for red and vp9. To keep backwards
compatibility with older Chrome versions, this change includes two
hacks:
1. Red packets will be retransmitted over the rtx codec associated with
   vp8 if no rtx codec is associated with red. This is how Chrome does
   it today and ensures that we still can send red over rtx to older
   versions.

2. If rtx packets associated with the media codec (vp8/vp9 etc) are
   received and red has been negotiated, we will assume that the sender
   incorrectly has packetized red inside the rtx header associated with
   media. We will therefore restore it with the red payload type
   instead, which ensures that we can still receive rtx associated with
   red from old versions.

Offering multiple rtx codecs to older versions should not be a problem
since old versions themselves only try to negotiate rtx for vp8.

BUG=webrtc:4024
==========

to

==========
Support multiple rtx codecs.

Adds negotiation of rtx codecs for red and vp9. To keep backwards
compatibility with older Chrome versions, this change includes two
hacks:
1. Red packets will be retransmitted over the rtx codec associated with
   vp8 if no rtx codec is associated with red. This is how Chrome does
   it today and ensures that we still can send red over rtx to older
   versions.

2. If rtx packets associated with the media codec (vp8/vp9 etc) are
   received and red has been negotiated, we will assume that the sender
   incorrectly has packetized red inside the rtx header associated with
   media. We will therefore restore it with the red payload type
   instead, which ensures that we can still receive rtx associated with
   red from old versions.

Offering multiple rtx codecs to older versions should not be a problem
since old versions themselves only try to negotiate rtx for vp8.

BUG=webrtc:4024
TEST=Verified by running apprtc and emulating packet loss between Chrome with
and without the patch.
==========

[pbos-webrtc, 2016-02-01 14:32:19]

LGTM with questions.

https://codereview.webrtc.org/1649493004/diff/40001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc (right):

https://codereview.webrtc.org/1649493004/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc:276: if
(red_payload_type_ != -1) {
Does this path still have test coverage? It should until we decide to remove it.
I couldn't easily see that with the test removal.

https://codereview.webrtc.org/1649493004/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc:281: // old enough.
Put which Chrome versions these are, so you can remember when it's old enough.

https://codereview.webrtc.org/1649493004/diff/40001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_sender.cc (right):

https://codereview.webrtc.org/1649493004/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_sender.cc:1843: // TODO(holmer): Remove once
old Chrome versions don't rely on this.
Do you want to log a warning here?

[stefan-webrtc, 2016-02-01 15:25:46]

Test added.

[stefan-webrtc, 2016-02-01 15:26:09]

https://codereview.webrtc.org/1649493004/diff/40001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc (right):

https://codereview.webrtc.org/1649493004/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc:276: if
(red_payload_type_ != -1) {
On 2016/02/01 14:32:19, pbos-webrtc wrote:
> Does this path still have test coverage? It should until we decide to remove
it.
> I couldn't easily see that with the test removal.

I added a test which specifically tests this path.

https://codereview.webrtc.org/1649493004/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc:281: // old enough.
On 2016/02/01 14:32:19, pbos-webrtc wrote:
> Put which Chrome versions these are, so you can remember when it's old enough.

Done.

https://codereview.webrtc.org/1649493004/diff/40001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_sender.cc (right):

https://codereview.webrtc.org/1649493004/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_sender.cc:1843: // TODO(holmer): Remove once
old Chrome versions don't rely on this.
On 2016/02/01 14:32:19, pbos-webrtc wrote:
> Do you want to log a warning here?

I don't think so as that would spam warnings if a Chrome M50 talks to a Chrome
M49.

[pbos-webrtc, 2016-02-01 15:26:54]

thanks, lgtm

[stefan-webrtc, 2016-02-03 09:44:35]

Description was changed from

==========
Support multiple rtx codecs.

Adds negotiation of rtx codecs for red and vp9. To keep backwards
compatibility with older Chrome versions, this change includes two
hacks:
1. Red packets will be retransmitted over the rtx codec associated with
   vp8 if no rtx codec is associated with red. This is how Chrome does
   it today and ensures that we still can send red over rtx to older
   versions.

2. If rtx packets associated with the media codec (vp8/vp9 etc) are
   received and red has been negotiated, we will assume that the sender
   incorrectly has packetized red inside the rtx header associated with
   media. We will therefore restore it with the red payload type
   instead, which ensures that we can still receive rtx associated with
   red from old versions.

Offering multiple rtx codecs to older versions should not be a problem
since old versions themselves only try to negotiate rtx for vp8.

BUG=webrtc:4024
TEST=Verified by running apprtc and emulating packet loss between Chrome with
and without the patch.
==========

to

==========
Support multiple rtx codecs.

Adds negotiation of rtx codecs for red and vp9. To keep backwards
compatibility with older Chrome versions, this change includes two
hacks:
1. Red packets will be retransmitted over the rtx codec associated with
   vp8 if no rtx codec is associated with red. This is how Chrome does
   it today and ensures that we still can send red over rtx to older
   versions.

2. If rtx packets associated with the media codec (vp8/vp9 etc) are
   received and red has been negotiated, we will assume that the sender
   incorrectly has packetized red inside the rtx header associated with
   media. We will therefore restore it with the red payload type
   instead, which ensures that we can still receive rtx associated with
   red from old versions.

Offering multiple rtx codecs to older versions should not be a problem
since old versions themselves only try to negotiate rtx for vp8.

TBR=mflodman@webrtc.org
BUG=webrtc:4024
TEST=Verified by running apprtc and emulating packet loss between Chrome with
and without the patch.
==========

[stefan-webrtc, 2016-02-03 10:24:21]

Rebase.

[stefan-webrtc, 2016-02-03 10:25:33]

The CQ bit was checked by stefan@webrtc.org

[stefan-webrtc, 2016-02-03 10:25:34]

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

[commit-bot: I haz the power, 2016-02-03 10:25:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1649493004/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1649493004/80001

[commit-bot: I haz the power, 2016-02-03 12:26:14]

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

[commit-bot: I haz the power, 2016-02-03 12:26:15]

Try jobs failed on following builders:
  linux_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[stefan-webrtc, 2016-02-03 12:30:15]

Description was changed from

==========
Support multiple rtx codecs.

Adds negotiation of rtx codecs for red and vp9. To keep backwards
compatibility with older Chrome versions, this change includes two
hacks:
1. Red packets will be retransmitted over the rtx codec associated with
   vp8 if no rtx codec is associated with red. This is how Chrome does
   it today and ensures that we still can send red over rtx to older
   versions.

2. If rtx packets associated with the media codec (vp8/vp9 etc) are
   received and red has been negotiated, we will assume that the sender
   incorrectly has packetized red inside the rtx header associated with
   media. We will therefore restore it with the red payload type
   instead, which ensures that we can still receive rtx associated with
   red from old versions.

Offering multiple rtx codecs to older versions should not be a problem
since old versions themselves only try to negotiate rtx for vp8.

TBR=mflodman@webrtc.org
BUG=webrtc:4024
TEST=Verified by running apprtc and emulating packet loss between Chrome with
and without the patch.
==========

to

==========
Support multiple rtx codecs.

Adds negotiation of rtx codecs for red and vp9. To keep backwards
compatibility with older Chrome versions, this change includes two
hacks:
1. Red packets will be retransmitted over the rtx codec associated with
   vp8 if no rtx codec is associated with red. This is how Chrome does
   it today and ensures that we still can send red over rtx to older
   versions.

2. If rtx packets associated with the media codec (vp8/vp9 etc) are
   received and red has been negotiated, we will assume that the sender
   incorrectly has packetized red inside the rtx header associated with
   media. We will therefore restore it with the red payload type
   instead, which ensures that we can still receive rtx associated with
   red from old versions.

Offering multiple rtx codecs to older versions should not be a problem
since old versions themselves only try to negotiate rtx for vp8.

R=pbos@webrtc.org
TBR=mflodman@webrtc.org
BUG=webrtc:4024
TEST=Verified by running apprtc and emulating packet loss between Chrome with
and without the patch.

Committed:
https://chromium.googlesource.com/external/webrtc/+/10880011d9f116a9aff87436d...
==========

[stefan-webrtc, 2016-02-03 12:30:16]

Committed patchset #5 (id:80001) manually as
10880011d9f116a9aff87436de231151e355ac7d (presubmit successful).

[commit-bot: I haz the power, 2016-02-03 12:30:17]

Description was changed from

==========
Support multiple rtx codecs.

Adds negotiation of rtx codecs for red and vp9. To keep backwards
compatibility with older Chrome versions, this change includes two
hacks:
1. Red packets will be retransmitted over the rtx codec associated with
   vp8 if no rtx codec is associated with red. This is how Chrome does
   it today and ensures that we still can send red over rtx to older
   versions.

2. If rtx packets associated with the media codec (vp8/vp9 etc) are
   received and red has been negotiated, we will assume that the sender
   incorrectly has packetized red inside the rtx header associated with
   media. We will therefore restore it with the red payload type
   instead, which ensures that we can still receive rtx associated with
   red from old versions.

Offering multiple rtx codecs to older versions should not be a problem
since old versions themselves only try to negotiate rtx for vp8.

R=pbos@webrtc.org
TBR=mflodman@webrtc.org
BUG=webrtc:4024
TEST=Verified by running apprtc and emulating packet loss between Chrome with
and without the patch.

Committed:
https://chromium.googlesource.com/external/webrtc/+/10880011d9f116a9aff87436d...
==========

to

==========
Support multiple rtx codecs.

Adds negotiation of rtx codecs for red and vp9. To keep backwards
compatibility with older Chrome versions, this change includes two
hacks:
1. Red packets will be retransmitted over the rtx codec associated with
   vp8 if no rtx codec is associated with red. This is how Chrome does
   it today and ensures that we still can send red over rtx to older
   versions.

2. If rtx packets associated with the media codec (vp8/vp9 etc) are
   received and red has been negotiated, we will assume that the sender
   incorrectly has packetized red inside the rtx header associated with
   media. We will therefore restore it with the red payload type
   instead, which ensures that we can still receive rtx associated with
   red from old versions.

Offering multiple rtx codecs to older versions should not be a problem
since old versions themselves only try to negotiate rtx for vp8.

R=pbos@webrtc.org
TBR=mflodman@webrtc.org
BUG=webrtc:4024
TEST=Verified by running apprtc and emulating packet loss between Chrome with
and without the patch.

Committed: https://crrev.com/10880011d9f116a9aff87436de231151e355ac7d
Cr-Commit-Position: refs/heads/master@{#11472}
==========

[commit-bot: I haz the power, 2016-02-03 12:30:18]

Patchset 5 (id:??) landed as
https://crrev.com/10880011d9f116a9aff87436de231151e355ac7d
Cr-Commit-Position: refs/heads/master@{#11472}