Removed RTPHeader from NetEq's Packet struct.

webrtc/modules/audio_coding/neteq/packet.h

 /*
  *  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.
  */
 
 #ifndef WEBRTC_MODULES_AUDIO_CODING_NETEQ_PACKET_H_
 #define WEBRTC_MODULES_AUDIO_CODING_NETEQ_PACKET_H_
 
 #include <list>
 #include <memory>
 
 #include "webrtc/base/buffer.h"
 #include "webrtc/modules/audio_coding/codecs/audio_decoder.h"
 #include "webrtc/modules/audio_coding/neteq/tick_timer.h"
 #include "webrtc/modules/include/module_common_types.h"

[hlundin-webrtc, 2016/10/12 20:39:57]

Can you ditch module_common_types.h now?

[ossu, 2016/10/12 21:46:02]

Yes, but I'll have to put it in packet_buffer.h instead.

[hlundin-webrtc, 2016/10/13 07:54:18]

Acknowledged.

 #include "webrtc/typedefs.h"
 
 namespace webrtc {
 
 // Struct for holding RTP packets.
 struct Packet {
   struct Priority {
     Priority() : codec_level(0), red_level(0) {}
     Priority(int codec_level, int red_level)
         : codec_level(codec_level), red_level(red_level) {
@@ skipping 28 matching lines @@
     bool operator<=(const Priority& b) const { return !(b > *this); }
     bool operator>=(const Priority& b) const { return !(b < *this); }
 
    private:
     void CheckInvariant() const {
       RTC_DCHECK_GE(codec_level, 0);
       RTC_DCHECK_GE(red_level, 0);
     }
   };
 
-  RTPHeader header;
+  uint32_t timestamp;
+  uint16_t sequenceNumber;

[hlundin-webrtc, 2016/10/12 20:39:58]

Please, make the member names follow the style guide: sequence_number and
payload_type. I would also argue that payload_type should be an int. The other
two need defined wrap-around to work properly, motivating the uints. (I do
appreciate that not changing the names in the first patch set makes reviewing
easier, so please, change the names in a separate patch set.)

[ossu, 2016/10/12 21:46:02]

Payload types are uint8_t in so many other places, I don't think it makes sense
to change it just here. It could be changed throughout in a separate CL.

Since they have a fixed, defined range of 0-127, I think, if anything, they
should be int8_t. That way we get away from the unsigned type (which the style
guide likes) and can check for invalidity by checking if payload type < 0
whenever we want to do such things.

Finally, eventually we'll probably move away from referencing payload types
directly down here, but leave that to an RTP specific layer above NetEq, to
accommodate other transports. We might as well tackle this issue at that time.

[hlundin-webrtc, 2016/10/13 07:54:18]

Acknowledged.

+  uint8_t payloadType;
   // Datagram excluding RTP header and header extension.
   rtc::Buffer payload;
   Priority priority;
   std::unique_ptr<TickTimer::Stopwatch> waiting_time;
   std::unique_ptr<AudioDecoder::EncodedAudioFrame> frame;
 
   Packet();
   ~Packet();
 
   // Comparison operators. Establish a packet ordering based on (1) timestamp,
   // (2) sequence number and (3) redundancy.
   // Timestamp and sequence numbers are compared taking wrap-around into
   // account. For two packets with the same sequence number and timestamp a
   // primary payload is considered "smaller" than a secondary.
   bool operator==(const Packet& rhs) const {
-    return (this->header.timestamp == rhs.header.timestamp &&
-            this->header.sequenceNumber == rhs.header.sequenceNumber &&
+    return (this->timestamp == rhs.timestamp &&
+            this->sequenceNumber == rhs.sequenceNumber &&
             this->priority == rhs.priority);
   }
   bool operator!=(const Packet& rhs) const { return !operator==(rhs); }
   bool operator<(const Packet& rhs) const {
-    if (this->header.timestamp == rhs.header.timestamp) {
-      if (this->header.sequenceNumber == rhs.header.sequenceNumber) {
+    if (this->timestamp == rhs.timestamp) {
+      if (this->sequenceNumber == rhs.sequenceNumber) {
         // Timestamp and sequence numbers are identical - deem the left hand
         // side to be "smaller" (i.e., "earlier") if it has higher priority.
         return this->priority < rhs.priority;
       }
-      return (static_cast<uint16_t>(rhs.header.sequenceNumber
-          - this->header.sequenceNumber) < 0xFFFF / 2);
+      return (static_cast<uint16_t>(rhs.sequenceNumber - this->sequenceNumber) <
+              0xFFFF / 2);
     }
-    return (static_cast<uint32_t>(rhs.header.timestamp
-        - this->header.timestamp) < 0xFFFFFFFF / 2);
+    return (static_cast<uint32_t>(rhs.timestamp - this->timestamp) <
+            0xFFFFFFFF / 2);
   }
   bool operator>(const Packet& rhs) const { return rhs.operator<(*this); }
   bool operator<=(const Packet& rhs) const { return !operator>(rhs); }
   bool operator>=(const Packet& rhs) const { return !operator<(rhs); }
 
   bool empty() const { return !frame && payload.empty(); }
 };
 
 // A list of packets.
 typedef std::list<Packet*> PacketList;
 
 }  // namespace webrtc
 #endif  // WEBRTC_MODULES_AUDIO_CODING_NETEQ_PACKET_H_