[Cast] Limit frames in flight by duration, and not by number of frames.

media/cast/sender/video_sender.cc

 // Copyright 2014 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/sender/video_sender.h"
 
 #include <algorithm>
 #include <cstring>
 
 #include "base/bind.h"
 #include "base/debug/trace_event.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
 #include "media/cast/cast_defines.h"
 #include "media/cast/net/cast_transport_config.h"
 #include "media/cast/sender/external_video_encoder.h"
 #include "media/cast/sender/video_encoder_impl.h"
 
 namespace media {
 namespace cast {
 
+namespace {
+
 // The following two constants are used to adjust the target
 // playout delay (when allowed). They were calculated using
 // a combination of cast_benchmark runs and manual testing.
-
+//
 // This is how many round trips we think we need on the network.
 const int kRoundTripsNeeded = 4;
-// This is an estimate of all the the constant time needed
-// independent of network quality.
+// This is an estimate of all the the constant time needed independent of
+// network quality (e.g., additional time that accounts for encode and decode
+// time).
 const int kConstantTimeMs = 75;
 
+}  // namespace
+
 // Note, we use a fixed bitrate value when external video encoder is used.
 // Some hardware encoder shows bad behavior if we set the bitrate too
 // frequently, e.g. quality drop, not abiding by target bitrate, etc.
 // See details: crbug.com/392086.
 VideoSender::VideoSender(
     scoped_refptr<CastEnvironment> cast_environment,
     const VideoSenderConfig& video_config,
     const CastInitializationCallback& initialization_cb,
     const CreateVideoEncodeAcceleratorCallback& create_vea_cb,
     const CreateVideoEncodeMemoryCallback& create_video_encode_mem_cb,
@@ skipping 46 matching lines @@
     cast_environment->PostTask(
         CastEnvironment::MAIN,
         FROM_HERE,
         base::Bind(initialization_cb, cast_initialization_status_));
   }
 
   media::cast::CastTransportRtpConfig transport_config;
   transport_config.ssrc = video_config.ssrc;
   transport_config.feedback_ssrc = video_config.incoming_feedback_ssrc;
   transport_config.rtp_payload_type = video_config.rtp_payload_type;
-  transport_config.stored_frames =
-      std::min(kMaxUnackedFrames,
-               1 + static_cast<int>(max_playout_delay_ *
-                                    max_frame_rate_ /
-                                    base::TimeDelta::FromSeconds(1)));
   transport_config.aes_key = video_config.aes_key;
   transport_config.aes_iv_mask = video_config.aes_iv_mask;
 
   transport_sender->InitializeVideo(
       transport_config,
       base::Bind(&VideoSender::OnReceivedCastFeedback,
                  weak_factory_.GetWeakPtr()),
       base::Bind(&VideoSender::OnMeasuredRoundTripTime,
                  weak_factory_.GetWeakPtr()));
 }
@@ skipping 21 matching lines @@
       rtp_timestamp,
       kFrameIdUnknown);
 
   // Used by chrome/browser/extension/api/cast_streaming/performance_test.cc
   TRACE_EVENT_INSTANT2(
       "cast_perf_test", "InsertRawVideoFrame",
       TRACE_EVENT_SCOPE_THREAD,
       "timestamp", capture_time.ToInternalValue(),
       "rtp_timestamp", rtp_timestamp);
 
-  if (ShouldDropNextFrame(capture_time)) {
-    VLOG(1) << "Dropping frame due to too many frames currently in-flight.";
+  // Drop frames that are out-of-order since the duration calculations assume
+  // frame timestamps are monotonically non-decreasing.
+  if (!last_enqueued_frame_reference_time_.is_null() &&
+      capture_time < last_enqueued_frame_reference_time_) {

[hubbe, 2014/09/18 17:56:05]

We don't want frames where capture time is identical either.
Change < to <=

[miu, 2014/09/18 21:38:43]

Done.

+    VLOG(1) << "Dropping video frame: Reference time is out-of-order.";
+    return;
+  }
+
+  // Two video frames are needed to compute the exact media duration added by
+  // the next frame.  If there are no frames in the encoder, compute a guess
+  // based on the configured |max_frame_rate_|.  Any error introduced by this
+  // guess will be eliminated when |duration_in_encoder_| is updated in
+  // OnEncodedVideoFrame().
+  const base::TimeDelta duration_added_by_next_frame = frames_in_encoder_ > 0 ?
+      capture_time - last_enqueued_frame_reference_time_ :
+      base::TimeDelta::FromMicroseconds(1000000.0 / max_frame_rate_ + 0.5);

[hubbe, 2014/09/18 17:56:05]

why + 0.5 ?

[miu, 2014/09/18 21:38:43]

Rounding to nearest int.  I made this more explicit by adding a
static_cast<int64>(...).

[hubbe, 2014/09/18 22:22:59]

Why not just use TimeDelta::FromSecondsD(1.0 / max_frame_rate_) ?

[miu, 2014/09/19 19:39:43]

Because that would be the right way to do it.  ;-)

