Cast: Avoid retransmit if we sent the same packet recently (less than RTT)

media/cast/transport/pacing/paced_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/transport/pacing/paced_sender.h"
 
 #include "base/big_endian.h"
 #include "base/bind.h"
 #include "base/message_loop/message_loop.h"
 
@@ skipping 55 matching lines @@
   for (size_t i = 0; i < packets.size(); i++) {
     packet_list_[packets[i].first] =
         make_pair(PacketType_Normal, packets[i].second);
   }
   if (state_ == State_Unblocked) {
     SendStoredPackets();
   }
   return true;
 }
 
-bool PacedSender::ResendPackets(const SendPacketVector& packets) {
+bool PacedSender::ResendPackets(const SendPacketVector& packets,
+                                base::TimeDelta rtt) {
+  VLOG(0) << "RESENDPACKETS!!!!!!!";

[Alpha Left Google, 2014/06/18 01:43:32]

This shouldn't be there.

[hubbe, 2014/06/18 20:22:28]

Already gone.

   if (packets.empty()) {
     return true;
   }
+  base::TimeTicks now = clock_->NowTicks();
   for (size_t i = 0; i < packets.size(); i++) {
+    std::map<PacketKey, base::TimeTicks>::const_iterator j =
+        sent_time_.find(packets[i].first);
+
+    if (j != sent_time_.end() && now - j->second < rtt) {
+      // TODO(hubbe): Log this.

[Alpha Left Google, 2014/06/18 01:43:32]

We should log a PACKET_RTX_REJECTED event in the log.

See here:
https://code.google.com/p/chromium/codesearch#chromium/src/media/cast/logging...

It is important to evaluate the impact of this change from logs. Once we have a
good and proven algorithm then we can drop the event later.

[hubbe, 2014/06/18 20:22:28]

Done.

+      continue;
+    }
+
     packet_list_[packets[i].first] =
         make_pair(PacketType_Resend, packets[i].second);
   }
   if (state_ == State_Unblocked) {
     SendStoredPackets();
   }
   return true;
 }
 
 bool PacedSender::SendRtcpPacket(uint32 ssrc, PacketRef packet) {
@@ skipping 10 matching lines @@
     }
 
   }
   return true;
 }
 
 void PacedSender::CancelSendingPacket(const PacketKey& packet_key) {
   packet_list_.erase(packet_key);
 }
 
-PacketRef PacedSender::GetNextPacket(PacketType* packet_type) {
+PacketRef PacedSender::GetNextPacket(PacketType* packet_type,
+                                     PacketKey* packet_key) {
   std::map<PacketKey, std::pair<PacketType, PacketRef> >::iterator i;
   i = packet_list_.begin();
   DCHECK(i != packet_list_.end());
   *packet_type = i->second.first;
+  *packet_key = i->first;
   PacketRef ret = i->second.second;
   packet_list_.erase(i);
   return ret;
 }
 
 bool PacedSender::empty() const {
   return packet_list_.empty();
 }
 
 size_t PacedSender::size() const {
   return packet_list_.size();
 }
 
 // This function can be called from three places:
 // 1. User called one of the Send* functions and we were in an unblocked state.
 // 2. state_ == State_TransportBlocked and the transport is calling us to
 //    let us know that it's ok to send again.
 // 3. state_ == State_BurstFull and there are still packets to send. In this
 //    case we called PostDelayedTask on this function to start a new burst.
 void PacedSender::SendStoredPackets() {
   State previous_state = state_;
+
+  if (state_ == State_TransportBlocked) {
+    base::TimeTicks sent_time = clock_->NowTicks();
+    sent_time_[in_progress_packet_key_] = sent_time;
+    sent_time_buffer_[in_progress_packet_key_] = sent_time;
+  }
+
   state_ = State_Unblocked;
   if (empty()) {
     return;
   }
 
   base::TimeTicks now = clock_->NowTicks();
   // I don't actually trust that PostDelayTask(x - now) will mean that
   // now >= x when the call happens, so check if the previous state was
   // State_BurstFull too.
   if (now >= burst_end_ || previous_state == State_BurstFull) {
@@ skipping 31 matching lines @@
                                 weak_factory_.GetWeakPtr());
   while (!empty()) {
     if (current_burst_size_ >= max_burst_size_) {
       transport_task_runner_->PostDelayedTask(FROM_HERE,
                                               cb,
                                               burst_end_ - now);
       state_ = State_BurstFull;
       return;
     }
     PacketType packet_type;
-    PacketRef packet = GetNextPacket(&packet_type);
+    PacketRef packet = GetNextPacket(&packet_type,
+                                     &in_progress_packet_key_);
 
     switch (packet_type) {
       case PacketType_Resend:
         LogPacketEvent(packet->data, true);
         break;
       case PacketType_Normal:
         LogPacketEvent(packet->data, false);
         break;
       case PacketType_RTCP:
         break;
     }
     if (!transport_->SendPacket(packet, cb)) {
       state_ = State_TransportBlocked;
       return;
     }
     current_burst_size_++;
+    base::TimeTicks sent_time = clock_->NowTicks();
+    sent_time_[in_progress_packet_key_] = sent_time;
+    sent_time_buffer_[in_progress_packet_key_] = sent_time;
+  }
+  // Keep ~1 second of data (1000 packets)
+  if (sent_time_buffer_.size() > kMaxBurstSize * 1000 / kPacingIntervalMs) {

[Alpha Left Google, 2014/06/18 01:43:32]

nit: Define 1000 as a constant at the top of this file.

[hubbe, 2014/06/18 20:22:27]

It's just the number of milliseconds in a second.
Do we really need a constant for that?

+    sent_time_.swap(sent_time_buffer_);
+    sent_time_buffer_.clear();
   }
   state_ = State_Unblocked;

[Alpha Left Google, 2014/06/18 01:43:32]

Please put a DCHECK for the sizes of the sent_time_ and sent_time_buffer_.

[hubbe, 2014/06/18 20:22:27]

Done.

 }
 
 void PacedSender::LogPacketEvent(const Packet& packet, bool retransmit) {
   // Get SSRC from packet and compare with the audio_ssrc / video_ssrc to see
   // if the packet is audio or video.
   DCHECK_GE(packet.size(), 12u);
   base::BigEndianReader reader(reinterpret_cast<const char*>(&packet[8]), 4);
   uint32 ssrc;
   bool success = reader.ReadU32(&ssrc);
   DCHECK(success);
@@ skipping 10 matching lines @@
   CastLoggingEvent event = retransmit ?
       PACKET_RETRANSMITTED : PACKET_SENT_TO_NETWORK;
   EventMediaType media_type = is_audio ? AUDIO_EVENT : VIDEO_EVENT;
   logging_->InsertSinglePacketEvent(clock_->NowTicks(), event, media_type,
       packet);
 }
 
 }  // namespace transport
 }  // namespace cast
 }  // namespace media