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

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

+/*
+ *  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.
+ *
+ */
+
+#ifndef WEBRTC_MODULES_RTP_RTCP_SOURCE_RTCP_PACKET_SDES_H_
+#define WEBRTC_MODULES_RTP_RTCP_SOURCE_RTCP_PACKET_SDES_H_
+
+#include <string>
+#include <vector>
+
+#include "webrtc/modules/rtp_rtcp/source/rtcp_packet.h"
+#include "webrtc/modules/rtp_rtcp/source/rtcp_utility.h"
+#include "webrtc/typedefs.h"
+
+namespace webrtc {
+namespace rtcp {
+
+class Sdes : public RtcpPacket {
+ public:
+  static const uint8_t kPacketType = 202;
+
+  Sdes() : RtcpPacket() {}
+
+  virtual ~Sdes() {}
+
+  // Parse assumes header is already parsed and validated.
+  bool Parse(const RTCPUtility::RtcpCommonHeader& header,
+             const uint8_t* payload);  // Size of the payload is in the header.
+
+  bool WithCName(uint32_t ssrc, const std::string& cname);
+
+  struct Chunk {
+    uint32_t ssrc;
+    std::string name;
+  };
+
+  size_t chunks_count() const { return chunks_.size(); }
+  const Chunk& chunk(size_t index) const { return chunks_[index]; }

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

Could we just return a const std::vector<Chunk>& instead?

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

I do not want to disclose to user that vector is used as internal storage
(may be later, for perfomance reasons, it will be replaced with a plain array)

In fact it may be better to go further: hide struct Chunk and instead of
function 'chunk' have two functions:
ssrc(size_t index) and cname(size_t index)

If we want to have other fields than cname (name, email, etc.) passed and
extracted from this packet, then function per field would give more flexability
how to organize it internally.

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

I get the sentiment, but I think this is premature optimization.
I'd rather have more easily read code, say being able to use a for-each loop
or pass the reference around. Should any of these places prove to actually be
noticeable performance wise, let's refactor at that point instead.

[danilchap, 2015/11/17 10:50:48]

Done.

+
+ protected:
+  bool Create(uint8_t* packet,
+              size_t* index,
+              size_t max_length,
+              RtcpPacket::PacketReadyCallback* callback) const override;
+
+ private:
+  static const int kMaxNumberOfChunks = 0x1f;
+
+  size_t BlockLength() const override;
+
+  std::vector<Chunk> chunks_;
+
+  RTC_DISALLOW_COPY_AND_ASSIGN(Sdes);
+};
+
+}  // namespace rtcp
+}  // namespace webrtc
+#endif  // WEBRTC_MODULES_RTP_RTCP_SOURCE_RTCP_PACKET_SDES_H_