Done.

+
+  if (ShouldDropNextFrame(duration_added_by_next_frame)) {
     base::TimeDelta new_target_delay = std::min(
         current_round_trip_time_ * kRoundTripsNeeded +
         base::TimeDelta::FromMilliseconds(kConstantTimeMs),
         max_playout_delay_);
     if (new_target_delay > target_playout_delay_) {
       VLOG(1) << "New target delay: " << new_target_delay.InMilliseconds();
       playout_delay_change_cb_.Run(new_target_delay);
     }
     return;
   }
 
   uint32 bitrate = congestion_control_->GetBitrate(
         capture_time + target_playout_delay_, target_playout_delay_);
   if (bitrate != last_bitrate_) {
     video_encoder_->SetBitRate(bitrate);
     last_bitrate_ = bitrate;
   }
 
   if (video_encoder_->EncodeVideoFrame(
           video_frame,
           capture_time,
           base::Bind(&VideoSender::OnEncodedVideoFrame,
                      weak_factory_.GetWeakPtr(),
                      bitrate))) {
     frames_in_encoder_++;
+    duration_in_encoder_ += duration_added_by_next_frame;
+    last_enqueued_frame_reference_time_ = capture_time;
   } else {
     VLOG(1) << "Encoder rejected a frame.  Skipping...";
   }
 }
 
 int VideoSender::GetNumberOfFramesInEncoder() const {
   return frames_in_encoder_;
 }
 
+base::TimeDelta VideoSender::GetInFlightMediaDuration() const {
+  if (GetUnacknowledgedFrameCount() > 0) {
+    const uint32 oldest_unacked_frame_id = latest_acked_frame_id_ + 1;
+    return duration_in_encoder_ +

[hubbe, 2014/09/18 17:56:05]

Isn't this equivalent to:

GetRecordedReferenceTime(last_sent_frame_id_) -
last_enqueued_frame_reference_time_

?

[miu, 2014/09/18 21:38:43]

Ah, good point.  But, I think you meant:

  last_enqueued_frame_reference_time_ -                  // *
  GetRecordedReferenceTime(oldest_unacked_frame_id)

*: This is the most recent event.  Also, this will be equal to
GetRecordedReferenceTime(last_sent_frame_id_) whenever frames_in_encoder_ is
zero.

+        (GetRecordedReferenceTime(last_sent_frame_id_) -
+             GetRecordedReferenceTime(oldest_unacked_frame_id));
+  } else {
+    return duration_in_encoder_;
+  }
+}
+
 void VideoSender::OnAck(uint32 frame_id) {
   video_encoder_->LatestFrameIdToReference(frame_id);
 }
 
 void VideoSender::OnEncoderInitialized(
     const CastInitializationCallback& initialization_cb,
     CastInitializationStatus status) {
   cast_initialization_status_ = status;
   initialization_cb.Run(status);
 }
 
 void VideoSender::OnEncodedVideoFrame(
     int encoder_bitrate,
     scoped_ptr<EncodedFrame> encoded_frame) {
   DCHECK(cast_environment_->CurrentlyOn(CastEnvironment::MAIN));
 
   frames_in_encoder_--;
   DCHECK_GE(frames_in_encoder_, 0);
 
+  duration_in_encoder_ =
+      last_enqueued_frame_reference_time_ - encoded_frame->reference_time;
+
   SendEncodedFrame(encoder_bitrate, encoded_frame.Pass());
 }
 
 }  // namespace cast
 }  // namespace media