Moved Opus-specific payload splitting into AudioDecoderOpus.

webrtc/modules/audio_coding/neteq/payload_splitter_unittest.cc

 /*
  *  Copyright (c) 2012 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.
  */
 
@@ skipping 126 matching lines @@
   return packet;
 }
 
 // Checks that |packet| has the attributes given in the remaining parameters.
 void VerifyPacket(const Packet* packet,
                   size_t payload_length,
                   uint8_t payload_type,
                   uint16_t sequence_number,
                   uint32_t timestamp,
                   uint8_t payload_value,
-                  bool primary = true) {
+                  Packet::Priority priority) {
   EXPECT_EQ(payload_length, packet->payload.size());
   EXPECT_EQ(payload_type, packet->header.payloadType);
   EXPECT_EQ(sequence_number, packet->header.sequenceNumber);
   EXPECT_EQ(timestamp, packet->header.timestamp);
-  EXPECT_EQ(primary, packet->primary);
+  EXPECT_EQ(priority, packet->priority);
   ASSERT_FALSE(packet->payload.empty());
   for (size_t i = 0; i < packet->payload.size(); ++i) {
     ASSERT_EQ(payload_value, packet->payload.data()[i]);
   }
 }
 
+void VerifyPacket(const Packet* packet,
+                  size_t payload_length,
+                  uint8_t payload_type,
+                  uint16_t sequence_number,
+                  uint32_t timestamp,
+                  uint8_t payload_value,
+                  bool primary) {
+  return VerifyPacket(packet, payload_length, payload_type, sequence_number,
+                      timestamp, payload_value,
+                      Packet::Priority{0, primary ? 0 : 1});
+}
+
 // Start of test definitions.
 
 TEST(PayloadSplitter, CreateAndDestroy) {
   PayloadSplitter* splitter = new PayloadSplitter;
   delete splitter;
 }
 
 // Packet A is split into A1 and A2.
 TEST(RedPayloadSplitter, OnePacketTwoPayloads) {
   uint8_t payload_types[] = {0, 0};
@@ skipping 69 matching lines @@
   // Manually change timestamp and sequence number of second packet.
   packet->header.timestamp += kTimestampOffset;
   packet->header.sequenceNumber++;
   packet_list.push_back(packet);
   PayloadSplitter splitter;
   EXPECT_EQ(PayloadSplitter::kOK, splitter.SplitRed(&packet_list));
   ASSERT_EQ(6u, packet_list.size());
   // Check first packet, A1.
   packet = packet_list.front();
   VerifyPacket(packet, kPayloadLength, payload_types[2], kSequenceNumber,
-               kBaseTimestamp, 2, true);
+               kBaseTimestamp, 2, {0, 0});
   delete packet;
   packet_list.pop_front();
   // Check second packet, A2.
   packet = packet_list.front();
   VerifyPacket(packet, kPayloadLength, payload_types[1], kSequenceNumber,
-               kBaseTimestamp - kTimestampOffset, 1, false);
+               kBaseTimestamp - kTimestampOffset, 1, {0, 1});
   delete packet;
   packet_list.pop_front();
   // Check third packet, A3.
   packet = packet_list.front();
   VerifyPacket(packet, kPayloadLength, payload_types[0], kSequenceNumber,
-               kBaseTimestamp - 2 * kTimestampOffset, 0, false);
+               kBaseTimestamp - 2 * kTimestampOffset, 0, {0, 2});
   delete packet;
   packet_list.pop_front();
   // Check fourth packet, B1.
   packet = packet_list.front();
   VerifyPacket(packet, kPayloadLength, payload_types[2], kSequenceNumber + 1,
-               kBaseTimestamp + kTimestampOffset, 2, true);
+               kBaseTimestamp + kTimestampOffset, 2, {0, 0});
   delete packet;
   packet_list.pop_front();
   // Check fifth packet, B2.
   packet = packet_list.front();
   VerifyPacket(packet, kPayloadLength, payload_types[1], kSequenceNumber + 1,
-               kBaseTimestamp, 1, false);
+               kBaseTimestamp, 1, {0, 1});
   delete packet;
   packet_list.pop_front();
   // Check sixth packet, B3.
   packet = packet_list.front();
   VerifyPacket(packet, kPayloadLength, payload_types[0], kSequenceNumber + 1,
-               kBaseTimestamp - kTimestampOffset, 0, false);
+               kBaseTimestamp - kTimestampOffset, 0, {0, 2});
   delete packet;
 }
 
 // Creates a list with 4 packets with these payload types:
 // 0 = CNGnb
 // 1 = PCMu
 // 2 = DTMF (AVT)
 // 3 = iLBC
 // We expect the method CheckRedPayloads to discard the iLBC packet, since it
 // is a non-CNG, non-DTMF payload of another type than the first speech payload
