Cast: Refactor RTCP handling

media/cast/net/rtcp/rtcp_receiver.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/net/rtcp/rtcp_receiver.h"
 
+#include "base/big_endian.h"
 #include "base/logging.h"
 #include "media/cast/net/cast_transport_defines.h"
 #include "media/cast/net/rtcp/rtcp_utility.h"
 
 namespace {
 
 // A receiver frame event is identified by frame RTP timestamp, event timestamp
 // and event type.
 // A receiver packet event is identified by all of the above plus packet id.
 // The key format is as follows:
@@ skipping 14 matching lines @@
   value1 |= frame_rtp_timestamp;
   return std::make_pair(
       value1, static_cast<uint64>(event_timestamp.ToInternalValue()));
 }
 
 }  // namespace
 
 namespace media {
 namespace cast {
 
-RtcpReceiver::RtcpReceiver(scoped_refptr<CastEnvironment> cast_environment,
-                           RtcpSenderFeedback* sender_feedback,
-                           RtcpReceiverFeedback* receiver_feedback,
-                           RtcpRttFeedback* rtt_feedback,
+RtcpReceiver::RtcpReceiver(RtcpMessageHandler* handler,
                            uint32 local_ssrc)
     : ssrc_(local_ssrc),
       remote_ssrc_(0),
-      sender_feedback_(sender_feedback),
-      receiver_feedback_(receiver_feedback),
-      rtt_feedback_(rtt_feedback),
-      cast_environment_(cast_environment),
+      handler_(handler),
       receiver_event_history_size_(0) {}

[miu, 2014/07/16 00:09:32]

To simplify, consider adding DCHECK(handler_) here, and then remove all the NULL
checks in the rest of this module.

[Alpha Left Google, 2014/07/17 01:01:45]

Done.

 
 RtcpReceiver::~RtcpReceiver() {}
 
+// static
+bool RtcpReceiver::IsRtcpPacket(const uint8* packet, size_t length) {

[miu, 2014/07/16 00:09:32]

You could get rid of this method, and roll it into GetSsrcOfSender.  See
comments below.

[Alpha Left Google, 2014/07/17 01:01:45]

0 is a valid SSRC so I would like to keep this method.

+  DCHECK_GE(length, kMinLengthOfRtcp) << "Invalid RTCP packet";
+  if (length < kMinLengthOfRtcp) return false;
+
+  uint8 packet_type = packet[1];
+  if (packet_type >= kPacketTypeLow &&
+      packet_type <= kPacketTypeHigh) {
+    return true;
+  }
+  return false;
+}
+
+// static
+uint32 RtcpReceiver::GetSsrcOfSender(const uint8* rtcp_buffer, size_t length) {
+  DCHECK_GE(length, kMinLengthOfRtcp) << "Invalid RTCP packet";

[miu, 2014/07/16 00:09:32]

Don't need this DCHECK if checking big_endian_reader.XXX() return values (see
below).

+  uint32 ssrc_of_sender;
+  base::BigEndianReader big_endian_reader(
+      reinterpret_cast<const char*>(rtcp_buffer), length);
+  big_endian_reader.Skip(4);  // Skip header

[miu, 2014/07/16 00:09:32]

Note: These two method calls on big_endian_reader return false on failure (i.e.,
they check bounds).  Suggest replacing with this code:

  uint8 packet_type;
  if (big_endian_reader.Skip(2) &&
      big_endian_reader.ReadU8(&packet_type) &&
      packet_type >= kPacketTypeLow &&
      packet_type <= kPacketTypeHigh &&
      big_endian_reader.Skip(1) &&
      big_endian_reader.ReadU32(&ssrc_of_sender)) {
    return ssrc_of_sender;
  } else {
    return 0;
  }

[Alpha Left Google, 2014/07/17 01:01:45]

IsRtcpPacket is actually useful because 0 is also a valid SSRC. The bounds check
is guaranteed by kMinLengthOfRtcp already, I've made it an if statement.

+  big_endian_reader.ReadU32(&ssrc_of_sender);
+  return ssrc_of_sender;
+}
+
 void RtcpReceiver::SetRemoteSSRC(uint32 ssrc) { remote_ssrc_ = ssrc; }
 
 void RtcpReceiver::SetCastReceiverEventHistorySize(size_t size) {
   receiver_event_history_size_ = size;
 }
 
 void RtcpReceiver::IncomingRtcpPacket(RtcpParser* rtcp_parser) {
   RtcpFieldTypes field_type = rtcp_parser->Begin();
   while (field_type != kRtcpNotValidCode) {
     // Each "case" is responsible for iterate the parser to the next top
@@ skipping 10 matching lines @@
         break;
       case kRtcpByeCode:
         HandleBYE(rtcp_parser);
         break;
       case kRtcpXrCode:
         HandleXr(rtcp_parser);
         break;
       case kRtcpGenericRtpFeedbackNackCode:
         HandleNACK(rtcp_parser);
         break;
-      case kRtcpGenericRtpFeedbackSrReqCode:
-        HandleSendReportRequest(rtcp_parser);
-        break;
       case kRtcpPayloadSpecificPliCode:
         HandlePLI(rtcp_parser);
         break;
       case kRtcpPayloadSpecificRpsiCode:
         HandleRpsi(rtcp_parser);
         break;
       case kRtcpPayloadSpecificFirCode:
         HandleFIR(rtcp_parser);
         break;
       case kRtcpPayloadSpecificAppCode:
         HandlePayloadSpecificApp(rtcp_parser);
         break;
       case kRtcpApplicationSpecificCastReceiverLogCode:
         HandleApplicationSpecificCastReceiverLog(rtcp_parser);
         break;
       case kRtcpPayloadSpecificRembCode:
       case kRtcpPayloadSpecificRembItemCode:
       case kRtcpPayloadSpecificCastCode:
       case kRtcpPayloadSpecificCastNackItemCode:
       case kRtcpApplicationSpecificCastReceiverLogFrameCode:
       case kRtcpApplicationSpecificCastReceiverLogEventCode:
       case kRtcpNotValidCode:
       case kRtcpReportBlockItemCode:
       case kRtcpSdesChunkCode:
+      case kRtcpGenericRtpFeedbackSrReqCode:
       case kRtcpGenericRtpFeedbackNackItemCode:
       case kRtcpPayloadSpecificFirItemCode:
       case kRtcpXrRrtrCode:
       case kRtcpXrDlrrCode:
       case kRtcpXrUnknownItemCode:
         rtcp_parser->Iterate();
         NOTREACHED() << "Invalid state";
         break;
     }
     field_type = rtcp_parser->FieldType();
@@ skipping 15 matching lines @@
     RtcpSenderInfo remote_sender_info;
     remote_sender_info.ntp_seconds =
         rtcp_field.sender_report.ntp_most_significant;
     remote_sender_info.ntp_fraction =
         rtcp_field.sender_report.ntp_least_significant;
     remote_sender_info.rtp_timestamp = rtcp_field.sender_report.rtp_timestamp;
     remote_sender_info.send_packet_count =
         rtcp_field.sender_report.sender_packet_count;
     remote_sender_info.send_octet_count =
         rtcp_field.sender_report.sender_octet_count;
-    if (receiver_feedback_) {
-      receiver_feedback_->OnReceivedSenderReport(remote_sender_info);
+    if (handler_) {
+      handler_->OnReceivedSenderReport(remote_sender_info);
     }
   }
   rtcp_field_type = rtcp_parser->Iterate();
   while (rtcp_field_type == kRtcpReportBlockItemCode) {
     HandleReportBlock(&rtcp_field, remote_ssrc);
     rtcp_field_type = rtcp_parser->Iterate();
   }
 }
 
 void RtcpReceiver::HandleReceiverReport(RtcpParser* rtcp_parser) {
@@ skipping 37 matching lines @@
   report_block.remote_ssrc = remote_ssrc;
   report_block.media_ssrc = rb.ssrc;
   report_block.fraction_lost = rb.fraction_lost;
   report_block.cumulative_lost = rb.cumulative_number_of_packets_lost;
   report_block.extended_high_sequence_number =
       rb.extended_highest_sequence_number;
   report_block.jitter = rb.jitter;
   report_block.last_sr = rb.last_sender_report;
   report_block.delay_since_last_sr = rb.delay_last_sender_report;
 
-  if (rtt_feedback_) {
-    rtt_feedback_->OnReceivedDelaySinceLastReport(
-        rb.ssrc, rb.last_sender_report, rb.delay_last_sender_report);
+  if (handler_) {
+    handler_->OnReceivedDelaySinceLastReport(
+        rb.last_sender_report, rb.delay_last_sender_report);
   }
 }
 
 void RtcpReceiver::HandleSDES(RtcpParser* rtcp_parser) {
   RtcpFieldTypes field_type = rtcp_parser->Iterate();
   while (field_type == kRtcpSdesChunkCode) {
     HandleSDESChunk(rtcp_parser);
     field_type = rtcp_parser->Iterate();
   }
 }
@@ skipping 28 matching lines @@
   if (remote_ssrc_ != remote_ssrc) {
     // Not to us.
     return;
   }
   const RtcpField& rtcp_field = rtcp_parser->Field();
   RtcpReceiverReferenceTimeReport remote_time_report;
   remote_time_report.remote_ssrc = remote_ssrc;
   remote_time_report.ntp_seconds = rtcp_field.rrtr.ntp_most_significant;
   remote_time_report.ntp_fraction = rtcp_field.rrtr.ntp_least_significant;
 
-  if (receiver_feedback_) {
-    receiver_feedback_->OnReceiverReferenceTimeReport(remote_time_report);
+  if (handler_) {
+    handler_->OnReceiverReferenceTimeReport(remote_time_report);
   }
 }
 
 void RtcpReceiver::HandleDlrr(RtcpParser* rtcp_parser) {
   const RtcpField& rtcp_field = rtcp_parser->Field();
   if (remote_ssrc_ != rtcp_field.dlrr.receivers_ssrc) {
     // Not to us.
     return;
   }
-  if (rtt_feedback_) {
-    rtt_feedback_->OnReceivedDelaySinceLastReport(
-        rtcp_field.dlrr.receivers_ssrc,
+  if (handler_) {
+    handler_->OnReceivedDelaySinceLastReport(
         rtcp_field.dlrr.last_receiver_report,
         rtcp_field.dlrr.delay_last_receiver_report);
   }
 }
 
 void RtcpReceiver::HandleNACK(RtcpParser* rtcp_parser) {
   const RtcpField& rtcp_field = rtcp_parser->Field();
   if (ssrc_ != rtcp_field.nack.media_ssrc) {
     RtcpFieldTypes field_type;
     // Message not to us. Iterate until we have passed this message.
@@ skipping 37 matching lines @@
 
 void RtcpReceiver::HandlePLI(RtcpParser* rtcp_parser) {
   const RtcpField& rtcp_field = rtcp_parser->Field();
   if (ssrc_ == rtcp_field.pli.media_ssrc) {
     // Received a signal that we need to send a new key frame.
     VLOG(2) << "Cast RTCP received PLI on our SSRC " << ssrc_;
   }
   rtcp_parser->Iterate();
 }
 
-void RtcpReceiver::HandleSendReportRequest(RtcpParser* rtcp_parser) {
-  if (receiver_feedback_) {
-    receiver_feedback_->OnReceivedSendReportRequest();
-  }
-  rtcp_parser->Iterate();
-}
-
 void RtcpReceiver::HandleRpsi(RtcpParser* rtcp_parser) {
   const RtcpField& rtcp_field = rtcp_parser->Field();
   if (rtcp_parser->Iterate() != kRtcpPayloadSpecificRpsiCode) {
     return;
   }
   if (rtcp_field.rpsi.number_of_valid_bits % 8 != 0) {
     // Continue
     return;
   }
   uint64 rpsi_picture_id = 0;
@@ skipping 83 matching lines @@
           rtcp_field.cast_receiver_log.rtp_timestamp,
           rtcp_parser,
           &frame_log.event_log_messages_);
       field_type = rtcp_parser->Iterate();
     }
 
     if (!frame_log.event_log_messages_.empty())
       receiver_log.push_back(frame_log);
   }
 
-  if (receiver_feedback_ && !receiver_log.empty()) {
-    receiver_feedback_->OnReceivedReceiverLog(receiver_log);
+  if (handler_ && !receiver_log.empty()) {
+    handler_->OnReceivedReceiverLog(receiver_log);
   }
 }
 
 void RtcpReceiver::HandleApplicationSpecificCastReceiverEventLog(
     uint32 frame_rtp_timestamp,
     RtcpParser* rtcp_parser,
     RtcpReceiverEventLogMessages* event_log_messages) {
   const RtcpField& rtcp_field = rtcp_parser->Field();
 
   const uint8 event = rtcp_field.cast_receiver_log.event;
@@ skipping 44 matching lines @@
       rtcp_field.cast_item.last_frame_id);
   cast_message.target_delay_ms_ = rtcp_field.cast_item.target_delay_ms;
 
   RtcpFieldTypes packet_type = rtcp_parser->Iterate();
   while (packet_type == kRtcpPayloadSpecificCastNackItemCode) {
     const RtcpField& rtcp_field = rtcp_parser->Field();
     HandlePayloadSpecificCastNackItem(
         &rtcp_field, &cast_message.missing_frames_and_packets_);
     packet_type = rtcp_parser->Iterate();
   }
-  if (sender_feedback_) {
-    sender_feedback_->OnReceivedCastFeedback(cast_message);
+  if (handler_) {
+    handler_->OnReceivedCastFeedback(cast_message);
   }
 }
 
 void RtcpReceiver::HandlePayloadSpecificCastNackItem(
     const RtcpField* rtcp_field,
     MissingFramesAndPacketsMap* missing_frames_and_packets) {
 
   MissingFramesAndPacketsMap::iterator frame_it =
       missing_frames_and_packets->find(rtcp_field->cast_nack_item.frame_id);
 
@@ skipping 38 matching lines @@
 void RtcpReceiver::HandleFIRItem(const RtcpField* rtcp_field) {
   // Is it our sender that is requested to generate a new keyframe.
   if (ssrc_ != rtcp_field->fir_item.ssrc)
     return;
 
   VLOG(2) << "Cast RTCP received FIR on our SSRC " << ssrc_;
 }
 
 }  // namespace cast
 }  // namespace media