Moved Opus-specific payload splitting into AudioDecoderOpus.

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 b28f77739cf3da7372b1b1836a6c016877c0f935..97dbb109aaefce2b07b3dfa4ef515bd2ce628dae 100644
--- a/webrtc/modules/audio_coding/codecs/audio_decoder.h
+++ b/webrtc/modules/audio_coding/codecs/audio_decoder.h
@@ -61,7 +61,7 @@ class AudioDecoder {
   struct ParseResult {
     ParseResult();
     ParseResult(uint32_t timestamp,
-                bool primary,
+                uint8_t priority,

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

"You should not use the unsigned integer types such as uint32_t, unless there is
a valid reason such as representing a bit pattern rather than a number, or you
need defined overflow modulo 2^N. In particular, do not use unsigned types to
say a number will never be negative. Instead, use assertions for this."
https://google.github.io/styleguide/cppguide.html#Integer_Types

[ossu, 2016/09/15 12:22:53]

You are technically correct - the best kind of correct! Perhaps these should
just be ints after all. Seems like a waste of bytes, though. Not that they are
very expensive...

[hlundin-webrtc, 2016/09/16 07:51:27]

Once upon a time, in an office building (not so) far far away, we used to spend
a lot of time tweaking the size of member variables. This code is more int than
uint8_t these days...

[kwiberg-webrtc, 2016/09/19 11:07:49]

Function arguments and local variables are stored in registers most of the time
unless they're live when you call a subroutine, and then (almost) any sort of
integer will occupy a full register.

+1 for making this an int.

[ossu, 2016/09/19 11:41:01]

I've thought about this change as also applying to Packet::Priority, which is
where the size "actually" matters. There having two ints where none can be < 0
and both will be really small is more of a waste, and a hassle. I can have them
like that, though, with CHECKs for < 0, but I think we're throwing away some
expressiveness in the API.

[kwiberg-webrtc, 2016/09/19 11:55:16]

I also thought about this change as applying to Packet::Priority. Why would you
need to CHECK that the values are nonnegative?

[ossu, 2016/09/19 14:07:33]

If we want some priority level to correspond to "normal audio packet from a
non-RED payload or primary RED-sub-block", e.g. for stats.

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

Ah, yes, for stats I can see why we'd care about the "absolute" priority and not
just the relative priority. Thanks.

                 std::unique_ptr<EncodedAudioFrame> frame);
     ParseResult(ParseResult&& b);
     ~ParseResult();
@@ -70,7 +70,9 @@ class AudioDecoder {
 
     // The timestamp of the frame is in samples per channel.
     uint32_t timestamp;
-    bool primary;
+    // The relative priority of the frame compared to other frames of the same
+    // payload and the same timeframe. A higher value means a lower priority.
+    uint8_t priority;
     std::unique_ptr<EncodedAudioFrame> frame;
   };
 
@@ -78,8 +80,7 @@ class AudioDecoder {
   // frames. Each frame must be at most 120 ms long. The decoder is free to swap
   // or move the data from the |payload| buffer.
   virtual std::vector<ParseResult> ParsePayload(rtc::Buffer* payload,
-                                                uint32_t timestamp,
-                                                bool is_primary);
+                                                uint32_t timestamp);
 
   // Decodes |encode_len| bytes from |encoded| and writes the result in
   // |decoded|. The maximum bytes allowed to be written into |decoded| is