Moved Opus-specific payload splitting into AudioDecoderOpus.

webrtc/modules/audio_coding/neteq/red_payload_splitter.h

Index: webrtc/modules/audio_coding/neteq/red_payload_splitter.h
diff --git a/webrtc/modules/audio_coding/neteq/payload_splitter.h b/webrtc/modules/audio_coding/neteq/red_payload_splitter.h
similarity index 56%
rename from webrtc/modules/audio_coding/neteq/payload_splitter.h
rename to webrtc/modules/audio_coding/neteq/red_payload_splitter.h
index a3e1b1d91c76bd047995407d0f8d80932941e4a4..deb86eb166d7f2c48e0757640a8f4bf5f30313c8 100644
--- a/webrtc/modules/audio_coding/neteq/payload_splitter.h
+++ b/webrtc/modules/audio_coding/neteq/red_payload_splitter.h
@@ -8,8 +8,8 @@
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
-#ifndef WEBRTC_MODULES_AUDIO_CODING_NETEQ_PAYLOAD_SPLITTER_H_
-#define WEBRTC_MODULES_AUDIO_CODING_NETEQ_PAYLOAD_SPLITTER_H_
+#ifndef WEBRTC_MODULES_AUDIO_CODING_NETEQ_RED_PAYLOAD_SPLITTER_H_
+#define WEBRTC_MODULES_AUDIO_CODING_NETEQ_RED_PAYLOAD_SPLITTER_H_
 
 #include "webrtc/base/constructormagic.h"
 #include "webrtc/modules/audio_coding/neteq/packet.h"
@@ -19,24 +19,14 @@ namespace webrtc {
 // Forward declarations.
 class DecoderDatabase;
 
-// This class handles splitting of payloads into smaller parts.
-
-// For RED and FEC the splitting is done internally. Other codecs' packets are
-// split by calling AudioDecoder::SplitPacket.
-class PayloadSplitter {
+// This class handles splitting of RED payloads into smaller parts.
+// Codec-specific packet splitting can be performed by
+// AudioDecoder::ParsePayload.
+class RedPayloadSplitter {

[kwiberg-webrtc, 2016/09/20 14:56:23]

Could you split this class into an interface and a private implementation? That
hides the implementation better and lets the compiler catch more mistakes for
us, for no runtime cost.

[ossu, 2016/09/20 15:45:21]

I think this goes outside the remit of this CL, IMHO. Also not sure why we'd
want to hide (or for that matter mock, as we do) this functionality. It's got no
state and should probably be moved into free functions somewhere.

[kwiberg-webrtc, 2016/09/20 16:19:15]

Fair enough. (This is a pervasive problem in NetEq. I thought we could chip away
at it one class at a time.)
There are unit tests that test the main NetEq object with its innards replaced
with mocks. Not sure if that's a good enough reason in this particular case,
though---I haven't checked if the tests in question are useful.

[ossu, 2016/09/21 10:23:13]

After a quick check, I'm not convinced that Mock(Red)PayloadSplitter is ever
used in the neteq_impl_unittests.c. Nothing seems to be setting up the mock
through EXPECT_CALL, so if it were used (and called by NetEqImpl), we should be
getting warnings from gmock about calling uninteresting mocks.

  public:
-  enum SplitterReturnCodes {
-    kOK = 0,
-    kNoSplit = 1,
-    kFrameSplitError = -2,
-    kUnknownPayloadType = -3,
-    kRedLengthMismatch = -4,
-    kFecSplitError = -5,
-  };
-
-  PayloadSplitter() {}
+  RedPayloadSplitter() {}
 
-  virtual ~PayloadSplitter() {}
+  virtual ~RedPayloadSplitter() {}
 
   // Splits each packet in |packet_list| into its separate RED payloads. Each
   // RED payload is packetized into a Packet. The original elements in
@@ -44,23 +34,18 @@ class PayloadSplitter {
   // Note that all packets in |packet_list| must be RED payloads, i.e., have
   // RED headers according to RFC 2198 at the very beginning of the payload.
   // Returns kOK or an error.
-  virtual int SplitRed(PacketList* packet_list);
-
-  // Iterates through |packet_list| and, duplicate each audio payload that has
-  // FEC as new packet for redundant decoding. The decoder database is needed to
-  // get information about which payload type each packet contains.
-  virtual int SplitFec(PacketList* packet_list,
-                       DecoderDatabase* decoder_database);
+  virtual bool SplitRed(PacketList* packet_list);
 
   // Checks all packets in |packet_list|. Packets that are DTMF events or
   // comfort noise payloads are kept. Except that, only one single payload type
-  // is accepted. Any packet with another payload type is discarded.
+  // is accepted. Any packet with another payload type is discarded.  Returns
+  // the number of discarded packets.
   virtual int CheckRedPayloads(PacketList* packet_list,
                                const DecoderDatabase& decoder_database);
 
  private:
-  RTC_DISALLOW_COPY_AND_ASSIGN(PayloadSplitter);
+  RTC_DISALLOW_COPY_AND_ASSIGN(RedPayloadSplitter);
 };
 
 }  // namespace webrtc
-#endif  // WEBRTC_MODULES_AUDIO_CODING_NETEQ_PAYLOAD_SPLITTER_H_
+#endif  // WEBRTC_MODULES_AUDIO_CODING_NETEQ_RED_PAYLOAD_SPLITTER_H_