@@ skipping 43 matching lines @@
   packet->payload.SetSize(packet->payload.size() - (kPayloadLength + 1));
   PacketList packet_list;
   packet_list.push_back(packet);
   PayloadSplitter splitter;
   EXPECT_EQ(PayloadSplitter::kRedLengthMismatch,
             splitter.SplitRed(&packet_list));
   ASSERT_EQ(1u, packet_list.size());
   // Check first packet.
   packet = packet_list.front();
   VerifyPacket(packet, kPayloadLength, payload_types[0], kSequenceNumber,
-               kBaseTimestamp - 2 * kTimestampOffset, 0, false);
+               kBaseTimestamp - 2 * kTimestampOffset, 0, {0, 2});
   delete packet;
   packet_list.pop_front();
 }
-
-TEST(FecPayloadSplitter, MixedPayload) {

[ossu, 2016/09/14 16:21:07]

Since splitting is now done on a per-codec basis, I can't really find a place
for this test as it is. There's nothing we can hand both a PCMU payload and an
Opus payload and make sure one of them is split while the other is not. There
should maybe be a test on the Opus codec that FEC payloads are identified and
split properly, though.

[hlundin-webrtc, 2016/09/15 09:33:30]

Acknowledged.

-  PacketList packet_list;
-  DecoderDatabase decoder_database(CreateBuiltinAudioDecoderFactory());
-
-  decoder_database.RegisterPayload(0, NetEqDecoder::kDecoderOpus, "opus");
-  decoder_database.RegisterPayload(1, NetEqDecoder::kDecoderPCMu, "pcmu");
-
-  Packet* packet = CreatePacket(0, 10, 0xFF, true);
-  packet_list.push_back(packet);
-
-  packet = CreatePacket(0, 10, 0); // Non-FEC Opus payload.
-  packet_list.push_back(packet);
-
-  packet = CreatePacket(1, 10, 0); // Non-Opus payload.
-  packet_list.push_back(packet);
-
-  PayloadSplitter splitter;
-  EXPECT_EQ(PayloadSplitter::kOK,
-            splitter.SplitFec(&packet_list, &decoder_database));
-  EXPECT_EQ(4u, packet_list.size());
-
-  // Check first packet.
-  packet = packet_list.front();
-  EXPECT_EQ(0, packet->header.payloadType);
-  EXPECT_EQ(kBaseTimestamp - 20 * 48, packet->header.timestamp);
-  EXPECT_EQ(10U, packet->payload.size());
-  EXPECT_FALSE(packet->primary);
-  delete packet;
-  packet_list.pop_front();
-
-  // Check second packet.
-  packet = packet_list.front();
-  EXPECT_EQ(0, packet->header.payloadType);
-  EXPECT_EQ(kBaseTimestamp, packet->header.timestamp);
-  EXPECT_EQ(10U, packet->payload.size());
-  EXPECT_TRUE(packet->primary);
-  delete packet;
-  packet_list.pop_front();
-
-  // Check third packet.
-  packet = packet_list.front();
-  VerifyPacket(packet, 10, 0, kSequenceNumber, kBaseTimestamp, 0, true);
-  delete packet;
-  packet_list.pop_front();
-
-  // Check fourth packet.
-  packet = packet_list.front();
-  VerifyPacket(packet, 10, 1, kSequenceNumber, kBaseTimestamp, 0, true);
-  delete packet;
-}
-
-TEST(FecPayloadSplitter, EmbedFecInRed) {

[ossu, 2016/09/14 16:21:07]

As RED and FEC is now indicated separately, rather than sharing the primary
flag, there's not much point in testing them together. To retain this test, it
would have to test that NetEq does the right assignments (i.e. the SplitRed
value goes into red_level and the codec's priority goes into codec_level).
That's a lot of testing for two lines of very simple code. Perhaps this test
could be turned into one testing that Packet::Priority orders things properly?

-  PacketList packet_list;
-  DecoderDatabase decoder_database(CreateBuiltinAudioDecoderFactory());
-
-  const int kTimestampOffset = 20 * 48;  // 20 ms * 48 kHz.
-  uint8_t payload_types[] = {0, 0};
-  decoder_database.RegisterPayload(0, NetEqDecoder::kDecoderOpus, "opus");
-  Packet* packet = CreateRedPayload(2, payload_types, kTimestampOffset, true);
-  packet_list.push_back(packet);
-
-  PayloadSplitter splitter;
-  EXPECT_EQ(PayloadSplitter::kOK,
-            splitter.SplitRed(&packet_list));
-  EXPECT_EQ(PayloadSplitter::kOK,
-            splitter.SplitFec(&packet_list, &decoder_database));
-
-  EXPECT_EQ(4u, packet_list.size());
-
-  // Check first packet. FEC packet copied from primary payload in RED.
-  packet = packet_list.front();
-  EXPECT_EQ(0, packet->header.payloadType);
-  EXPECT_EQ(kBaseTimestamp - kTimestampOffset, packet->header.timestamp);
-  EXPECT_EQ(kPayloadLength, packet->payload.size());
-  EXPECT_FALSE(packet->primary);
-  EXPECT_EQ(packet->payload[3], 1);
-  delete packet;
-  packet_list.pop_front();
-
-  // Check second packet. Normal packet copied from primary payload in RED.
-  packet = packet_list.front();
-  EXPECT_EQ(0, packet->header.payloadType);
-  EXPECT_EQ(kBaseTimestamp, packet->header.timestamp);
-  EXPECT_EQ(kPayloadLength, packet->payload.size());
-  EXPECT_TRUE(packet->primary);
-  EXPECT_EQ(packet->payload[3], 1);
-  delete packet;
-  packet_list.pop_front();
-
-  // Check third packet. FEC packet copied from secondary payload in RED.
-  packet = packet_list.front();
-  EXPECT_EQ(0, packet->header.payloadType);
-  EXPECT_EQ(kBaseTimestamp - 2 * kTimestampOffset, packet->header.timestamp);
-  EXPECT_EQ(kPayloadLength, packet->payload.size());
-  EXPECT_FALSE(packet->primary);
-  EXPECT_EQ(packet->payload[3], 0);
-  delete packet;
-  packet_list.pop_front();
-
-  // Check fourth packet. Normal packet copied from primary payload in RED.
-  packet = packet_list.front();
-  EXPECT_EQ(0, packet->header.payloadType);
-  EXPECT_EQ(kBaseTimestamp - kTimestampOffset, packet->header.timestamp);
-  EXPECT_EQ(kPayloadLength, packet->payload.size());
-  EXPECT_TRUE(packet->primary);
-  EXPECT_EQ(packet->payload[3], 0);
-  delete packet;
-  packet_list.pop_front();
-}
 
 }  // namespace webrtc