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

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

Index: webrtc/modules/rtp_rtcp/source/rtcp_packet/sdes.cc
diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet/sdes.cc b/webrtc/modules/rtp_rtcp/source/rtcp_packet/sdes.cc
new file mode 100644
index 0000000000000000000000000000000000000000..9eccb048e7555b88ebc7f42bffcbaadd20814801
--- /dev/null
+++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/sdes.cc
@@ -0,0 +1,168 @@
+/*
+ *  Copyright (c) 2015 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/rtp_utility.h"
+#include "webrtc/modules/rtp_rtcp/source/byte_io.h"
+
+using webrtc::RTCPUtility::RtcpCommonHeader;
+
+namespace webrtc {
+namespace rtcp {
+
+// 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        ...
+//   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+bool Sdes::Parse(const RtcpCommonHeader& header, const uint8_t* payload) {
+  RTC_DCHECK(header.packet_type == kPacketType);
+
+  const uint8_t chunk_count = 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.

[sprang_webrtc, 2015/11/12 15:27:15]

This will only be a problem if we try to reuse an instance, right?
Otherwise it might actually be interesting to get what data we actually managed
to parse.

[danilchap, 2015/11/12 16:59:09]

If packet is partially parsed, it means it is damaged. I am not sure data that
can't be trusted is interesting.
Keeping object clean doesn't add any noticeable overhead.

[sprang_webrtc, 2015/11/16 09:50:34]

Fair enough.

+
+  RTC_DCHECK_EQ(0u, header.payload_size_bytes % 4u);
+  const uint8_t* const payload_end = payload + header.payload_size_bytes;
+  const uint8_t* next_chunk = payload;

[sprang_webrtc, 2015/11/12 15:27:15]

Don't like the name next_chunk, since it doesn't necessarily point to the next
chunk.
current_data ? Dunno, something that makes it clearer it's pointing into the
buffer where we're currently parsing.

[danilchap, 2015/11/12 16:59:09]

Agree, can't keep it stay at a chunk since it also run through chunk items.
Renamed to 'looking_at'

+  chunks.resize(chunk_count);
+  for (size_t i = 0; i < chunk_count; ++i) {
+    // Each chunk consumes at least 8 bytes.
+    if (payload_end - next_chunk < 8) {
+      LOG(LS_WARNING) << "Not enough space left for chunk #" << (i + 1);
+      return false;
+    }
+    chunks[i].ssrc = ByteReader<uint32_t>::ReadBigEndian(next_chunk);
+    next_chunk += sizeof(uint32_t);
+    bool cname_found = false;
+
+    const uint8_t kTerminateId = 0;
+    const uint8_t kCnameId = 1;
+    uint8_t item_type;
+    while ((item_type = *(next_chunk++)) != kTerminateId) {
+      if (next_chunk >= 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 = *(next_chunk++);
+      if (next_chunk + size + 1 > payload_end) {
+        LOG(LS_WARNING) << "Unexpected end of packet while reading chunk #"
+                        << (i + 1) << ". Expected to find text of size "
+                        << size;
+        return false;
+      }
+      if (item_type == kCnameId) {
+        if (cname_found) {
+          LOG(LS_WARNING) << "Found extra CNAME for same ssrc in chunk #"
+                          << (i + 1);
+          return false;
+        }
+        cname_found = true;
+        chunks[i].name.assign(reinterpret_cast<const char*>(next_chunk), size);
+      }
+      next_chunk += size;
+    }
+    if (!cname_found) {
+      LOG(LS_WARNING) << "CNAME not found for chunk #" << (i + 1);
+      return false;
+    }
+    // Adjust to 32bit boundary.
+    next_chunk += (payload_end - next_chunk) % 4;
+  }
+
+  chunks_.swap(chunks);
+  return true;
+}
+
+bool Sdes::WithCName(uint32_t ssrc, const std::string& cname) {
+  RTC_DCHECK(cname.length() <= 0xff);
+  if (chunks_.size() >= kMaxNumberOfChunks) {
+    LOG(LS_WARNING) << "Max SDES chunks reached.";
+    return false;
+  }
+  Chunk chunk;
+  chunk.ssrc = ssrc;
+  chunk.name = cname;
+  chunks_.push_back(chunk);
+  return true;
+}
+
+bool Sdes::Create(uint8_t* packet,
+                  size_t* index,
+                  size_t max_length,
+                  RtcpPacket::PacketReadyCallback* callback) const {
+  RTC_DCHECK(!chunks_.empty());
+  while (*index + BlockLength() > max_length) {
+    if (!OnBufferFull(packet, index, callback))
+      return false;
+  }
+  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.name.size());
+    memcpy(&packet[*index + 6], chunk.name.data(), chunk.name.size());
+    *index += (6 + chunk.name.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.name.size()) % 4);
+    memset(packet + *index, 0, padding_size);
+    *index += padding_size;
+  }
+
+  RTC_DCHECK_EQ(index_end, *index);
+  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_) {
+    size_t padding_size = 4 - ((6 + chunk.name.size()) % 4);
+    length += 6 + chunk.name.size() + padding_size;
+  }
+  RTC_DCHECK_EQ(0u, length % 4);
+  return length;

[sprang_webrtc, 2015/11/12 15:27:15]

Should we perhaps store this as a member instead, and update it in Parse() and
WithCName() ?

[danilchap, 2015/11/12 16:59:09]

Agree, calculation of the length is not trivial but is used several times during
Create function (including implicitly in the HeaderLength) Can't see simpler way
to speed this up a bit.

+}
+
+}  // namespace rtcp
+}  // namespace webrtc