Remove RED/RTX workaround from sender/receiver and VideoEngine2.

Remove RED/RTX workaround from sender/receiver and VideoEngine2.

In older Chrome versions, the associated payload type in the RTX header
of retransmitted packets was always set to be the original media payload type,
regardless of the actual payload type of the packet. This meant that packets
encapsulated with RED headers had incorrect payload type information in the
RTX header. Due to an assumption in the receiver, this incorrect payload type
information would effectively be undone, leading to a working system.

Albeit working, this behaviour was undesired, and thus removed. In the interim,
several workarounds were introduced to not destroy interop between old and
new Chrome versions:
  (1) https://codereview.webrtc.org/1649493004
      - If no payload type mapping existed for RED over RTX, the payload type
        of the underlying media would be used.
      - If RED had been negotiated, received RTX packets would always be
        assumed to contain RED.
  (2) https://codereview.webrtc.org/1964473002
      - If RED was removed from the remote description answer, it would be
        disabled in the local receiver as well.
  (3) https://codereview.webrtc.org/2033763002
      - If RED was negotiated in the SDP, it would always be used, regardless
        if ULPFEC was negotiated and used, or not.

Since the Chrome versions that exhibited the original bug now are very old,
this CL removes the workarounds from (1) and (2). In particular, after this
change, we will have the following behaviour:
  - We assume that a payload type mapping for RED over RTX always is set.
    If this is not the case, the RTX packet is not sent.
  - The associated payload type of received RTX packets will always be obeyed.
  - The (non)-existence of RED in the remote description does not affect the
    local receiver.
The workaround in (3) still needs to exist, in order to interop with receivers
that did not have the workarounds in (1) and (2) removed. The change in (3)
can be removed in a couple of Chrome versions.

