Fix Bug: Video with Variable Frame Rate plays at incorrect speed.

media/filters/video_cadence_estimator.cc

Index: media/filters/video_cadence_estimator.cc
diff --git a/media/filters/video_cadence_estimator.cc b/media/filters/video_cadence_estimator.cc
index 8804bf2d63a92edac0379b9ad6e6f697f2ea165c..74670ad16aa3de161111609b9273ca2502324d8d 100644
--- a/media/filters/video_cadence_estimator.cc
+++ b/media/filters/video_cadence_estimator.cc
@@ -18,6 +18,15 @@ namespace media {
 // require some time to elapse before a cadence switch is accepted.
 const int kMinimumCadenceDurationMs = 100;
 
+// The numbers are used to decide whether the current video is variable FPS or
+// constant FPS. If ratio of the sample deviation and the sample average is
+// above |kVariableFPSFactor|, then it is recognized as a variable FPS, and if
+// the ratio is below |kConstantFPSFactor|, then it is recognized as a constant
+// FPS, and if the ratio is in between the two factors, then we do not change
+// previous recognition.
+const double kVariableFPSFactor = 0.1;

[DaleCurtis, 2015/11/20 23:50:46]

Your variable frame rate clip was 0.7? If so it seems like this should be around
that value as well.

[qiangchen, 2015/11/25 18:06:33]

Simply excluding that extreme case may not be optimal. Then one can make another
variable FPS video with variation a little below the threshold and then the
issue still exists obviously.

I'll try to use a more theoretical way to determine the threshold.

That is near the threshold, Cadence method would result in almost same render
length error with that of drift method.

[DaleCurtis, 2015/11/25 21:29:24]

Sure, but I think we want to err on the side of caution here, lets just exclude
things that are *absolutely* variable frame rate material until we have more
data points.

[qiangchen, 2015/12/01 19:37:53]

Done.

+const double kConstantFPSFactor = 0.07;
+
 // Records the number of cadence changes to UMA.
 static void HistogramCadenceChangeCount(int cadence_changes) {
   const int kCadenceChangeMax = 10;
@@ -66,7 +75,8 @@ VideoCadenceEstimator::VideoCadenceEstimator(
     base::TimeDelta minimum_time_until_max_drift)
     : cadence_hysteresis_threshold_(
           base::TimeDelta::FromMilliseconds(kMinimumCadenceDurationMs)),
-      minimum_time_until_max_drift_(minimum_time_until_max_drift) {
+      minimum_time_until_max_drift_(minimum_time_until_max_drift),
+      is_variable_frame_rate_(false) {
   Reset();
 }
 
@@ -83,10 +93,29 @@ void VideoCadenceEstimator::Reset() {
 bool VideoCadenceEstimator::UpdateCadenceEstimate(
     base::TimeDelta render_interval,
     base::TimeDelta frame_duration,
+    base::TimeDelta frame_duration_deviation,
     base::TimeDelta max_acceptable_drift) {
   DCHECK_GT(render_interval, base::TimeDelta());
   DCHECK_GT(frame_duration, base::TimeDelta());
 
+  if (frame_duration_deviation.InMillisecondsF() >
+      kVariableFPSFactor * frame_duration.InMillisecondsF()) {
+    is_variable_frame_rate_ = true;
+  } else if (frame_duration_deviation.InMillisecondsF() <
+             kConstantFPSFactor * frame_duration.InMillisecondsF()) {
+    is_variable_frame_rate_ = false;
+  }
+
+  // Variable FPS detected, turn off Cadence by force.
+  if (is_variable_frame_rate_) {
+    render_intervals_cadence_held_ = 0;
+    if (cadence_ != Cadence()) {

[DaleCurtis, 2015/11/20 23:50:46]

if !cadence_.empty() cadence_.clear()

[qiangchen, 2015/11/25 18:06:33]

Done.

+      cadence_ = Cadence();
+      return true;
+    }
+    return false;
+  }
+
   base::TimeDelta time_until_max_drift;
 
   // See if we can find a cadence which fits the data.