Cast: Fix rtcp event dedup logic in rtcp_receiver.

media/cast/rtcp/rtcp_receiver.cc

Index: media/cast/rtcp/rtcp_receiver.cc
diff --git a/media/cast/rtcp/rtcp_receiver.cc b/media/cast/rtcp/rtcp_receiver.cc
index 8c0e593d6e22a2c7332ad8aa2d8b309466901b15..f8d9df4b9e66d0d628194c3da3989b2c06dc1391 100644
--- a/media/cast/rtcp/rtcp_receiver.cc
+++ b/media/cast/rtcp/rtcp_receiver.cc
@@ -10,35 +10,11 @@
 
 namespace {
 
-media::cast::CastLoggingEvent TranslateToLogEventFromWireFormat(uint8 event) {
-  switch (event) {
-    case 1:
-      return media::cast::kAudioAckSent;
-    case 2:
-      return media::cast::kAudioPlayoutDelay;
-    case 3:
-      return media::cast::kAudioFrameDecoded;
-    case 4:
-      return media::cast::kAudioPacketReceived;
-    case 5:
-      return media::cast::kVideoAckSent;
-    case 6:
-      return media::cast::kVideoFrameDecoded;
-    case 7:
-      return media::cast::kVideoRenderDelay;
-    case 8:
-      return media::cast::kVideoPacketReceived;
-    case 9:
-      return media::cast::kDuplicateAudioPacketReceived;
-    case 10:
-      return media::cast::kDuplicateVideoPacketReceived;
-    default:
-      // If the sender adds new log messages we will end up here until we add
-      // the new messages in the receiver.
-      VLOG(1) << "Unexpected log message received: " << static_cast<int>(event);
-      NOTREACHED();
-      return media::cast::kUnknown;
-  }
+bool IsRtcpPacketEvent(media::cast::CastLoggingEvent event_type) {
+  return event_type == media::cast::kAudioPacketReceived ||
+         event_type == media::cast::kVideoPacketReceived ||
+         event_type == media::cast::kDuplicateAudioPacketReceived ||
+         event_type == media::cast::kDuplicateVideoPacketReceived;
 }
 
 media::cast::transport::RtcpSenderFrameStatus
@@ -61,12 +37,16 @@ TranslateToFrameStatusFromWireFormat(uint8 status) {
   }
 }
 
-// A receiver event is identified by frame RTP timestamp, event timestamp and
-// event type.
+// 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.
 size_t HashReceiverEvent(uint32 frame_rtp_timestamp,

[Alpha Left Google, 2014/05/06 20:55:52]

I suggest we change the hash set to use a key of std::pair<uint64, uint64>. I
thought size_t was enough. This means the return type of this method should be:

typedef std::pair<uint64, uint64> ReceiverEventKey;

// <Describe how to combine multiple fields here..>
ReceiverEventKey ComputeReceiverEventKey(...);

[imcheng, 2014/05/06 21:19:47]

Are you suggesting that we get rid of the base::HashInts64 call below and just
let the hash_set take care of the hashing? Because reading this again, I do not
understand why is it necessary to call base::HashInts64.

On 2014/05/06 20:55:52, Alpha wrote:

[Alpha Left Google, 2014/05/06 21:21:45]

That's right. I used it because base::hash_map might not support std::pair<> on
some platform and size_t was easier. But for correctness (that this patch is
trying to fix) using std::pair<> is the right thing to do.

[imcheng, 2014/05/06 22:29:39]

Done.

                          const base::TimeTicks& event_timestamp,
-                         media::cast::CastLoggingEvent event_type) {
+                         uint8 event_type,
+                         uint16 packet_id_or_zero) {
   uint64 value1 = event_type;
+  value1 <<= 16;
+  value1 |= packet_id_or_zero;
   value1 <<= 32;
   value1 |= frame_rtp_timestamp;
   return base::HashInts64(
@@ -490,8 +470,10 @@ void RtcpReceiver::HandleApplicationSpecificCastReceiverEventLog(
     RtcpReceiverEventLogMessages* event_log_messages) {
   const RtcpField& rtcp_field = rtcp_parser->Field();
 
-  const CastLoggingEvent event_type =
-      TranslateToLogEventFromWireFormat(rtcp_field.cast_receiver_log.event);
+  const uint8 event = rtcp_field.cast_receiver_log.event;
+  const CastLoggingEvent event_type = TranslateToLogEventFromWireFormat(event);
+  uint16 packet_id = IsRtcpPacketEvent(event_type) ?
+      rtcp_field.cast_receiver_log.delay_delta_or_packet_id.packet_id : 0;
   const base::TimeTicks event_timestamp =
       base::TimeTicks() +
       base::TimeDelta::FromMilliseconds(
@@ -506,7 +488,7 @@ void RtcpReceiver::HandleApplicationSpecificCastReceiverEventLog(
   // Different events may have the same hash value. That's okay because full
   // accuracy is not important in this case.
   const size_t event_hash =
-      HashReceiverEvent(frame_rtp_timestamp, event_timestamp, event_type);
+      HashReceiverEvent(frame_rtp_timestamp, event_timestamp, event, packet_id);
   if (receiver_event_hash_set_.find(event_hash) !=
       receiver_event_hash_set_.end()) {
     return;