Packetizer/Depacketizer for RTP.

remoting/protocol/rtp_utils.cc

Index: remoting/protocol/rtp_utils.cc
diff --git a/remoting/protocol/rtp_utils.cc b/remoting/protocol/rtp_utils.cc
index 133779511362ee6b579eda37a27b255662311373..c8f8c730a5c1d6506368d44d4c5fb26fd5603718 100644
--- a/remoting/protocol/rtp_utils.cc
+++ b/remoting/protocol/rtp_utils.cc
@@ -21,17 +21,35 @@ const uint8 kRtpVersionNumber = 2;
 const int kRtpMaxSources = 16;
 }  // namespace
 
-int GetRtpHeaderSize(int sources) {
-  DCHECK_GE(sources, 0);
-  DCHECK_LT(sources, kRtpMaxSources);
-  return kRtpBaseHeaderSize + sources * 4;
+static inline uint8 ExtractBits(uint8 byte, int bits_count, int shift) {
+  return (byte >> shift) & ((1 << bits_count) - 1);
+}
+
+// RTP Header format:
+//
+//  0                   1                   2                   3
+//  0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+// |V=2|P|X|  CC   |M|     PT      |       sequence number         |
+// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+// |                           timestamp                           |
+// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+// |           synchronization source (SSRC) identifier            |
+// +=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+

[awong, 2010/11/16 01:08:40]

nice diagram.:)

[Sergey Ulanov, 2010/11/16 01:52:25]

I didn't draw it myself, it's from RTP spec :)

+// |            contributing source (CSRC) identifiers             |
+// |                             ....                              |
+// +=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+
+
+int GetRtpHeaderSize(const RtpHeader& header) {
+  DCHECK_GE(header.sources, 0);
+  DCHECK_LT(header.sources, kRtpMaxSources);
+  return kRtpBaseHeaderSize + header.sources * 4;
 }
 