TESTED=Using AppRTC between patched Chrome (connected to ethernet) and standard Chrome M54 (connected to lossy internal Google WiFi), with and without FEC turned off using AppRTC flag. Also using "Munge SDP" sample on patched Chrome over loopback interface, with 100ms delay and 5% packet loss simulated using tc.
BUG=webrtc:6650
Committed: https://crrev.com/e6f98c7a379aae970e7570ac3cf99e2a21f256c0
Cr-Commit-Position: refs/heads/master@{#15038}

[brandtr, 2016-11-02 11:28:24]

Description was changed from

==========
Remove RED/RTX workaround from sender/receiver and VideoEngine2.

In older Chrome versions, the associated payload type in the RTX header
of retransmitted packets was always set to be the original media payload type,
regardless of the actual payload type of the packet. This meant that packets
encapsulated with RED headers had incorrect payload type information in the
RTX header. Due to an assumption in the receiver, this incorrect payload type
information would effectively be undone, leading to a working system.

Albeit working, this behaviour was undesired, and thus removed. In the interim,
several workarounds were introduced to not destroy interop between old and
new Chrome versions:
  - https://codereview.webrtc.org/1649493004
      - If no payload type mapping existed for RED over RTX, the payload type
        of the underlying media would be used.
      - If RED had been negotiated, received RTX packets would always be
        assumed to contain RED.
  - https://codereview.webrtc.org/1964473002
      - If RED was removed from the remote description answer, it would be
        disabled in the local receiver as well.
  - https://codereview.webrtc.org/2033763002
      - If RED was negotiated in the SDP, it would always be used, regardless
        if ULPFEC was negotiated and used, or not.

Since the Chrome versions that exhibited the original bug now are very old,
this CL removes the workarounds described above. In particular, after this
change, we will have the following behaviour:
  - We assume that a payload type mapping for RED over RTX always is set.
    If this is not the case, the created RTX packet will not have a valid
    payload type.
  - The associated payload type of received RTX packets will always be obeyed.
  - The (non)-existence of RED in the remote description does not affect the
    local receiver.
  - RED will only be used when ULPFEC is enabled, even if RED but not ULPFEC
    has been negotiated in the SDP.

TESTED=Using AppRTC between patched Chrome (connected to ethernet) and standard
Chrome M54 (connected to lossy internal Google WiFi), with and without FEC
turned off using AppRTC flag. Also using "Munge SDP" sample on patched Chrome
over loopback interface, with 100ms delay and 5% packet loss simulated using tc.
BUG=None
==========

to

==========
Remove RED/RTX workaround from sender/receiver and VideoEngine2.

In older Chrome versions, the associated payload type in the RTX header
of retransmitted packets was always set to be the original media payload type,
regardless of the actual payload type of the packet. This meant that packets
encapsulated with RED headers had incorrect payload type information in the
RTX header. Due to an assumption in the receiver, this incorrect payload type
information would effectively be undone, leading to a working system.

Albeit working, this behaviour was undesired, and thus removed. In the interim,
several workarounds were introduced to not destroy interop between old and
new Chrome versions:
  - https://codereview.webrtc.org/1649493004
      - If no payload type mapping existed for RED over RTX, the payload type
        of the underlying media would be used.
      - If RED had been negotiated, received RTX packets would always be
        assumed to contain RED.
  - https://codereview.webrtc.org/1964473002
      - If RED was removed from the remote description answer, it would be
        disabled in the local receiver as well.
  - https://codereview.webrtc.org/2033763002
      - If RED was negotiated in the SDP, it would always be used, regardless
        if ULPFEC was negotiated and used, or not.

Since the Chrome versions that exhibited the original bug now are very old,
this CL removes the workarounds described above. In particular, after this
change, we will have the following behaviour:
  - We assume that a payload type mapping for RED over RTX always is set.
    If this is not the case, the created RTX packet will not have a valid
    payload type.
  - The associated payload type of received RTX packets will always be obeyed.
  - The (non)-existence of RED in the remote description does not affect the
    local receiver.
  - RED will only be used when ULPFEC is enabled, even if RED but not ULPFEC
    has been negotiated in the SDP.

TESTED=Using AppRTC between patched Chrome (connected to ethernet) and standard
Chrome M54 (connected to lossy internal Google WiFi), with and without FEC
turned off using AppRTC flag. Also using "Munge SDP" sample on patched Chrome
over loopback interface, with 100ms delay and 5% packet loss simulated using tc.
BUG=webrtc:6650
==========

[brandtr, 2016-11-02 11:34:19]

Patchset #1 (id:1) has been deleted

[brandtr, 2016-11-02 11:54:18]

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

[brandtr, 2016-11-02 11:54:19]

Please have a look at this CL.

Stefan: general
Danil: changes to RTP module

Do you have suggestions for anyone else to take a closer look at the
VideoEngine2 changes?

[danilchap, 2016-11-02 13:56:53]

https://codereview.webrtc.org/2469093003/diff/20001/webrtc/modules/rtp_rtcp/i...
File webrtc/modules/rtp_rtcp/include/rtp_payload_registry.h (left):

https://codereview.webrtc.org/2469093003/diff/20001/webrtc/modules/rtp_rtcp/i...
webrtc/modules/rtp_rtcp/include/rtp_payload_registry.h:175: // When true, use
rtx_payload_type_map_ when restoring RTX packets to get the
When false, still use rtx_payload_type_map_.
Nice to see unused confusing variables removed.

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

https://codereview.webrtc.org/2469093003/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc:260: int
associated_payload_type;
might as well remove this variable and use apt_mapping->second directly now.

https://codereview.webrtc.org/2469093003/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc:263: LOG(LS_WARNING) <<
"No RTX associated payload type mapping was available; "
Better to ensure you do not spam this messages (If there is an rtx packet with
mismatch payload_type, likely there will be a lot of them).

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

https://codereview.webrtc.org/2469093003/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_sender.cc:1164: if (kv !=
rtx_payload_type_map_.end())
this look like hack 'If no payload type mapping existed, the payload type of the
underlying media would be used.'
Is it still needed, or is it safe to reject sending rtx packet:
if (kv == rtx_map_.end())
  return nullptr;

[stefan-webrtc, 2016-11-02 14:24:53]

https://codereview.webrtc.org/2469093003/diff/20001/webrtc/video/video_send_s...
File webrtc/video/video_send_stream.cc (right):

https://codereview.webrtc.org/2469093003/diff/20001/webrtc/video/video_send_s...
webrtc/video/video_send_stream.cc:958: DisableUlpfec();
As discussed offline, this is not safe to do without waiting a few versions.

[brandtr, 2016-11-03 11:54:08]

