Send Sdes using RtcpPacket

BUG=2450
R=asapersson@webrtc.org

Review URL: https://codereview.webrtc.org/1196863003.

Cr-Commit-Position: refs/heads/master@{#9504}

webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h

@@ -365,8 +365,7 @@ class RtpRtcp : public Module {
     *
     *   return -1 on failure else 0
     */
-    virtual int32_t AddMixedCNAME(uint32_t SSRC,
+    virtual int32_t AddMixedCNAME(uint32_t SSRC, const char* c_name) = 0;
-                                  const char cName[RTCP_CNAME_SIZE]) = 0;
 
     /*
     *   RemoveMixedCNAME

webrtc/modules/rtp_rtcp/source/rtcp_sender.cc

@@ -170,13 +170,12 @@ RTCPSender::RTCPSender(
 
       xr_send_receiver_reference_time_enabled_(false),
       packet_type_counter_observer_(packet_type_counter_observer) {
-  memset(cname_, 0, sizeof(cname_));
   memset(last_send_report_, 0, sizeof(last_send_report_));
   memset(last_rtcp_time_, 0, sizeof(last_rtcp_time_));
 
   builders_[kRtcpSr] = &RTCPSender::BuildSR;
   builders_[kRtcpRr] = &RTCPSender::BuildRR;
-  builders_[kRtcpSdes] = &RTCPSender::BuildSDEC;
+  builders_[kRtcpSdes] = &RTCPSender::BuildSDES;
   builders_[kRtcpTransmissionTimeOffset] =
       &RTCPSender::BuildExtendedJitterReport;
   builders_[kRtcpPli] = &RTCPSender::BuildPLI;
@@ -196,8 +195,6 @@ RTCPSender::RTCPSender(
 }
 
 RTCPSender::~RTCPSender() {
-  for (auto it : csrc_cnames_)
-    delete it.second;
 }
 
 int32_t RTCPSender::RegisterSendTransport(Transport* outgoingTransport) {
@@ -333,34 +330,28 @@ int32_t RTCPSender::SetCNAME(const char* c_name) {
 
   DCHECK_LT(strlen(c_name), static_cast<size_t>(RTCP_CNAME_SIZE));
   CriticalSectionScoped lock(critical_section_rtcp_sender_.get());
-  cname_[RTCP_CNAME_SIZE - 1] = 0;
+  cname_ = c_name;
-  strncpy(cname_, c_name, RTCP_CNAME_SIZE - 1);
   return 0;
 }
 
-int32_t RTCPSender::AddMixedCNAME(uint32_t SSRC,
+int32_t RTCPSender::AddMixedCNAME(uint32_t SSRC, const char* c_name) {
-                                  const char cName[RTCP_CNAME_SIZE]) {
+  assert(c_name);
-  assert(cName);
+  DCHECK_LT(strlen(c_name), static_cast<size_t>(RTCP_CNAME_SIZE));
   CriticalSectionScoped lock(critical_section_rtcp_sender_.get());
-  if (csrc_cnames_.size() >= kRtpCsrcSize) {
+  if (csrc_cnames_.size() >= kRtpCsrcSize)
     return -1;
-  }
+
-  RTCPCnameInformation* ptr = new RTCPCnameInformation();
+  csrc_cnames_[SSRC] = c_name;
-  ptr->name[RTCP_CNAME_SIZE - 1] = 0;
-  strncpy(ptr->name, cName, RTCP_CNAME_SIZE - 1);
-  csrc_cnames_[SSRC] = ptr;
   return 0;
 }
 
 int32_t RTCPSender::RemoveMixedCNAME(uint32_t SSRC) {
   CriticalSectionScoped lock(critical_section_rtcp_sender_.get());
-  std::map<uint32_t, RTCPCnameInformation*>::iterator it =
+  auto it = csrc_cnames_.find(SSRC);
-      csrc_cnames_.find(SSRC);
 
   if (it == csrc_cnames_.end())
     return -1;
 
-  delete it->second;
   csrc_cnames_.erase(it);
   return 0;
 }
@@ -535,80 +526,22 @@ RTCPSender::BuildResult RTCPSender::BuildSR(RtcpContext* ctx) {
   return BuildResult::kSuccess;
 }
 
-RTCPSender::BuildResult RTCPSender::BuildSDEC(RtcpContext* ctx) {
+RTCPSender::BuildResult RTCPSender::BuildSDES(RtcpContext* ctx) {
-  size_t lengthCname = strlen(cname_);
+  size_t length_cname = cname_.length();
-  assert(lengthCname < RTCP_CNAME_SIZE);
+  CHECK_LT(length_cname, static_cast<size_t>(RTCP_CNAME_SIZE));
 
-  // sanity
+  rtcp::Sdes sdes;
-  if (ctx->position + 12 + lengthCname >= IP_PACKET_SIZE) {
+  sdes.WithCName(ssrc_, cname_);
-    LOG(LS_WARNING) << "Failed to build SDEC.";
+
+  for (const auto it : csrc_cnames_)
+    sdes.WithCName(it.first, it.second);
+
+  PacketBuiltCallback callback(ctx);
+  if (!sdes.BuildExternalBuffer(&ctx->buffer[ctx->position],
+                                ctx->buffer_size - ctx->position, &callback)) {
     return BuildResult::kTruncated;
   }
-  // SDEC Source Description
 
-  // We always need to add SDES CNAME
-  size_t size = 0x80 + 1 + csrc_cnames_.size();
-  DCHECK_LE(size, std::numeric_limits<uint8_t>::max());
-  *ctx->AllocateData(1) = static_cast<uint8_t>(size);
-  *ctx->AllocateData(1) = 202;
-
-  // handle SDES length later on
-  uint32_t SDESLengthPos = ctx->position;
-  ctx->AllocateData(2);
-
-  // Add our own SSRC
-  ByteWriter<uint32_t>::WriteBigEndian(ctx->AllocateData(4), ssrc_);
-
-  // CNAME = 1
-  *ctx->AllocateData(1) = 1;
-  DCHECK_LE(lengthCname, std::numeric_limits<uint8_t>::max());
-  *ctx->AllocateData(1) = static_cast<uint8_t>(lengthCname);
-
-  uint16_t SDESLength = 10;
-
-  memcpy(ctx->AllocateData(lengthCname), cname_, lengthCname);
-  SDESLength += static_cast<uint16_t>(lengthCname);
-
-  uint16_t padding = 0;
-  // We must have a zero field even if we have an even multiple of 4 bytes
-  do {
-    ++padding;
-    *ctx->AllocateData(1) = 0;
-  } while ((ctx->position % 4) != 0);
-  SDESLength += padding;
-
-  for (auto it = csrc_cnames_.begin(); it != csrc_cnames_.end(); ++it) {
-    RTCPCnameInformation* cname = it->second;
-    uint32_t SSRC = it->first;
-
-    // Add SSRC
-    ByteWriter<uint32_t>::WriteBigEndian(ctx->AllocateData(4), SSRC);
-
-    // CNAME = 1
-    *ctx->AllocateData(1) = 1;
-
-    size_t length = strlen(cname->name);
-    assert(length < RTCP_CNAME_SIZE);
-
-    *ctx->AllocateData(1) = static_cast<uint8_t>(length);
-    SDESLength += 6;
-
-    memcpy(ctx->AllocateData(length), cname->name, length);
-
-    SDESLength += length;
-    uint16_t padding = 0;
-
-    // We must have a zero field even if we have an even multiple of 4 bytes
-    do {
-      ++padding;
-      *ctx->AllocateData(1) = 0;
-    } while ((ctx->position % 4) != 0);
-    SDESLength += padding;
-  }
-  // in 32-bit words minus one and we don't count the header
-  uint16_t buffer_length = (SDESLength / 4) - 1;
-  ByteWriter<uint16_t>::WriteBigEndian(&ctx->buffer[SDESLengthPos],
-                                       buffer_length);
   return BuildResult::kSuccess;
 }
 
@@ -1409,7 +1342,7 @@ int RTCPSender::PrepareRTCP(const FeedbackState& feedback_state,
       SetFlag(sending_ ? kRtcpSr : kRtcpRr, true);
   }
 
-  if (IsFlagPresent(kRtcpSr) || (IsFlagPresent(kRtcpRr) && cname_[0] != 0))
+  if (IsFlagPresent(kRtcpSr) || (IsFlagPresent(kRtcpRr) && !cname_.empty()))
     SetFlag(kRtcpSdes, true);
 
   // We need to send our NTP even if we haven't received any reports.
@@ -1469,7 +1402,7 @@ int RTCPSender::PrepareRTCP(const FeedbackState& feedback_state,
     }
 
     uint32_t start_position = context.position;
-    BuildResult result = (*this.*(builder->second))(&context);
+    BuildResult result = (this->*(builder->second))(&context);
     switch (result) {
       case BuildResult::kError:
         return -1;

webrtc/modules/rtp_rtcp/source/rtcp_sender.h

@@ -98,7 +98,7 @@ public:
 
  int32_t SetCNAME(const char* cName);
 
- int32_t AddMixedCNAME(uint32_t SSRC, const char cName[RTCP_CNAME_SIZE]);
+ int32_t AddMixedCNAME(uint32_t SSRC, const char* c_name);
 
  int32_t RemoveMixedCNAME(uint32_t SSRC);
 
@@ -200,7 +200,7 @@ private:
      EXCLUSIVE_LOCKS_REQUIRED(critical_section_rtcp_sender_);
  BuildResult BuildExtendedJitterReport(RtcpContext* context)
      EXCLUSIVE_LOCKS_REQUIRED(critical_section_rtcp_sender_);
- BuildResult BuildSDEC(RtcpContext* context)
+ BuildResult BuildSDES(RtcpContext* context)
      EXCLUSIVE_LOCKS_REQUIRED(critical_section_rtcp_sender_);
  BuildResult BuildPLI(RtcpContext* context)
      EXCLUSIVE_LOCKS_REQUIRED(critical_section_rtcp_sender_);
@@ -252,14 +252,13 @@ private:
  uint32_t ssrc_ GUARDED_BY(critical_section_rtcp_sender_);
  // SSRC that we receive on our RTP channel
  uint32_t remote_ssrc_ GUARDED_BY(critical_section_rtcp_sender_);
- char cname_[RTCP_CNAME_SIZE] GUARDED_BY(critical_section_rtcp_sender_);
+ std::string cname_ GUARDED_BY(critical_section_rtcp_sender_);
 
  ReceiveStatistics* receive_statistics_
      GUARDED_BY(critical_section_rtcp_sender_);
  std::map<uint32_t, rtcp::ReportBlock> report_blocks_
      GUARDED_BY(critical_section_rtcp_sender_);
- // TODO(sprang): Can we avoid pointers here?
+ std::map<uint32_t, std::string> csrc_cnames_
- std::map<uint32_t, RTCPUtility::RTCPCnameInformation*> csrc_cnames_
      GUARDED_BY(critical_section_rtcp_sender_);
 
  // Sent

webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc

@@ -497,8 +497,7 @@ int32_t ModuleRtpRtcpImpl::SetCNAME(const char* c_name) {
   return rtcp_sender_.SetCNAME(c_name);
 }
 
-int32_t ModuleRtpRtcpImpl::AddMixedCNAME(uint32_t ssrc,
+int32_t ModuleRtpRtcpImpl::AddMixedCNAME(uint32_t ssrc, const char* c_name) {
-                                         const char c_name[RTCP_CNAME_SIZE]) {
   return rtcp_sender_.AddMixedCNAME(ssrc, c_name);
 }
 

webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h

@@ -144,8 +144,7 @@ class ModuleRtpRtcpImpl : public RtpRtcp {
                     uint32_t* rtcp_arrival_time_frac,
                     uint32_t* rtcp_timestamp) const override;
 
-  int32_t AddMixedCNAME(uint32_t ssrc,
+  int32_t AddMixedCNAME(uint32_t ssrc, const char* c_name) override;
-                        const char c_name[RTCP_CNAME_SIZE]) override;
 
   int32_t RemoveMixedCNAME(uint32_t ssrc) override;