Cast: Restructure RTCP interface to better fit our usage.

media/cast/rtcp/rtcp_sender.cc

Index: media/cast/rtcp/rtcp_sender.cc
diff --git a/media/cast/rtcp/rtcp_sender.cc b/media/cast/rtcp/rtcp_sender.cc
index cd1c50c579c97db3d27bdf8f5fc22eb70f55d90c..dd38bd569348f2ea28d0a37f83a85e815c40d873 100644
--- a/media/cast/rtcp/rtcp_sender.cc
+++ b/media/cast/rtcp/rtcp_sender.cc
@@ -30,46 +30,73 @@ RtcpSender::RtcpSender(PacedPacketSender* outgoing_transport,
 
 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) {
-    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 & kRtcpDlrr) {
+    DCHECK(dlrr) << "Invalid argument";
+    BuildDlrrRb(dlrr, &packet);
   }
-  if (packet_type_flags & kRtcpRemb) {
-    // Implement this for webrtc interop.
-    NOTIMPLEMENTED();
+  if (packet_type_flags & kRtcpSenderLog) {
+    DCHECK(sender_log) << "Invalid argument";
+    BuildSenderLog(sender_log, &packet);
   }
-  if (packet_type_flags & kRtcpNack) {
-    // Implement this for webrtc interop.
+  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();
   }
-  if (packet_type_flags & kRtcpDlrr) {
-    DCHECK(dlrr) << "Invalid argument";
-    BuildDlrrRb(dlrr, &packet);
+  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 & kRtcpBye) {
+    BuildBye(&packet);
   }
   if (packet_type_flags & kRtcpRrtr) {
     DCHECK(rrtr) << "Invalid argument";
@@ -79,7 +106,10 @@ void RtcpSender::SendRtcp(uint32 packet_type_flags,
     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);
@@ -542,5 +572,18 @@ void RtcpSender::BuildCast(const RtcpCastMessage* cast,
                     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