Description was changed from

==========
Remove RED/RTX workaround from sender/receiver and VideoEngine2.

In older Chrome versions, the associated payload type in the RTX header
of retransmitted packets was always set to be the original media payload type,
regardless of the actual payload type of the packet. This meant that packets
encapsulated with RED headers had incorrect payload type information in the
RTX header. Due to an assumption in the receiver, this incorrect payload type
information would effectively be undone, leading to a working system.

Albeit working, this behaviour was undesired, and thus removed. In the interim,
several workarounds were introduced to not destroy interop between old and
new Chrome versions:
  - https://codereview.webrtc.org/1649493004
      - If no payload type mapping existed for RED over RTX, the payload type
        of the underlying media would be used.
      - If RED had been negotiated, received RTX packets would always be
        assumed to contain RED.
  - https://codereview.webrtc.org/1964473002
      - If RED was removed from the remote description answer, it would be
        disabled in the local receiver as well.
  - https://codereview.webrtc.org/2033763002
      - If RED was negotiated in the SDP, it would always be used, regardless
        if ULPFEC was negotiated and used, or not.

Since the Chrome versions that exhibited the original bug now are very old,
this CL removes the workarounds described above. In particular, after this
change, we will have the following behaviour:
  - We assume that a payload type mapping for RED over RTX always is set.
    If this is not the case, the created RTX packet will not have a valid
    payload type.
  - The associated payload type of received RTX packets will always be obeyed.
  - The (non)-existence of RED in the remote description does not affect the
    local receiver.
  - RED will only be used when ULPFEC is enabled, even if RED but not ULPFEC
    has been negotiated in the SDP.

TESTED=Using AppRTC between patched Chrome (connected to ethernet) and standard
Chrome M54 (connected to lossy internal Google WiFi), with and without FEC
turned off using AppRTC flag. Also using "Munge SDP" sample on patched Chrome
over loopback interface, with 100ms delay and 5% packet loss simulated using tc.
BUG=webrtc:6650
==========

to

==========
Remove RED/RTX workaround from sender/receiver and VideoEngine2.

In older Chrome versions, the associated payload type in the RTX header
of retransmitted packets was always set to be the original media payload type,
regardless of the actual payload type of the packet. This meant that packets
encapsulated with RED headers had incorrect payload type information in the
RTX header. Due to an assumption in the receiver, this incorrect payload type
information would effectively be undone, leading to a working system.

Albeit working, this behaviour was undesired, and thus removed. In the interim,
several workarounds were introduced to not destroy interop between old and
new Chrome versions:
  (1) https://codereview.webrtc.org/1649493004
      - If no payload type mapping existed for RED over RTX, the payload type
        of the underlying media would be used.
      - If RED had been negotiated, received RTX packets would always be
        assumed to contain RED.
  (2) https://codereview.webrtc.org/1964473002
      - If RED was removed from the remote description answer, it would be
        disabled in the local receiver as well.
  (3) https://codereview.webrtc.org/2033763002
      - If RED was negotiated in the SDP, it would always be used, regardless
        if ULPFEC was negotiated and used, or not.

Since the Chrome versions that exhibited the original bug now are very old,
this CL removes the workarounds from (1) and (2). In particular, after this
change, we will have the following behaviour:
  - We assume that a payload type mapping for RED over RTX always is set.
    If this is not the case, the RTX packet is not sent.
  - The associated payload type of received RTX packets will always be obeyed.
  - The (non)-existence of RED in the remote description does not affect the
    local receiver.
The workaround in (3) still needs to exist, in order to interop with receivers
that did not have the workarounds in (1) and (2) removed. The change in (3)
can be removed in a couple of Chrome versions.

TESTED=Using AppRTC between patched Chrome (connected to ethernet) and standard
Chrome M54 (connected to lossy internal Google WiFi), with and without FEC
turned off using AppRTC flag. Also using "Munge SDP" sample on patched Chrome
over loopback interface, with 100ms delay and 5% packet loss simulated using tc.
BUG=webrtc:6650
==========

[brandtr, 2016-11-03 12:02:48]

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

https://codereview.webrtc.org/2469093003/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc:260: int
associated_payload_type;
On 2016/11/02 13:56:52, danilchap wrote:
> might as well remove this variable and use apt_mapping->second directly now.