-void PackRtpHeader(uint8* buffer, int buffer_size,
-                   const RtpHeader& header) {
+void PackRtpHeader(const RtpHeader& header, uint8* buffer, int buffer_size) {
   DCHECK_LT(header.sources, kRtpMaxSources);
   DCHECK_LT(header.payload_type, 1 << 7);
-  CHECK_GT(buffer_size, GetRtpHeaderSize(header.sources));
+  CHECK_GE(buffer_size, GetRtpHeaderSize(header));
 
   buffer[0] = (kRtpVersionNumber << 6) +
       ((uint8)header.padding << 5) +
@@ -48,10 +66,6 @@ void PackRtpHeader(uint8* buffer, int buffer_size,
   }
 }
 
-static inline uint8 ExtractBits(uint8 byte, int bits_count, int shift) {
-  return (byte >> shift) & ((1 << bits_count) - 1);
-}
-
 int UnpackRtpHeader(const uint8* buffer, int buffer_size, RtpHeader* header) {
   if (buffer_size < kRtpBaseHeaderSize) {
     return -1;
@@ -75,14 +89,137 @@ int UnpackRtpHeader(const uint8* buffer, int buffer_size, RtpHeader* header) {
 
   DCHECK_LT(header->sources, 16);
 
-  if (buffer_size < GetRtpHeaderSize(header->sources)) {
+  if (buffer_size < GetRtpHeaderSize(*header)) {
     return -1;
   }
   for (int i = 0; i < header->sources; i++) {
     header->source_id[i] = GetBE32(buffer + i * 4 + 12);
   }
 
-  return GetRtpHeaderSize(header->sources);
+  return GetRtpHeaderSize(*header);
+}
+
+// VP8 Payload Descriptor format:
+//
+//  0                   1                   2                   3
+//  0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+// | RSV |I|N|FI |B|         PictureID (integer #bytes)            |
+// +-+-+-+-+-+-+-+-+                                               |
+// :                                                               :
+// |               +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+// |               : (VP8 data or VP8 payload header; byte aligned)|
+// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+//
+// I: 1 bit

[Alpha Left Google, 2010/11/15 23:46:35]

Doesn't say what is RSV in the description.

[Sergey Ulanov, 2010/11/16 00:22:55]

Done.

+// PictureID present. When set to one, a PictureID is provided after
+// the first byte of the payload descriptor. When set to zero, the
+// PictureID is omitted, and the one-byte payload descriptor is
+// immediately followed by the VP8 payload.
+//
+// N: 1 bit
+// Non-reference frame. When set to one, the frame can be discarded
+// without affecting any other future or past frames.
+//
+// FI: 2 bits
+// Fragmentation information field. This field contains information
+// about the fragmentation of VP8 payloads carried in the RTP
+// packet. The four different values are listed below.
+//
+// FI   Fragmentation status
+//   00 The RTP packet contains no fragmented VP8 partitions. The
+//      payload is one or several complete partitions.
+//   01 The RTP packet contains the first part of a fragmented
+//      partition. The fragment must be placed in its own RTP packet.
+//   10 The RTP packet contains a fragment that is neither the first nor
+//      the last part of a fragmented partition. The fragment must be
+//      placed in its own RTP packet.
+//   11 The RTP packet contains the last part of a fragmented
+//      partition. The fragment must be placed in its own RTP packet.
+//
+// B: 1 bit
+// Beginning VP8 frame. When set to 1 this signals that a new VP8
+// frame starts in this RTP packet.
+//
+// PictureID: Multiple of 8 bits
+// This is a running index of the frames. The field is present only if
+// the I bit is equal to one. The most significant bit of each byte is
+// an extension flag. The 7 following bits carry (parts of) the
+// PictureID. If the extension flag is one, the PictureID continues in
+// the next byte. If the extension flag is zero, the 7 remaining bits
+// are the last (and least significant) bits in the PictureID. The
+// sender may choose any number of bytes for the PictureID. The
+// PictureID SHALL start on a random number, and SHALL wrap after
+// reaching the maximum ID as chosen by the application
+
+int GetVp8DescriptorSize(const Vp8Descriptor& descriptor) {
+  if (descriptor.picture_id == kuint32max)
+    return 1;
+  int result = 2;
+  // We need 1 byte per each 7 bits in picture_id.
+  uint32 picture_id = descriptor.picture_id >> 7;
+  while (picture_id > 0) {
+    picture_id >>= 7;
+    ++result;
+  }
+  return result;
+}
+
+void PackVp8Descriptor(const Vp8Descriptor& descriptor, uint8* buffer,
+                       int buffer_size) {
+  CHECK_GT(buffer_size, 0);

[Alpha Left Google, 2010/11/15 23:46:35]

don't use CHECK here.

[Sergey Ulanov, 2010/11/16 00:22:55]

Why? I think, it is better to use CHECK here to prevent potential security risk.

[awong, 2010/11/16 01:08:40]

I think CHECK makes sense.  allowing a negative size in production code sounds
dangerous.

+
+  buffer[0] =

[Alpha Left Google, 2010/11/15 23:46:35]

This doesn't seem to match the diagram above..

For example B is bit 7 but according to this state it is bit 4.

[Sergey Ulanov, 2010/11/16 00:22:55]

B is frame_beginning. It is 7'th bit in the first byte, i.e. lowest bit (keep in
mind that the diagrams are big-endian).
I = (picture_id != kuint32max), it is 3'rd bit.

[Alpha Left Google, 2010/11/16 00:47:08]

I see.. if this is the case this code won't work on BE system isn't it?
Shouldn't we just assume this is a regular integer and does a SetBE(value) at
the end?

+      ((uint8)(descriptor.picture_id != kuint32max) << 4) +

[awong, 2010/11/16 01:08:40]

Since we're playing in bits, I'm personally happier seeing | over +.  Any reason
to choose +?

[Sergey Ulanov, 2010/11/16 01:52:25]

Agree that | is better here. Fixed here and in PackRtpHeader.

+      ((uint8)descriptor.non_reference_frame << 3) +
+      (descriptor.fragmentation_info << 1) +
+      ((uint8)descriptor.frame_beginning);
+
+  uint32 picture_id = descriptor.picture_id;
+  if (picture_id == kuint32max)
+    return;
+
+  int pos = 1;
+  while (picture_id > 0) {
+    CHECK_LT(pos, buffer_size);

[Alpha Left Google, 2010/11/15 23:46:35]

and here.

[Sergey Ulanov, 2010/11/16 00:22:55]

Same reason as above. The check is fairly cheap, and we risk creating security
holes without it.

+    buffer[pos] = picture_id & 0x7F;
+    picture_id >>= 7;
+
+    // Set the extension bit if neccessary.
+    if (picture_id > 0)
+      buffer[pos] |= 1 << 7;
+    ++pos;
+  }
+}
+
+int UnpackVp8Descriptor(const uint8* buffer, int buffer_size,
+                        Vp8Descriptor* descriptor) {
+  if (buffer_size <= 0)
+    return -1;
+
+  bool picture_id_present = ExtractBits(buffer[0], 1, 4) != 0;

[Alpha Left Google, 2010/11/15 23:46:35]

What about 
= !!ExtractBits(...);

[Sergey Ulanov, 2010/11/16 00:22:55]

IMHO, !=0 looks nicer, no?

+  descriptor->non_reference_frame = ExtractBits(buffer[0], 1, 3) != 0;

[Alpha Left Google, 2010/11/15 23:46:35]

ditto.

+  descriptor->fragmentation_info = ExtractBits(buffer[0], 2, 1);
+  descriptor->frame_beginning = ExtractBits(buffer[0], 1, 0) != 0;

[Alpha Left Google, 2010/11/15 23:46:35]

ditto.

+
+  // Decode PictureID if it is present.
+  if (picture_id_present) {
+    bool extension = true;
+    int pos = 1;
+    descriptor->picture_id = 0;
+    while (extension) {
+      if (pos >= buffer_size)
+        return -1;
+
+      descriptor->picture_id |= buffer[pos] & 0x7F;
+      extension = (buffer[pos] & 0x80) != 0;
+      pos += 1;
+    }
+    return pos;
+  } else {
+    descriptor->picture_id = kuint32max;
+    return 1;
+  }
 }
 
 }  // namespace protocol