Fix RTPPayloadRegistry to correctly restore RTX, if a valid mapping exists.

webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc

 /*
  *  Copyright (c) 2013 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
 #include "webrtc/modules/rtp_rtcp/interface/rtp_payload_registry.h"
 
 #include "webrtc/modules/rtp_rtcp/source/byte_io.h"
 #include "webrtc/system_wrappers/interface/logging.h"
 
 namespace webrtc {
 
 RTPPayloadRegistry::RTPPayloadRegistry(RTPPayloadStrategy* rtp_payload_strategy)
     : crit_sect_(CriticalSectionWrapper::CreateCriticalSection()),
       rtp_payload_strategy_(rtp_payload_strategy),
       red_payload_type_(-1),
       ulpfec_payload_type_(-1),
       incoming_payload_type_(-1),
       last_received_payload_type_(-1),
       last_received_media_payload_type_(-1),
       rtx_(false),
       rtx_payload_type_(-1),
-      ssrc_rtx_(0) {
-}
+      use_rtx_payload_mapping_on_restore_(false),
+      ssrc_rtx_(0) {}
 
 RTPPayloadRegistry::~RTPPayloadRegistry() {
   while (!payload_type_map_.empty()) {
     RtpUtility::PayloadTypeMap::iterator it = payload_type_map_.begin();
     delete it->second;
     payload_type_map_.erase(it);
   }
 }
 
 int32_t RTPPayloadRegistry::RegisterReceivePayload(
@@ skipping 185 matching lines @@
 
 bool RTPPayloadRegistry::IsRtx(const RTPHeader& header) const {
   CriticalSectionScoped cs(crit_sect_.get());
   return IsRtxInternal(header);
 }
 
 bool RTPPayloadRegistry::IsRtxInternal(const RTPHeader& header) const {
   return rtx_ && ssrc_rtx_ == header.ssrc;
 }
 
-bool RTPPayloadRegistry::RestoreOriginalPacket(uint8_t** restored_packet,
+bool RTPPayloadRegistry::RestoreOriginalPacket(uint8_t* restored_packet,
                                                const uint8_t* packet,
                                                size_t* packet_length,
                                                uint32_t original_ssrc,
                                                const RTPHeader& header) const {
   if (kRtxHeaderSize + header.headerLength + header.paddingLength >
       *packet_length) {
     return false;
   }
   const uint8_t* rtx_header = packet + header.headerLength;
   uint16_t original_sequence_number = (rtx_header[0] << 8) + rtx_header[1];
 
   // Copy the packet into the restored packet, except for the RTX header.
-  memcpy(*restored_packet, packet, header.headerLength);
-  memcpy(*restored_packet + header.headerLength,
+  memcpy(restored_packet, packet, header.headerLength);
+  memcpy(restored_packet + header.headerLength,
          packet + header.headerLength + kRtxHeaderSize,
          *packet_length - header.headerLength - kRtxHeaderSize);
   *packet_length -= kRtxHeaderSize;
 
   // Replace the SSRC and the sequence number with the originals.
-  ByteWriter<uint16_t>::WriteBigEndian(*restored_packet + 2,
+  ByteWriter<uint16_t>::WriteBigEndian(restored_packet + 2,
                                        original_sequence_number);
-  ByteWriter<uint32_t>::WriteBigEndian(*restored_packet + 8, original_ssrc);
+  ByteWriter<uint32_t>::WriteBigEndian(restored_packet + 8, original_ssrc);
 
   CriticalSectionScoped cs(crit_sect_.get());
   if (!rtx_)
     return true;
 
-  if (rtx_payload_type_ == -1 || incoming_payload_type_ == -1) {
-    LOG(LS_WARNING) << "Incorrect RTX configuration, dropping packet.";
-    return false;
+  int associated_payload_type;
+  auto apt_mapping = rtx_payload_type_map_.find(header.payloadType);
+  if (use_rtx_payload_mapping_on_restore_ &&
+      apt_mapping != rtx_payload_type_map_.end()) {
+    associated_payload_type = apt_mapping->second;
+  } else {
+    // In the future, this will be a bug. For now, just assume this RTX packet
+    // matches the last non-RTX payload type we received. There are cases where
+    // this could break, especially where RTX is sent outside of NACKing (e.g.
+    // padding with redundant payloads).
+    if (rtx_payload_type_ == -1 || incoming_payload_type_ == -1) {
+      LOG(LS_WARNING) << "Incorrect RTX configuration, dropping packet.";
+      return false;
+    }
+    associated_payload_type = incoming_payload_type_;
   }
-  // TODO(changbin): Will use RTX APT map for restoring packets,
-  // thus incoming_payload_type_ should be removed in future.
-  (*restored_packet)[1] = static_cast<uint8_t>(incoming_payload_type_);
+
+  restored_packet[1] = static_cast<uint8_t>(associated_payload_type);
   if (header.markerBit) {
-    (*restored_packet)[1] |= kRtpMarkerBitMask;  // Marker bit is set.
+    restored_packet[1] |= kRtpMarkerBitMask;  // Marker bit is set.
   }
   return true;
 }
 
 void RTPPayloadRegistry::SetRtxSsrc(uint32_t ssrc) {
   CriticalSectionScoped cs(crit_sect_.get());
   ssrc_rtx_ = ssrc;
   rtx_ = true;
 }
 
@@ skipping 181 matching lines @@
 RTPPayloadStrategy* RTPPayloadStrategy::CreateStrategy(
     const bool handling_audio) {
   if (handling_audio) {
     return new RTPPayloadAudioStrategy();
   } else {
     return new RTPPayloadVideoStrategy();
   }
 }
 
 }  // namespace webrtc