Kept, because now I use it in multiple places.

https://codereview.webrtc.org/2469093003/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc:263: LOG(LS_WARNING) <<
"No RTX associated payload type mapping was available; "
On 2016/11/02 13:56:52, danilchap wrote:
> Better to ensure you do not spam this messages (If there is an rtx packet with
> mismatch payload_type, likely there will be a lot of them).

Done.

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

https://codereview.webrtc.org/2469093003/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_sender.cc:1164: if (kv !=
rtx_payload_type_map_.end())
On 2016/11/02 13:56:52, danilchap wrote:
> this look like hack 'If no payload type mapping existed, the payload type of
the
> underlying media would be used.'
> Is it still needed, or is it safe to reject sending rtx packet:
> if (kv == rtx_map_.end())
>   return nullptr;

It should be safe to reject sending it, because all associated payload types are
set here:
https://cs.chromium.org/chromium/src/third_party/webrtc/video/video_send_stre...

+holmer: can you verify this?

https://codereview.webrtc.org/2469093003/diff/20001/webrtc/video/video_send_s...
File webrtc/video/video_send_stream.cc (right):

https://codereview.webrtc.org/2469093003/diff/20001/webrtc/video/video_send_s...
webrtc/video/video_send_stream.cc:958: DisableUlpfec();
On 2016/11/02 14:24:53, stefan-webrtc (holmer) wrote:
> As discussed offline, this is not safe to do without waiting a few versions.

Thanks for finding this!

I tried to compile all combinations of old/new senders/receivers here:
https://docs.google.com/document/d/1SOWOgiOGdwRNEf9YDflz-SPdp0HDLUFp7JuuSl8tvf8.
I think by keeping the send behaviour as is, we should be fine.

[danilchap, 2016-11-03 12:10:36]

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

https://codereview.webrtc.org/2469093003/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc:261: int
associated_payload_type = apt_mapping->second;
do not think you can do this if apt_mapping == end()

