[rtp_rtcp] rtcp::Sdes cleaned and got Parse function

webrtc/modules/rtp_rtcp/source/rtcp_packet/sdes.cc

 /*
  *  Copyright (c) 2016 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.
  */
 
 #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/sdes.h"
 
+#include "webrtc/base/checks.h"
 #include "webrtc/base/logging.h"
 #include "webrtc/modules/rtp_rtcp/source/byte_io.h"
 
-using webrtc::RTCPUtility::PT_SDES;
+using webrtc::RTCPUtility::RtcpCommonHeader;
 
 namespace webrtc {
 namespace rtcp {
-namespace {
-void AssignUWord8(uint8_t* buffer, size_t* offset, uint8_t value) {
-  buffer[(*offset)++] = value;
-}
-
-void AssignUWord32(uint8_t* buffer, size_t* offset, uint32_t value) {
-  ByteWriter<uint32_t>::WriteBigEndian(buffer + *offset, value);
-  *offset += 4;
-}
 // Source Description (SDES) (RFC 3550).
 //
 //         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
 //        +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
 // header |V=2|P|    SC   |  PT=SDES=202  |             length            |
 //        +=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+
 // chunk  |                          SSRC/CSRC_1                          |
 //   1    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
 //        |                           SDES items                          |
 //        |                              ...                              |
 //        +=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+
 // chunk  |                          SSRC/CSRC_2                          |
 //   2    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
 //        |                           SDES items                          |
 //        |                              ...                              |
 //        +=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+
 //
 // Canonical End-Point Identifier SDES Item (CNAME)
 //
 //    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
 //   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
 //   |    CNAME=1    |     length    | user and domain name        ...
 //   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-void CreateSdes(const std::vector<Sdes::Chunk>& chunks,
-                uint8_t* buffer,
-                size_t* pos) {
-  const uint8_t kSdesItemType = 1;
-  for (std::vector<Sdes::Chunk>::const_iterator it = chunks.begin();
-       it != chunks.end(); ++it) {
-    AssignUWord32(buffer, pos, (*it).ssrc);
-    AssignUWord8(buffer, pos, kSdesItemType);
-    AssignUWord8(buffer, pos, (*it).name.length());
-    memcpy(buffer + *pos, (*it).name.data(), (*it).name.length());
-    *pos += (*it).name.length();
-    memset(buffer + *pos, 0, (*it).null_octets);
-    *pos += (*it).null_octets;
-  }
+namespace {
+size_t ChunkSize(const Sdes::Chunk& chunk) {
+  // Chunk:
+  // SSRC/CSRC (4 bytes) | CNAME=1 (1 byte) | length (1 byte) | cname | padding.
+  size_t chunk_payload_size = 4 + 1 + 1 + chunk.cname.size();
+  size_t padding_size = 4 - (chunk_payload_size % 4);  // Minimum 1.
+  return chunk_payload_size + padding_size;
 }
 }  // namespace
 
+Sdes::Sdes() : block_length_(RtcpPacket::kHeaderLength) {}
+
+Sdes::~Sdes() {}
+
+bool Sdes::Parse(const RtcpCommonHeader& header, const uint8_t* payload) {
+  RTC_CHECK(header.packet_type == kPacketType);
+
+  const uint8_t number_of_chunks = header.count_or_format;
+  std::vector<Chunk> chunks;  // Read chunk into temporary array, so that in
+                              // case of an error original array would stay
+                              // unchanged.
+  size_t block_length = kHeaderLength;
+
+  if (header.payload_size_bytes % 4 != 0) {
+    LOG(LS_WARNING) << "Invalid payload size " << header.payload_size_bytes
+                    << " bytes for a valid Sdes packet. Size should be"
+                       " multiple of 4 bytes";
+  }
+  const uint8_t* const payload_end = payload + header.payload_size_bytes;
+  const uint8_t* looking_at = payload;
+  chunks.resize(number_of_chunks);
+  for (size_t i = 0; i < number_of_chunks; ++i) {
+    // Each chunk consumes at least 8 bytes.
+    if (payload_end - looking_at < 8) {
+      LOG(LS_WARNING) << "Not enough space left for chunk #" << (i + 1);
+      return false;
+    }
+    chunks[i].ssrc = ByteReader<uint32_t>::ReadBigEndian(looking_at);
+    looking_at += sizeof(uint32_t);
+    bool cname_found = false;
+
+    const uint8_t kTerminateItemType = 0;
+    const uint8_t kCnameItemType = 1;

[åsapersson, 2016/02/11 10:20:32]

use same name here and in Create (kCnameId)

[danilchap, 2016/02/11 11:13:37]

Using now same name here, in Create and in unittests (kCnameTag)

+    uint8_t item_type;
+    while ((item_type = *(looking_at++)) != kTerminateItemType) {
+      if (looking_at >= payload_end) {
+        LOG(LS_WARNING) << "Unexpected end of packet while reading chunk #"
+                        << (i + 1) << ". Expected to find size of the text.";
+        return false;
+      }
+      uint8_t size = *(looking_at++);

[åsapersson, 2016/02/11 10:20:32]

maybe call length as in figure

[danilchap, 2016/02/11 11:13:37]

Done.

+      if (looking_at + size + 1 > payload_end) {

[åsapersson, 2016/02/11 10:20:32]

maybe looking_at + size >= payload_end

[danilchap, 2016/02/11 11:13:37]

+1 has a different meaning here. Explained with a constant.

+        LOG(LS_WARNING) << "Unexpected end of packet while reading chunk #"
+                        << (i + 1) << ". Expected to find text of size "
+                        << size;
+        return false;
+      }
+      if (item_type == kCnameItemType) {
+        if (cname_found) {
+          LOG(LS_WARNING) << "Found extra CNAME for same ssrc in chunk #"
+                          << (i + 1);
+          return false;
+        }
+        cname_found = true;
+        chunks[i].cname.assign(reinterpret_cast<const char*>(looking_at), size);
+      }
+      looking_at += size;
+    }

[åsapersson, 2016/02/11 10:20:32]

is a chuck with zero items valid?

[danilchap, 2016/02/11 11:13:37]

Chunk need to have a cname. chunk with zero items can't.

+    if (!cname_found) {
+      LOG(LS_WARNING) << "CNAME not found for chunk #" << (i + 1);
+      return false;
+    }
+    block_length += ChunkSize(chunks[i]);

[åsapersson, 2016/02/11 10:20:33]

is block_length supposed to not include non-cname items?

[danilchap, 2016/02/11 11:13:37]

block_length is for packing Sdes back to byte array. Because non-cname items are
not stored and will not be included in rebuild packet, their size implicitly
ignored. If chunk would include another field that we would send, ChunkSize
would count its size.

[åsapersson, 2016/02/11 12:28:45]

Maybe add a comment.

[danilchap, 2016/02/11 12:48:47]

Done.

+    // Adjust to 32bit boundary.
+    looking_at += (payload_end - looking_at) % 4;
+  }
+
+  chunks_ = std::move(chunks);
+  block_length_ = block_length;
+  return true;
+}
+
+bool Sdes::WithCName(uint32_t ssrc, const std::string& cname) {
+  RTC_DCHECK_LE(cname.length(), 0xffu);
+  if (chunks_.size() >= kMaxNumberOfChunks) {
+    LOG(LS_WARNING) << "Max SDES chunks reached.";
+    return false;
+  }
+  Chunk chunk;
+  chunk.ssrc = ssrc;
+  chunk.cname = cname;
+  chunks_.push_back(chunk);
+  block_length_ += ChunkSize(chunk);
+  return true;
+}
+
 bool Sdes::Create(uint8_t* packet,
                   size_t* index,
                   size_t max_length,
                   RtcpPacket::PacketReadyCallback* callback) const {
-  assert(!chunks_.empty());
+  RTC_DCHECK(!chunks_.empty());

[åsapersson, 2016/02/11 10:20:32]

zero is valid right? maybe remove

[danilchap, 2016/02/11 11:13:37]

Done.

   while (*index + BlockLength() > max_length) {
     if (!OnBufferFull(packet, index, callback))
       return false;
   }
-  CreateHeader(chunks_.size(), PT_SDES, HeaderLength(), packet, index);
-  CreateSdes(chunks_, packet, index);
+  const size_t index_end = *index + BlockLength();
+  CreateHeader(chunks_.size(), kPacketType, HeaderLength(), packet, index);
+
+  const uint8_t kCnameId = 1;
+  for (const Sdes::Chunk& chunk : chunks_) {
+    ByteWriter<uint32_t>::WriteBigEndian(&packet[*index + 0], chunk.ssrc);
+    ByteWriter<uint8_t>::WriteBigEndian(&packet[*index + 4], kCnameId);
+    ByteWriter<uint8_t>::WriteBigEndian(&packet[*index + 5],
+                                        chunk.cname.size());
+    memcpy(&packet[*index + 6], chunk.cname.data(), chunk.cname.size());
+    *index += (6 + chunk.cname.size());
+
+    // In each chunk, the list of items must be terminated by one or more null
+    // octets. The next chunk must start on a 32-bit boundary.
+    // CNAME (1 byte) | length (1 byte) | name | padding.
+    size_t padding_size = 4 - ((6 + chunk.cname.size()) % 4);
+    const int kPadding = 0;
+    memset(packet + *index, kPadding, padding_size);
+    *index += padding_size;
+  }
+
+  RTC_CHECK_EQ(*index, index_end);
   return true;
 }
-
-bool Sdes::WithCName(uint32_t ssrc, const std::string& cname) {
-  assert(cname.length() <= 0xff);
-  if (chunks_.size() >= kMaxNumberOfChunks) {
-    LOG(LS_WARNING) << "Max SDES chunks reached.";
-    return false;
-  }
-  // In each chunk, the list of items must be terminated by one or more null
-  // octets. The next chunk must start on a 32-bit boundary.
-  // CNAME (1 byte) | length (1 byte) | name | padding.
-  int null_octets = 4 - ((2 + cname.length()) % 4);
-  Chunk chunk;
-  chunk.ssrc = ssrc;
-  chunk.name = cname;
-  chunk.null_octets = null_octets;
-  chunks_.push_back(chunk);
-  return true;
-}
-
-size_t Sdes::BlockLength() const {
-  // Header (4 bytes).
-  // Chunk:
-  // SSRC/CSRC (4 bytes) | CNAME (1 byte) | length (1 byte) | name | padding.
-  size_t length = kHeaderLength;
-  for (const Chunk& chunk : chunks_)
-    length += 6 + chunk.name.length() + chunk.null_octets;
-  assert(length % 4 == 0);
-  return length;
-}
 }  // namespace rtcp
 }  // namespace webrtc