Cast: Restructure RTCP interface to better fit our usage.

media/cast/rtcp/rtcp_sender.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/cast/rtcp/rtcp_sender.h"
 
 #include <algorithm>
 #include <vector>
 
 #include "base/debug/trace_event.h"
@@ skipping 12 matching lines @@
                        uint32 sending_ssrc,
                        const std::string& c_name)
      : ssrc_(sending_ssrc),
        c_name_(c_name),
        transport_(outgoing_transport) {
   DCHECK_LT(c_name_.length(), kRtcpCnameSize) << "Invalid config";
 }
 
 RtcpSender::~RtcpSender() {}
 
-void RtcpSender::SendRtcp(uint32 packet_type_flags,
-                          const RtcpSenderInfo* sender_info,
-                          const RtcpReportBlock* report_block,
-                          uint32 pli_remote_ssrc,
-                          const RtcpDlrrReportBlock* dlrr,
-                          const RtcpReceiverReferenceTimeReport* rrtr,
-                          const RtcpCastMessage* cast_message) {
+void RtcpSender::SendRtcpFromRtpSender(uint32 packet_type_flags,
+                                       const RtcpSenderInfo* sender_info,
+                                       const RtcpDlrrReportBlock* dlrr,
+                                       const RtcpSenderLogMessage* sender_log) {
+  if (packet_type_flags & kRtcpRr ||

[Alpha Left Google, 2013/11/13 23:58:40]

This condition in incorrect. It should be the opposite.

[pwestin, 2013/11/15 00:20:32]

Please clarify; this does what I want it to do. If any of the bits representing
kRtcpRr, kRtcpPli .. etc is set some one miss use the function.

+      packet_type_flags & kRtcpPli ||
+      packet_type_flags & kRtcpRrtr ||
+      packet_type_flags & kRtcpCast ||
+      packet_type_flags & kRtcpReceiverLog ||
+      packet_type_flags & kRtcpRpsi ||
+      packet_type_flags & kRtcpRemb ||
+      packet_type_flags & kRtcpNack) {
+    NOTREACHED() << "Invalid argument";
+  }
+
   std::vector<uint8> packet;
   packet.reserve(kIpPacketSize);
   if (packet_type_flags & kRtcpSr) {
     DCHECK(sender_info) << "Invalid argument";
-    BuildSR(*sender_info, report_block, &packet);
+    BuildSR(*sender_info, NULL, &packet);
     BuildSdec(&packet);
-  } else if (packet_type_flags & kRtcpRr) {
+  }
+  if (packet_type_flags & kRtcpBye) {
+    BuildBye(&packet);
+  }
+  if (packet_type_flags & kRtcpDlrr) {
+    DCHECK(dlrr) << "Invalid argument";
+    BuildDlrrRb(dlrr, &packet);
+  }
+  if (packet_type_flags & kRtcpSenderLog) {
+    DCHECK(sender_log) << "Invalid argument";
+    BuildSenderLog(sender_log, &packet);
+  }
+  if (packet.empty()) return;  // Sanity don't send empty packets.

[Alpha Left Google, 2013/11/13 23:58:40]

nit: drop return to next line.

[pwestin, 2013/11/15 00:20:32]

Done.

+
+  transport_->SendRtcpPacket(packet);
+}
+
+void RtcpSender::SendRtcpFromRtpReceiver(
+    uint32 packet_type_flags,
+    const RtcpReportBlock* report_block,
+    const RtcpReceiverReferenceTimeReport* rrtr,
+    const RtcpCastMessage* cast_message,
+    const RtcpReceiverLogMessage* receiver_log) {
+  if (packet_type_flags & kRtcpSr ||

[Alpha Left Google, 2013/11/13 23:58:40]

This condition is not correct. It should be the opposite.

[pwestin, 2013/11/15 00:20:32]

Please clarify; this does what I want it to do. If any of the bits representing
kRtcpSr, kRtcpDlrr .. etc is set some one miss use the function.

+      packet_type_flags & kRtcpDlrr ||
+      packet_type_flags & kRtcpSenderLog) {
+    NOTREACHED() << "Invalid argument";
+  }
+  if (packet_type_flags & kRtcpPli ||
+      packet_type_flags & kRtcpRpsi ||
+      packet_type_flags & kRtcpRemb ||
+      packet_type_flags & kRtcpNack) {
+    // Implement these for webrtc interop.
+    NOTIMPLEMENTED();
+  }
+  std::vector<uint8> packet;
+  packet.reserve(kIpPacketSize);
+
+  if (packet_type_flags & kRtcpRr) {
     BuildRR(report_block, &packet);
     if (!c_name_.empty()) {
       BuildSdec(&packet);
     }
   }
-  if (packet_type_flags & kRtcpPli) {
-    BuildPli(pli_remote_ssrc, &packet);
-  }
   if (packet_type_flags & kRtcpBye) {
     BuildBye(&packet);
   }
-  if (packet_type_flags & kRtcpRpsi) {
-    // Implement this for webrtc interop.
-    NOTIMPLEMENTED();
-  }
-  if (packet_type_flags & kRtcpRemb) {
-    // Implement this for webrtc interop.
-    NOTIMPLEMENTED();
-  }
-  if (packet_type_flags & kRtcpNack) {
-    // Implement this for webrtc interop.
-    NOTIMPLEMENTED();
-  }
-  if (packet_type_flags & kRtcpDlrr) {
-    DCHECK(dlrr) << "Invalid argument";
-    BuildDlrrRb(dlrr, &packet);
-  }
   if (packet_type_flags & kRtcpRrtr) {
     DCHECK(rrtr) << "Invalid argument";
     BuildRrtr(rrtr, &packet);
   }
   if (packet_type_flags & kRtcpCast) {
     DCHECK(cast_message) << "Invalid argument";
     BuildCast(cast_message, &packet);
   }
-
+  if (packet_type_flags & kRtcpReceiverLog) {
+    DCHECK(receiver_log) << "Invalid argument";
+    BuildReceiverLog(receiver_log, &packet);
+  }
   if (packet.empty()) return;  // Sanity don't send empty packets.
 
   transport_->SendRtcpPacket(packet);
 }
 
 void RtcpSender::BuildSR(const RtcpSenderInfo& sender_info,
                          const RtcpReportBlock* report_block,
                          std::vector<uint8>* packet) const {
   // Sender report.
   size_t start_size = packet->size();
@@ skipping 442 matching lines @@
   }
   DCHECK_LE(number_of_loss_fields, kRtcpMaxCastLossFields);
   (*packet)[cast_size_pos] = static_cast<uint8>(4 + number_of_loss_fields);
   (*packet)[cast_loss_field_pos] = static_cast<uint8>(number_of_loss_fields);
 
   // Frames with missing packets.
   TRACE_COUNTER_ID1("cast_rtcp", "RtcpSender::CastNACK", ssrc_,
                     cast->missing_frames_and_packets_.size());
 }
 
+void RtcpSender::BuildSenderLog(const RtcpSenderLogMessage* sender_log_message,
+                                std::vector<uint8>* packet) const {
+  // TODO(pwestin): Implement.
+  NOTIMPLEMENTED();
+}
+
+void RtcpSender::BuildReceiverLog(
+    const RtcpReceiverLogMessage* receiver_log_message,
+    std::vector<uint8>* packet) const {
+  // TODO(pwestin): Implement.
+  NOTIMPLEMENTED();
+}
+
 }  // namespace cast
 }  // namespace media