https://codereview.webrtc.org/2469093003/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc:265:
associated_payload_type) == 0) {
guess you want to use header.payloadType instead of undefined
associated_payload_type here.

[stefan-webrtc, 2016-11-03 12:17:15]

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

https://codereview.webrtc.org/2469093003/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc:265:
associated_payload_type) == 0) {
I prefer find() over count() when you don't care about the actual count.

https://codereview.webrtc.org/2469093003/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc:275: }
Shouldn't we return false here too?

[brandtr, 2016-11-03 13:48:09]

Addressed comments and improved code.

This still needs some further testing though: with the last PS in this CL, after
munging SDP such that RED is enabled but ULPFEC is not, I'm able to set up a
loopback call, but I don't get a picture. Before the change in PS5, I was able
to get a picture after the same munging.

This result should not be due to this CL however, as I can reproduce using
Chrome stable. It's still related obviously, and I should study the reason
behind the lack of picture further.

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

https://codereview.webrtc.org/2469093003/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc:261: int
associated_payload_type = apt_mapping->second;
On 2016/11/03 12:10:36, danilchap wrote:
> do not think you can do this if apt_mapping == end()

Wow. This was terrible, sorry for that. I guess this would have led to a crash,
if apt_mapping == .end()? Now it should hopefully be better.

https://codereview.webrtc.org/2469093003/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc:265:
associated_payload_type) == 0) {
On 2016/11/03 12:17:15, stefan-webrtc (holmer) wrote:
> I prefer find() over count() when you don't care about the actual count.

Done.

https://codereview.webrtc.org/2469093003/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc:275: }
On 2016/11/03 12:17:15, stefan-webrtc (holmer) wrote:
> Shouldn't we return false here too?

Yes!

[danilchap, 2016-11-03 14:21:08]

webrtc/modules/rtp_rtcp/ lgtm

https://codereview.webrtc.org/2469093003/diff/140001/webrtc/modules/rtp_rtcp/...
File webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc (right):

https://codereview.webrtc.org/2469093003/diff/140001/webrtc/modules/rtp_rtcp/...
webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc:270: << "Suppressing
further warnings for this payload type.";
may merge this string with previous one without extra '<<':
<< static_cast<int>(payloadaType)
<< ". Suppressing further warnings...";

[brandtr, 2016-11-03 14:30:10]

https://codereview.webrtc.org/2469093003/diff/140001/webrtc/modules/rtp_rtcp/...
File webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc (right):

https://codereview.webrtc.org/2469093003/diff/140001/webrtc/modules/rtp_rtcp/...
webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc:270: << "Suppressing
further warnings for this payload type.";
On 2016/11/03 14:21:08, danilchap wrote:
> may merge this string with previous one without extra '<<':
> << static_cast<int>(payloadaType)
> << ". Suppressing further warnings...";

That's how I originally wrote it, but I thought it looked a bit ugly :) I guess
the performance penalty should not be too bad from this one extra stream
operation?

[danilchap, 2016-11-03 14:47:50]

https://codereview.webrtc.org/2469093003/diff/140001/webrtc/modules/rtp_rtcp/...
File webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc (right):

https://codereview.webrtc.org/2469093003/diff/140001/webrtc/modules/rtp_rtcp/...
webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc:270: << "Suppressing
further warnings for this payload type.";
On 2016/11/03 14:30:10, brandtr wrote:
> On 2016/11/03 14:21:08, danilchap wrote:
> > may merge this string with previous one without extra '<<':
> > << static_cast<int>(payloadaType)
> > << ". Suppressing further warnings...";
> 
> That's how I originally wrote it, but I thought it looked a bit ugly :) I
guess
> the performance penalty should not be too bad from this one extra stream
> operation?

yep, performance penalty is negligitble.

[brandtr, 2016-11-03 15:09:06]

Probable reason for broken video when negotiating RED without ULPFEC: the
RtpStreamReceiver will only accept RED or ULPFEC whenever BOTH are present in
the local description. See here:
https://cs.chromium.org/chromium/src/third_party/webrtc/video/rtp_stream_rece...
https://cs.chromium.org/chromium/src/third_party/webrtc/video/rtp_stream_rece...

[brandtr, 2016-11-03 15:33:56]

Rebase.

[brandtr, 2016-11-07 12:08:43]

Rebase.

[brandtr, 2016-11-07 12:13:02]

Now separately enabling RED and ULPFEC in RtpStreamReceiver. This allows us to
receive RED packets, even if ULPFEC is not used.

Please have a second look :)

[stefan-webrtc, 2016-11-07 12:45:14]

lgtm

[brandtr, 2016-11-10 08:24:44]

Hey holmer,

I missed removing a now unnecessary config variable in the previous patchsets,
so I uploaded a new change for that. Can you check that this is still good to
go?

I found the issue with video_loopback (see separate CL), so I'm feeling good
landing this now. This change probably warrants a note in the release notes
through, what do you think?

[brandtr, 2016-11-10 09:04:36]

Rebase.

[stefan-webrtc, 2016-11-11 10:22:08]

On 2016/11/10 08:24:44, brandtr wrote:
> Hey holmer,
> 
> I missed removing a now unnecessary config variable in the previous patchsets,
> so I uploaded a new change for that. Can you check that this is still good to
> go?
> 
> I found the issue with video_loopback (see separate CL), so I'm feeling good
> landing this now. This change probably warrants a note in the release notes
> through, what do you think?

lgtm. Yes, this should go into the notes.

[brandtr, 2016-11-11 10:31:07]

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

[commit-bot: I haz the power, 2016-11-11 10:31:21]

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-11 10:58:02]

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

[commit-bot: I haz the power, 2016-11-11 10:58:03]

Dry run: This issue passed the CQ dry run.

[brandtr, 2016-11-11 11:25:05]

The CQ bit was checked by brandtr@webrtc.org

[brandtr, 2016-11-11 11:25:06]

The patchset sent to the CQ was uploaded after l-g-t-m from danilchap@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2469093003/#ps250001 (title:
"Fix warning message.")

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

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-11 11:28:33]

Description was changed from

==========
Remove RED/RTX workaround from sender/receiver and VideoEngine2.

In older Chrome versions, the associated payload type in the RTX header
of retransmitted packets was always set to be the original media payload type,
regardless of the actual payload type of the packet. This meant that packets
encapsulated with RED headers had incorrect payload type information in the
RTX header. Due to an assumption in the receiver, this incorrect payload type
information would effectively be undone, leading to a working system.

