Moved codec-specific audio packet splitting into decoders.

webrtc/modules/audio_coding/neteq/payload_splitter.cc

Index: webrtc/modules/audio_coding/neteq/payload_splitter.cc
diff --git a/webrtc/modules/audio_coding/neteq/payload_splitter.cc b/webrtc/modules/audio_coding/neteq/payload_splitter.cc
index 530e9d064dc703a655c7dd00aa0392f4140ea0a2..24ec92e418c486f8dcb61e7b05cfed5514b40bfe 100644
--- a/webrtc/modules/audio_coding/neteq/payload_splitter.cc
+++ b/webrtc/modules/audio_coding/neteq/payload_splitter.cc
@@ -232,211 +232,50 @@ int PayloadSplitter::SplitAudio(PacketList* packet_list,
       LOG(LS_WARNING) << "SplitAudio unknown payload type";
       return kUnknownPayloadType;
     }
-    // No splitting for a sync-packet.
-    if (packet->sync_packet) {
+    // No splitting for a sync-packet nor for any format handled internally.
+    if (packet->sync_packet ||
+        info->IsComfortNoise() || info->IsDtmf() || info->IsRed()) {
       ++it;
       continue;
     }
-    PacketList new_packets;
-    switch (info->codec_type) {
-      case NetEqDecoder::kDecoderPCMu:
-      case NetEqDecoder::kDecoderPCMa: {
-        // 8 bytes per ms; 8 timestamps per ms.
-        SplitBySamples(packet, 8, 8, &new_packets);
-        break;
-      }
-      case NetEqDecoder::kDecoderPCMu_2ch:
-      case NetEqDecoder::kDecoderPCMa_2ch: {
-        // 2 * 8 bytes per ms; 8 timestamps per ms.
-        SplitBySamples(packet, 2 * 8, 8, &new_packets);
-        break;
-      }
-      case NetEqDecoder::kDecoderG722: {
-        // 8 bytes per ms; 16 timestamps per ms.
-        SplitBySamples(packet, 8, 16, &new_packets);
-        break;
-      }
-      case NetEqDecoder::kDecoderPCM16B: {
-        // 16 bytes per ms; 8 timestamps per ms.
-        SplitBySamples(packet, 16, 8, &new_packets);
-        break;
-      }
-      case NetEqDecoder::kDecoderPCM16Bwb: {
-        // 32 bytes per ms; 16 timestamps per ms.
-        SplitBySamples(packet, 32, 16, &new_packets);
-        break;
-      }
-      case NetEqDecoder::kDecoderPCM16Bswb32kHz: {
-        // 64 bytes per ms; 32 timestamps per ms.
-        SplitBySamples(packet, 64, 32, &new_packets);
-        break;
-      }
-      case NetEqDecoder::kDecoderPCM16Bswb48kHz: {
-        // 96 bytes per ms; 48 timestamps per ms.
-        SplitBySamples(packet, 96, 48, &new_packets);
-        break;
-      }
-      case NetEqDecoder::kDecoderPCM16B_2ch: {
-        // 2 * 16 bytes per ms; 8 timestamps per ms.
-        SplitBySamples(packet, 2 * 16, 8, &new_packets);
-        break;
-      }
-      case NetEqDecoder::kDecoderPCM16Bwb_2ch: {
-        // 2 * 32 bytes per ms; 16 timestamps per ms.
-        SplitBySamples(packet, 2 * 32, 16, &new_packets);
-        break;
-      }
-      case NetEqDecoder::kDecoderPCM16Bswb32kHz_2ch: {
-        // 2 * 64 bytes per ms; 32 timestamps per ms.
-        SplitBySamples(packet, 2 * 64, 32, &new_packets);
-        break;
-      }
-      case NetEqDecoder::kDecoderPCM16Bswb48kHz_2ch: {
-        // 2 * 96 bytes per ms; 48 timestamps per ms.
-        SplitBySamples(packet, 2 * 96, 48, &new_packets);
-        break;
-      }
-      case NetEqDecoder::kDecoderPCM16B_5ch: {
-        // 5 * 16 bytes per ms; 8 timestamps per ms.
-        SplitBySamples(packet, 5 * 16, 8, &new_packets);
-        break;
-      }
-      case NetEqDecoder::kDecoderILBC: {
-        size_t bytes_per_frame;
-        int timestamps_per_frame;
-        if (packet->payload_length >= 950) {
-          LOG(LS_WARNING) << "SplitAudio too large iLBC payload";
-          return kTooLargePayload;
-        }
-        if (packet->payload_length % 38 == 0) {
-          // 20 ms frames.
-          bytes_per_frame = 38;
-          timestamps_per_frame = 160;
-        } else if (packet->payload_length % 50 == 0) {
-          // 30 ms frames.
-          bytes_per_frame = 50;
-          timestamps_per_frame = 240;
-        } else {
-          LOG(LS_WARNING) << "SplitAudio invalid iLBC payload";
-          return kFrameSplitError;
-        }
-        int ret = SplitByFrames(packet, bytes_per_frame, timestamps_per_frame,
-                                &new_packets);
-        if (ret < 0) {
-          return ret;
-        } else if (ret == kNoSplit) {
-          // Do not split at all. Simply advance to the next packet in the list.
-          ++it;
-          // We do not have any new packets to insert, and should not delete the
-          // old one. Skip the code after the switch case, and jump straight to
-          // the next packet in the while loop.
-          continue;
-        }
-        break;
-      }
-      default: {
-        // Do not split at all. Simply advance to the next packet in the list.
-        ++it;
-        // We do not have any new packets to insert, and should not delete the
-        // old one. Skip the code after the switch case, and jump straight to
-        // the next packet in the while loop.
-        continue;
+    const AudioDecoder* decoder = info->GetDecoder();
+    RTC_DCHECK(decoder);
+    AudioDecoder::PacketSplits splits =
+        decoder->SplitPacket(packet->payload, packet->payload_length);
+    if (splits.empty()) {
+      return kFrameSplitError;
+    } else if (splits.size() == 1 &&
+               splits[0].byte_offset == 0 &&
+               splits[0].num_bytes == packet->payload_length &&
+               splits[0].timestamp_offset == 0) {
+      // No splitting necessary; move on to the next packet

[kwiberg-webrtc, 2016/08/26 12:39:25]

Hmm. Wouldn't it be better to test just the first thing, and DCHECK the
remaining three?

[ossu, 2016/08/26 13:05:30]

Nah, I think it's fine if you can figure out that some data could be discarded
already in SplitPacket. Though the extra copying probably isn't worth it.

+      ++it;
+    } else {
+      PacketList new_packets;
+      for (const auto& split : splits) {
+        Packet* new_packet = new Packet;

[kwiberg-webrtc, 2016/08/26 12:39:25]

Ew. Consider using unique_ptr in this block, and using .release() when you put
it in |new_packets| at the end.

[ossu, 2016/08/26 13:05:30]

Hello. I'm the messenger. Please don't shoot me! :)

+        const uint8_t* payload_ptr = packet->payload + split.byte_offset;
+        new_packet->payload_length = split.num_bytes;
+        new_packet->header = packet->header;
+        new_packet->header.timestamp += split.timestamp_offset;
+        new_packet->primary = packet->primary;
+        new_packet->payload = new uint8_t[split.num_bytes];
+        memcpy(new_packet->payload, payload_ptr, split.num_bytes);
+        new_packets.push_back(new_packet);
       }
-    }
-    // Insert new packets into original list, before the element pointed to by
-    // iterator |it|.
-    packet_list->splice(it, new_packets, new_packets.begin(),
-                        new_packets.end());
-    // Delete old packet payload.
-    delete [] (*it)->payload;
-    delete (*it);
-    // Remove |it| from the packet list. This operation effectively moves the
-    // iterator |it| to the next packet in the list. Thus, we do not have to
-    // increment it manually.
-    it = packet_list->erase(it);
-  }
-  return kOK;
-}
-
-void PayloadSplitter::SplitBySamples(const Packet* packet,
-                                     size_t bytes_per_ms,
-                                     uint32_t timestamps_per_ms,
-                                     PacketList* new_packets) {
-  assert(packet);
-  assert(new_packets);
-
-  size_t split_size_bytes = packet->payload_length;
-
-  // Find a "chunk size" >= 20 ms and < 40 ms.
-  size_t min_chunk_size = bytes_per_ms * 20;
-  // Reduce the split size by half as long as |split_size_bytes| is at least
-  // twice the minimum chunk size (so that the resulting size is at least as
-  // large as the minimum chunk size).
-  while (split_size_bytes >= 2 * min_chunk_size) {
-    split_size_bytes >>= 1;
-  }
-  uint32_t timestamps_per_chunk = static_cast<uint32_t>(
-      split_size_bytes * timestamps_per_ms / bytes_per_ms);
-  uint32_t timestamp = packet->header.timestamp;
-
-  uint8_t* payload_ptr = packet->payload;
-  size_t len = packet->payload_length;
-  while (len >= (2 * split_size_bytes)) {
-    Packet* new_packet = new Packet;
-    new_packet->payload_length = split_size_bytes;
-    new_packet->header = packet->header;
-    new_packet->header.timestamp = timestamp;
-    timestamp += timestamps_per_chunk;
-    new_packet->primary = packet->primary;
-    new_packet->payload = new uint8_t[split_size_bytes];
-    memcpy(new_packet->payload, payload_ptr, split_size_bytes);
-    payload_ptr += split_size_bytes;
-    new_packets->push_back(new_packet);
-    len -= split_size_bytes;
-  }
 
-  if (len > 0) {
-    Packet* new_packet = new Packet;
-    new_packet->payload_length = len;
-    new_packet->header = packet->header;
-    new_packet->header.timestamp = timestamp;
-    new_packet->primary = packet->primary;
-    new_packet->payload = new uint8_t[len];
-    memcpy(new_packet->payload, payload_ptr, len);
-    new_packets->push_back(new_packet);
-  }
-}
-
-int PayloadSplitter::SplitByFrames(const Packet* packet,
-                                   size_t bytes_per_frame,
-                                   uint32_t timestamps_per_frame,
-                                   PacketList* new_packets) {
-  if (packet->payload_length % bytes_per_frame != 0) {
-    LOG(LS_WARNING) << "SplitByFrames length mismatch";
-    return kFrameSplitError;
-  }
-
-  if (packet->payload_length == bytes_per_frame) {
-    // Special case. Do not split the payload.
-    return kNoSplit;
-  }
-
-  uint32_t timestamp = packet->header.timestamp;
-  uint8_t* payload_ptr = packet->payload;
-  size_t len = packet->payload_length;
-  while (len > 0) {
-    assert(len >= bytes_per_frame);
-    Packet* new_packet = new Packet;
-    new_packet->payload_length = bytes_per_frame;
-    new_packet->header = packet->header;
-    new_packet->header.timestamp = timestamp;
-    timestamp += timestamps_per_frame;
-    new_packet->primary = packet->primary;
-    new_packet->payload = new uint8_t[bytes_per_frame];
-    memcpy(new_packet->payload, payload_ptr, bytes_per_frame);
-    payload_ptr += bytes_per_frame;
-    new_packets->push_back(new_packet);
-    len -= bytes_per_frame;
+      // Insert new packets into original list, before the element pointed to by
+      // iterator |it|.
+      packet_list->splice(it, new_packets, new_packets.begin(),
+                          new_packets.end());
+      // Delete old packet payload.
+      delete [] (*it)->payload;
+      delete (*it);
+      // Remove |it| from the packet list. This operation effectively moves the
+      // iterator |it| to the next packet in the list. Thus, we do not have to
+      // increment it manually.
+      it = packet_list->erase(it);
+    }
   }
   return kOK;
 }