VideoAdapter: Drop frames based on actual fps instead of expected fps

webrtc/media/base/videoadapter.cc

 /*
  *  Copyright (c) 2010 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 100 matching lines @@
 
 namespace cricket {
 
 VideoAdapter::VideoAdapter()
     : frames_in_(0),
       frames_out_(0),
       frames_scaled_(0),
       adaption_changes_(0),
       previous_width_(0),
       previous_height_(0),
-      input_interval_(0),
-      interval_next_frame_(0),
+      previous_timestamp_ns_(0),

[nisse-webrtc, 2016/05/18 11:44:00]

Please make it an rtc::Optional, instead of using 0 as a magic value for unset.

[magjed_webrtc, 2016/05/18 16:35:22]

Done.

+      interval_ns_next_frame_(0),

[nisse-webrtc, 2016/05/18 11:44:00]

There should be a comment somewhere explaining the meaning of this variable. I
take it is time left until the next frame should be sent?

It might be clearer to replace both previous_timestamp_ns_ and
interval_ns_next_frame_ by a variable holding the absolute time of the next
frame.

nit: I think unit should be moved to the end of the name.

[magjed_webrtc, 2016/05/18 16:35:22]

Done.

       resolution_request_max_pixel_count_(std::numeric_limits<int>::max()),
       resolution_request_max_pixel_count_step_up_(0) {}
 
 VideoAdapter::~VideoAdapter() {}
 
-void VideoAdapter::SetExpectedInputFrameInterval(int64_t interval) {
-  // TODO(perkj): Consider measuring input frame rate instead.
-  // Frame rate typically varies depending on lighting.
+bool VideoAdapter::ShouldDrop(int64_t in_timestamp_ns) {

[nisse-webrtc, 2016/05/18 11:44:00]

I'd prefer if false meant that the frame should be dropped. Name could be
"KeepFrame".

[magjed_webrtc, 2016/05/18 16:35:23]

Done.

   rtc::CritScope cs(&critical_section_);
-  input_interval_ = interval;
+  if (!requested_format_ || requested_format_->interval == 0)
+    return false;
+
+  // Reset counters if it's the first frame, if input fps is higher than
+  // requested fps, or if timestamps are out of order. Always output the
+  // first frame after a reset.
+  const int64_t frame_delta_ns = (in_timestamp_ns - previous_timestamp_ns_);
+  if (previous_timestamp_ns_ == 0 || frame_delta_ns <= 0 ||
+      frame_delta_ns >= requested_format_->interval) {
+    interval_ns_next_frame_ = 0;
+    previous_timestamp_ns_ = in_timestamp_ns;
+    return false;
+  }
+
+  // Increment counters.
+  interval_ns_next_frame_ += frame_delta_ns;
+  previous_timestamp_ns_ = in_timestamp_ns;
+
+  // Drop if elapsed time is less then requested.
+  if (interval_ns_next_frame_ < requested_format_->interval)
+    return true;
+
+  // Time to output new frame.
+  interval_ns_next_frame_ -= requested_format_->interval;
+  return false;
 }
 
 void VideoAdapter::AdaptFrameResolution(int in_width,
                                         int in_height,
+                                        int64_t in_timestamp_ns,
                                         int* cropped_width,
                                         int* cropped_height,
                                         int* out_width,
                                         int* out_height) {
   rtc::CritScope cs(&critical_section_);
   ++frames_in_;
 
   // The max output pixel count is the minimum of the requests from
   // OnOutputFormatRequest and OnResolutionRequest.
   int max_pixel_count = resolution_request_max_pixel_count_;
   if (requested_format_) {
     max_pixel_count = std::min(
         max_pixel_count, requested_format_->width * requested_format_->height);
   }
 
   // Drop the input frame if necessary.
-  bool should_drop = false;
-  if (max_pixel_count == 0) {
-    // Drop all frames as the output format is 0x0.
-    should_drop = true;
-  } else if (requested_format_ && requested_format_->interval > 0) {
-    // Drop some frames based on input fps and output fps.
-    // Normally output fps is less than input fps.
-    interval_next_frame_ += input_interval_;
-    if (interval_next_frame_ >= requested_format_->interval) {
-      interval_next_frame_ -= requested_format_->interval;
-      // Reset |interval_next_frame_| if it accumulates too much to avoid
-      // "catching up" behaviour.
-      if (interval_next_frame_ >= requested_format_->interval)
-        interval_next_frame_ = 0;
-    } else {
-      should_drop = true;
-    }
-  }
-  if (should_drop) {
+  if (max_pixel_count == 0 || ShouldDrop(in_timestamp_ns)) {
     // Show VAdapt log every 90 frames dropped. (3 seconds)
     if ((frames_in_ - frames_out_) % 90 == 0) {
       // TODO(fbarchard): Reduce to LS_VERBOSE when adapter info is not needed
       // in default calls.
       LOG(LS_INFO) << "VAdapt Drop Frame: scaled " << frames_scaled_
                    << " / out " << frames_out_
                    << " / in " << frames_in_
                    << " Changes: " << adaption_changes_
                    << " Input: " << in_width
                    << "x" << in_height
-                   << " i" << input_interval_
+                   << " timestamp: " << in_timestamp_ns
                    << " Output: i"
                    << (requested_format_ ? requested_format_->interval : 0);
     }
 
     // Drop frame.
     *cropped_width = 0;
     *cropped_height = 0;
     *out_width = 0;
     *out_height = 0;
     return;
@@ skipping 38 matching lines @@
   ++frames_out_;
   if (scale.numerator != scale.denominator)
     ++frames_scaled_;
 
   if (previous_width_ && (previous_width_ != *out_width ||
                           previous_height_ != *out_height)) {
     ++adaption_changes_;
     LOG(LS_INFO) << "Frame size changed: scaled " << frames_scaled_ << " / out "
                  << frames_out_ << " / in " << frames_in_
                  << " Changes: " << adaption_changes_ << " Input: " << in_width
-                 << "x" << in_height << " i" << input_interval_
+                 << "x" << in_height
                  << " Scale: " << scale.numerator << "/" << scale.denominator
                  << " Output: " << *out_width << "x" << *out_height << " i"
                  << (requested_format_ ? requested_format_->interval : 0);
   }
 
   previous_width_ = *out_width;
   previous_height_ = *out_height;
 }
 
 void VideoAdapter::OnOutputFormatRequest(const VideoFormat& format) {
   rtc::CritScope cs(&critical_section_);
   requested_format_ = rtc::Optional<VideoFormat>(format);
-  interval_next_frame_ = 0;
+  previous_timestamp_ns_ = 0;
+  interval_ns_next_frame_ = 0;
 }
 
 void VideoAdapter::OnResolutionRequest(
     rtc::Optional<int> max_pixel_count,
     rtc::Optional<int> max_pixel_count_step_up) {
   rtc::CritScope cs(&critical_section_);
   resolution_request_max_pixel_count_ =
       max_pixel_count.value_or(std::numeric_limits<int>::max());
   resolution_request_max_pixel_count_step_up_ =
       max_pixel_count_step_up.value_or(0);
 }
 
 }  // namespace cricket