Albeit working, this behaviour was undesired, and thus removed. In the interim,
several workarounds were introduced to not destroy interop between old and
new Chrome versions:
  (1) https://codereview.webrtc.org/1649493004
      - If no payload type mapping existed for RED over RTX, the payload type
        of the underlying media would be used.
      - If RED had been negotiated, received RTX packets would always be
        assumed to contain RED.
  (2) https://codereview.webrtc.org/1964473002
      - If RED was removed from the remote description answer, it would be
        disabled in the local receiver as well.
  (3) https://codereview.webrtc.org/2033763002
      - If RED was negotiated in the SDP, it would always be used, regardless
        if ULPFEC was negotiated and used, or not.

Since the Chrome versions that exhibited the original bug now are very old,
this CL removes the workarounds from (1) and (2). In particular, after this
change, we will have the following behaviour:
  - We assume that a payload type mapping for RED over RTX always is set.
    If this is not the case, the RTX packet is not sent.
  - The associated payload type of received RTX packets will always be obeyed.
  - The (non)-existence of RED in the remote description does not affect the
    local receiver.
The workaround in (3) still needs to exist, in order to interop with receivers
that did not have the workarounds in (1) and (2) removed. The change in (3)
can be removed in a couple of Chrome versions.

TESTED=Using AppRTC between patched Chrome (connected to ethernet) and standard
Chrome M54 (connected to lossy internal Google WiFi), with and without FEC
turned off using AppRTC flag. Also using "Munge SDP" sample on patched Chrome
over loopback interface, with 100ms delay and 5% packet loss simulated using tc.
BUG=webrtc:6650
==========

to

==========
Remove RED/RTX workaround from sender/receiver and VideoEngine2.

In older Chrome versions, the associated payload type in the RTX header
of retransmitted packets was always set to be the original media payload type,
regardless of the actual payload type of the packet. This meant that packets
encapsulated with RED headers had incorrect payload type information in the
RTX header. Due to an assumption in the receiver, this incorrect payload type
information would effectively be undone, leading to a working system.

Albeit working, this behaviour was undesired, and thus removed. In the interim,
several workarounds were introduced to not destroy interop between old and
new Chrome versions:
  (1) https://codereview.webrtc.org/1649493004
      - If no payload type mapping existed for RED over RTX, the payload type
        of the underlying media would be used.
      - If RED had been negotiated, received RTX packets would always be
        assumed to contain RED.
  (2) https://codereview.webrtc.org/1964473002
      - If RED was removed from the remote description answer, it would be
        disabled in the local receiver as well.
  (3) https://codereview.webrtc.org/2033763002
      - If RED was negotiated in the SDP, it would always be used, regardless
        if ULPFEC was negotiated and used, or not.

Since the Chrome versions that exhibited the original bug now are very old,
this CL removes the workarounds from (1) and (2). In particular, after this
change, we will have the following behaviour:
  - We assume that a payload type mapping for RED over RTX always is set.
    If this is not the case, the RTX packet is not sent.
  - The associated payload type of received RTX packets will always be obeyed.
  - The (non)-existence of RED in the remote description does not affect the
    local receiver.
The workaround in (3) still needs to exist, in order to interop with receivers
that did not have the workarounds in (1) and (2) removed. The change in (3)
can be removed in a couple of Chrome versions.

TESTED=Using AppRTC between patched Chrome (connected to ethernet) and standard
Chrome M54 (connected to lossy internal Google WiFi), with and without FEC
turned off using AppRTC flag. Also using "Munge SDP" sample on patched Chrome
over loopback interface, with 100ms delay and 5% packet loss simulated using tc.
BUG=webrtc:6650
==========

[commit-bot: I haz the power, 2016-11-11 11:28:34]

Committed patchset #13 (id:250001)

[commit-bot: I haz the power, 2016-11-11 11:28:43]

Description was changed from

==========
Remove RED/RTX workaround from sender/receiver and VideoEngine2.

In older Chrome versions, the associated payload type in the RTX header
of retransmitted packets was always set to be the original media payload type,
regardless of the actual payload type of the packet. This meant that packets
encapsulated with RED headers had incorrect payload type information in the
RTX header. Due to an assumption in the receiver, this incorrect payload type
information would effectively be undone, leading to a working system.

