Moved codec-specific audio packet splitting into decoders.

webrtc/modules/audio_coding/codecs/audio_decoder.h

Index: webrtc/modules/audio_coding/codecs/audio_decoder.h
diff --git a/webrtc/modules/audio_coding/codecs/audio_decoder.h b/webrtc/modules/audio_coding/codecs/audio_decoder.h
index 13581bc247705a740b04e1d9a1cac73e454b3fa3..cd01b8e03d4399d6c24a9f1b9afbcea98c6ed4bd 100644
--- a/webrtc/modules/audio_coding/codecs/audio_decoder.h
+++ b/webrtc/modules/audio_coding/codecs/audio_decoder.h
@@ -11,7 +11,7 @@
 #ifndef WEBRTC_MODULES_AUDIO_CODING_NETEQ_INCLUDE_AUDIO_DECODER_H_
 #define WEBRTC_MODULES_AUDIO_CODING_NETEQ_INCLUDE_AUDIO_DECODER_H_
 
-#include <stdlib.h>  // NULL
+#include <vector>
 
 #include "webrtc/base/constructormagic.h"
 #include "webrtc/typedefs.h"
@@ -30,6 +30,14 @@ class AudioDecoder {
   // Used by PacketDuration below. Save the value -1 for errors.
   enum { kNotImplemented = -2 };
 
+  struct PacketSplit {
+    size_t byte_offset;
+    size_t num_bytes;
+    size_t timestamp_offset;
+  };
+
+  typedef std::vector<PacketSplit> PacketSplits;

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

I'm not sure it's worth having a typedef for this. It's not that much shorter,
and it obscures what the type is.

+
   AudioDecoder() = default;
   virtual ~AudioDecoder() = default;
 
@@ -74,6 +82,14 @@ class AudioDecoder {
                              uint32_t rtp_timestamp,
                              uint32_t arrival_timestamp);
 
+  // Calculates positions where the packet can be split, and the respective
+  // timestamps of those positions. Returns a single PacketSplit spanning the
+  // whole payload (i.e. { 0, payload_len, 0 }) if the packet should not be
+  // split. This is the default behavior. If the packet is invalid, it is
+  // possible to return an empty PacketSplits vector.

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

Should we note that this (and the other static function below) are experimental
and shouldn't be used?

+  virtual PacketSplits SplitPacket(const uint8_t* payload,
+                                   size_t payload_len) const;

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

Consider using a single rtc::ArrayView<const uint8_t> argument instead.

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

I've used this since it's how the rest of the API looks. An ArrayView is better,
but not at the cost of conformance, IMO.

[kwiberg-webrtc, 2016/08/26 22:05:13]

That's unavoidable if you want a gradual shift from old to new style. But
switching everything at once is problematic in other ways, as is not switching.

+
   // Returns the last error code from the decoder.
   virtual int ErrorCode();
 
@@ -116,6 +132,12 @@ class AudioDecoder {
                                       int16_t* decoded,
                                       SpeechType* speech_type);
 
+  // Splits a payload consisting of inidividual samples into reasonable chunks.
+  // Utility function for use by sample-based codecs to implement SplitPacket.

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

Hmm. If things are expected to change further, maybe this utility should live
somewhere else than in our public API.

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

Yeah, I don't love it here... It could be useful to provide this as a utility
for others to use as well, however they could just copy the functionality.
Anyway, it should maybe be in some separate utility header, possibly an internal
one.

+  static PacketSplits SplitBySamples(const uint8_t* payload,
+                                     size_t payload_len,
+                                     size_t bytes_per_ms,
+                                     uint32_t timestamps_per_ms);

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

Consider replacing the first two arguments with an rtc::ArrayView<const
uint8_t>.

  private:
   RTC_DISALLOW_COPY_AND_ASSIGN(AudioDecoder);
 };