Cast: Move retransmission to the transport

media/cast/net/cast_transport_sender_impl.cc

Index: media/cast/net/cast_transport_sender_impl.cc
diff --git a/media/cast/net/cast_transport_sender_impl.cc b/media/cast/net/cast_transport_sender_impl.cc
index 248c69d6bb1e9c3b5d227ef45746aa122c533292..705a17652d274cdf67e7526d9fec3ed7583c5467 100644
--- a/media/cast/net/cast_transport_sender_impl.cc
+++ b/media/cast/net/cast_transport_sender_impl.cc
@@ -111,8 +111,11 @@ void CastTransportSenderImpl::InitializeAudio(
   }
 
   audio_rtcp_session_.reset(
-      new Rtcp(cast_message_cb,
-               rtt_cb,
+      new Rtcp(base::Bind(&CastTransportSenderImpl::OnReceivedCastMessage,
+                          weak_factory_.GetWeakPtr(), config.ssrc,
+                          cast_message_cb),
+               base::Bind(&CastTransportSenderImpl::OnReceivedRtt,
+                          weak_factory_.GetWeakPtr(), rtt_cb),
                base::Bind(&CastTransportSenderImpl::OnReceivedLogMessage,
                           weak_factory_.GetWeakPtr(), AUDIO_EVENT),
                clock_,
@@ -142,8 +145,11 @@ void CastTransportSenderImpl::InitializeVideo(
   }
 
   video_rtcp_session_.reset(
-      new Rtcp(cast_message_cb,
-               rtt_cb,
+      new Rtcp(base::Bind(&CastTransportSenderImpl::OnReceivedCastMessage,

[hubbe, 2014/08/11 19:11:49]

Do we really want one rtcp object for audio and one for video?
Or should we have one rtcp object for all channels?

[Alpha Left Google, 2014/08/11 21:03:17]

Yes we do. RTCP object maintain some states internally. We could eventually
merge them but that's no the scope of this change.

+                          weak_factory_.GetWeakPtr(), config.ssrc,
+                          cast_message_cb),
+               base::Bind(&CastTransportSenderImpl::OnReceivedRtt,
+                          weak_factory_.GetWeakPtr(), rtt_cb),
                base::Bind(&CastTransportSenderImpl::OnReceivedLogMessage,
                           weak_factory_.GetWeakPtr(), VIDEO_EVENT),
                clock_,
@@ -202,21 +208,49 @@ void CastTransportSenderImpl::SendSenderReport(
   }
 }
 
+void CastTransportSenderImpl::CancelSendingFrames(
+    uint32 ssrc,
+    const std::set<uint32>& frame_ids) {
+  // Cancel resends of acked frames.
+  MissingFramesAndPacketsMap missing_frames_and_packets;
+  PacketIdSet missing;
+  for (std::set<uint32>::const_iterator it = frame_ids.begin();
+       it != frame_ids.end(); ++it) {
+    missing_frames_and_packets[*it] = missing;
+  }
+  ResendPackets(ssrc, missing_frames_and_packets, true, base::TimeDelta());

[hubbe, 2014/08/11 19:11:50]

Why not call audio/video_sender->CancelSendingFrames() instead?

[Alpha Left Google, 2014/08/11 21:03:18]

Done.

+}
+
+void CastTransportSenderImpl::ResendFrameForKickstart(uint32 ssrc,
+                                                      uint32 frame_id) {
+  // Send the last packet of the encoded frame to kick start
+  // retransmission. This gives enough information to the receiver what
+  // packets and frames are missing.
+  MissingFramesAndPacketsMap missing_frames_and_packets;
+  PacketIdSet missing;
+  missing.insert(kRtcpCastLastPacket);
+  missing_frames_and_packets.insert(std::make_pair(frame_id, missing));
+
+  // Sending this extra packet is to kick-start the session. There is
+  // no need to optimize re-transmission for this case.
+  ResendPackets(ssrc, missing_frames_and_packets, false, most_recent_rtt_);

[hubbe, 2014/08/11 19:11:49]

Why not call audio/video_sender_->ResendFrameForKickStart() instead?

[Alpha Left Google, 2014/08/11 21:03:17]

Done.

+}
+
 void CastTransportSenderImpl::ResendPackets(
-    bool is_audio,
+    uint32 ssrc,
     const MissingFramesAndPacketsMap& missing_packets,
     bool cancel_rtx_if_not_in_list,
     base::TimeDelta dedupe_window) {
-  if (is_audio) {
-    DCHECK(audio_sender_) << "Audio sender uninitialized";
+  if (audio_sender_ && ssrc == audio_sender_->ssrc()) {
     audio_sender_->ResendPackets(missing_packets,
                                  cancel_rtx_if_not_in_list,
                                  dedupe_window);
-  } else {
-    DCHECK(video_sender_) << "Video sender uninitialized";
+  } else if (video_sender_ && ssrc == video_sender_->ssrc()) {
     video_sender_->ResendPackets(missing_packets,
                                  cancel_rtx_if_not_in_list,
                                  dedupe_window);
+  } else {
+    NOTREACHED() << "Invalid request for retransmission.";
   }
 }
 
@@ -293,5 +327,39 @@ void CastTransportSenderImpl::OnReceivedLogMessage(
   }
 }
 
+void CastTransportSenderImpl::OnReceivedRtt(const RtcpRttCallback& rtt_cb,
+                                            base::TimeDelta rtt,
+                                            base::TimeDelta avg_rtt,
+                                            base::TimeDelta min_rtt,
+                                            base::TimeDelta max_rtt) {
+  if (!rtt_cb.is_null())
+    rtt_cb.Run(rtt, avg_rtt, min_rtt, max_rtt);
+
+  most_recent_rtt_ = rtt;

[hubbe, 2014/08/11 19:11:49]

Seems like it would be cleaner to just have the rtcp object store this.

[Alpha Left Google, 2014/08/11 21:03:18]

Done.

+}
+
+void CastTransportSenderImpl::OnReceivedCastMessage(
+    uint32 ssrc,
+    const RtcpCastMessageCallback& cast_message_cb,
+    const RtcpCastMessage& cast_message) {
+  if (!cast_message_cb.is_null())
+    cast_message_cb.Run(cast_message);
+
+  if (cast_message.missing_frames_and_packets.empty())
+    return;
+
+  const bool is_audio = audio_sender_ && audio_sender_->ssrc() == ssrc;
+
+  // This call does two things.
+  // 1. Specifies that retransmissions for packets not listed in the set are
+  //    cancelled.
+  // 2. Specifies a deduplication window. For video this would be the most
+  //    recent RTT. For audio there is no deduplication.
+  ResendPackets(ssrc,
+                cast_message.missing_frames_and_packets,
+                true,
+                is_audio ? base::TimeDelta() : most_recent_rtt_);
+}
+
 }  // namespace cast
 }  // namespace media