Albeit working, this behaviour was undesired, and thus removed. In the interim,
several workarounds were introduced to not destroy interop between old and
new Chrome versions:
  (1) https://codereview.webrtc.org/1649493004
      - If no payload type mapping existed for RED over RTX, the payload type
        of the underlying media would be used.
      - If RED had been negotiated, received RTX packets would always be
        assumed to contain RED.
  (2) https://codereview.webrtc.org/1964473002
      - If RED was removed from the remote description answer, it would be
        disabled in the local receiver as well.
  (3) https://codereview.webrtc.org/2033763002
      - If RED was negotiated in the SDP, it would always be used, regardless
        if ULPFEC was negotiated and used, or not.

Since the Chrome versions that exhibited the original bug now are very old,
this CL removes the workarounds from (1) and (2). In particular, after this
change, we will have the following behaviour:
  - We assume that a payload type mapping for RED over RTX always is set.
    If this is not the case, the RTX packet is not sent.
  - The associated payload type of received RTX packets will always be obeyed.
  - The (non)-existence of RED in the remote description does not affect the
    local receiver.
The workaround in (3) still needs to exist, in order to interop with receivers
that did not have the workarounds in (1) and (2) removed. The change in (3)
can be removed in a couple of Chrome versions.

TESTED=Using AppRTC between patched Chrome (connected to ethernet) and standard
Chrome M54 (connected to lossy internal Google WiFi), with and without FEC
turned off using AppRTC flag. Also using "Munge SDP" sample on patched Chrome
over loopback interface, with 100ms delay and 5% packet loss simulated using tc.
BUG=webrtc:6650
==========

to

==========
Remove RED/RTX workaround from sender/receiver and VideoEngine2.

In older Chrome versions, the associated payload type in the RTX header
of retransmitted packets was always set to be the original media payload type,
regardless of the actual payload type of the packet. This meant that packets
encapsulated with RED headers had incorrect payload type information in the
RTX header. Due to an assumption in the receiver, this incorrect payload type
information would effectively be undone, leading to a working system.

Albeit working, this behaviour was undesired, and thus removed. In the interim,
several workarounds were introduced to not destroy interop between old and
new Chrome versions:
  (1) https://codereview.webrtc.org/1649493004
      - If no payload type mapping existed for RED over RTX, the payload type
        of the underlying media would be used.
      - If RED had been negotiated, received RTX packets would always be
        assumed to contain RED.
  (2) https://codereview.webrtc.org/1964473002
      - If RED was removed from the remote description answer, it would be
        disabled in the local receiver as well.
  (3) https://codereview.webrtc.org/2033763002
      - If RED was negotiated in the SDP, it would always be used, regardless
        if ULPFEC was negotiated and used, or not.

Since the Chrome versions that exhibited the original bug now are very old,
this CL removes the workarounds from (1) and (2). In particular, after this
change, we will have the following behaviour:
  - We assume that a payload type mapping for RED over RTX always is set.
    If this is not the case, the RTX packet is not sent.
  - The associated payload type of received RTX packets will always be obeyed.
  - The (non)-existence of RED in the remote description does not affect the
    local receiver.
The workaround in (3) still needs to exist, in order to interop with receivers
that did not have the workarounds in (1) and (2) removed. The change in (3)
can be removed in a couple of Chrome versions.

TESTED=Using AppRTC between patched Chrome (connected to ethernet) and standard
Chrome M54 (connected to lossy internal Google WiFi), with and without FEC
turned off using AppRTC flag. Also using "Munge SDP" sample on patched Chrome
over loopback interface, with 100ms delay and 5% packet loss simulated using tc.
BUG=webrtc:6650

Committed: https://crrev.com/e6f98c7a379aae970e7570ac3cf99e2a21f256c0
Cr-Commit-Position: refs/heads/master@{#15038}
==========

[commit-bot: I haz the power, 2016-11-11 11:28:44]

Patchset 13 (id:??) landed as
https://crrev.com/e6f98c7a379aae970e7570ac3cf99e2a21f256c0
Cr-Commit-Position: refs/heads/master@{#15038}