Implement support for 2-pattern cadence.

An N pattern cadence is simply figuring out if N times the
frame duration has an integer cadence (M) relative to the
render interval, then figuring out how to distribute N
values such that sum(x1 + x2 + x3 ... xN) == M.

I haven't dug through our discussion on how to compute this
for N > 2, but for N == 2, it's simple to just use
x1 == ceil(M/2), x2 == floor(M/2)

This change refactors the main function in VideoCadenceEstimator
into smaller pieces (an original review request :) in order to
reuse them for detecting 2-pattern cadence.

BUG=439548
TEST=all unit tests still pass, 3:2 cadence tests pass.

Review URL: https://codereview.chromium.org/1125893002

Cr-Commit-Position: refs/heads/master@{#329813}

media/filters/video_cadence_estimator.cc

@@ -5,7 +5,9 @@
 #include "media/filters/video_cadence_estimator.h"
 
 #include <algorithm>
+#include <iterator>
 #include <limits>
+#include <string>
 
 #include "base/metrics/histogram_macros.h"
 
@@ -24,10 +26,10 @@ static void HistogramCadenceChangeCount(int cadence_changes) {
 }
 
 VideoCadenceEstimator::VideoCadenceEstimator(
-    base::TimeDelta minimum_time_until_glitch)
+    base::TimeDelta minimum_time_until_max_drift)
     : cadence_hysteresis_threshold_(
           base::TimeDelta::FromMilliseconds(kMinimumCadenceDurationMs)),
-      minimum_time_until_glitch_(minimum_time_until_glitch) {
+      minimum_time_until_max_drift_(minimum_time_until_max_drift) {
   Reset();
 }
 
@@ -35,8 +37,8 @@ VideoCadenceEstimator::~VideoCadenceEstimator() {
 }
 
 void VideoCadenceEstimator::Reset() {
-  cadence_ = fractional_cadence_ = 0;
-  pending_cadence_ = pending_fractional_cadence_ = 0;
+  cadence_.clear();
+  pending_cadence_.clear();
   cadence_changes_ = render_intervals_cadence_held_ = 0;
   first_update_call_ = true;
 }
@@ -48,21 +50,12 @@ bool VideoCadenceEstimator::UpdateCadenceEstimate(
   DCHECK_GT(render_interval, base::TimeDelta());
   DCHECK_GT(frame_duration, base::TimeDelta());
 
-  base::TimeDelta time_until_cadence_glitch;
-  base::TimeDelta time_until_fractional_cadence_glitch;
-
-  // See if the clamped cadence fits acceptable thresholds for exhausting drift.
-  int new_cadence = 0, new_fractional_cadence = 0;
-  if (CalculateCadence(render_interval, frame_duration, max_acceptable_drift,
-                       false, &new_cadence, &time_until_cadence_glitch)) {
-    DCHECK(new_cadence);
-  } else if (CalculateCadence(render_interval, frame_duration,
-                              max_acceptable_drift, true,
-                              &new_fractional_cadence,
-                              &time_until_fractional_cadence_glitch)) {
-    new_cadence = 1;
-    DCHECK(new_fractional_cadence);
-  }
+  base::TimeDelta time_until_max_drift;
+
+  // See if we can find a cadence which fits the data.
+  Cadence new_cadence =
+      CalculateCadence(render_interval, frame_duration, max_acceptable_drift,
+                       &time_until_max_drift);
 
   // If this is the first time UpdateCadenceEstimate() has been called,
   // initialize the histogram with a zero count for cadence changes; this
@@ -73,9 +66,8 @@ bool VideoCadenceEstimator::UpdateCadenceEstimate(
     HistogramCadenceChangeCount(0);
   }
 
-  // Nothing changed, so do nothing.
-  if (new_cadence == cadence_ &&
-      new_fractional_cadence == fractional_cadence_) {
+  // If nothing changed, do nothing.
+  if (new_cadence == cadence_) {
     // Clear cadence hold to pending values from accumulating incorrectly.
     render_intervals_cadence_held_ = 0;
     return false;
@@ -84,18 +76,14 @@ bool VideoCadenceEstimator::UpdateCadenceEstimate(
   // Wait until enough render intervals have elapsed before accepting the
   // cadence change.  Prevents oscillation of the cadence selection.
   bool update_pending_cadence = true;
-  if ((new_cadence == pending_cadence_ &&
-       new_fractional_cadence == pending_fractional_cadence_) ||
+  if (new_cadence == pending_cadence_ ||
       cadence_hysteresis_threshold_ <= render_interval) {
     if (++render_intervals_cadence_held_ * render_interval >=
         cadence_hysteresis_threshold_) {
-      DVLOG(1) << "Cadence switch: (" << cadence_ << ", " << fractional_cadence_
-               << ") -> (" << new_cadence << ", " << new_fractional_cadence
-               << ") :: (" << time_until_cadence_glitch << ", "
-               << time_until_fractional_cadence_glitch << ")";
-
-      cadence_ = new_cadence;
-      fractional_cadence_ = new_fractional_cadence;
+      DVLOG(1) << "Cadence switch: " << CadenceToString(cadence_) << " -> "
+               << CadenceToString(new_cadence)
+               << " :: Time until drift exceeded: " << time_until_max_drift;
+      cadence_.swap(new_cadence);
 
       // Note: Because this class is transitively owned by a garbage collected
       // object, WebMediaPlayer, we log cadence changes as they are encountered.
@@ -106,86 +94,161 @@ bool VideoCadenceEstimator::UpdateCadenceEstimate(
     update_pending_cadence = false;
   }
 
-  DVLOG(2) << "Hysteresis prevented cadence switch: (" << cadence_ << ", "
-           << fractional_cadence_ << ") -> (" << new_cadence << ", "
-           << new_fractional_cadence << ") :: (" << time_until_cadence_glitch
-           << ", " << time_until_fractional_cadence_glitch << ")";
+  DVLOG(2) << "Hysteresis prevented cadence switch: "
+           << CadenceToString(cadence_) << " -> "
+           << CadenceToString(new_cadence);
 
   if (update_pending_cadence) {
-    pending_cadence_ = new_cadence;
-    pending_fractional_cadence_ = new_fractional_cadence;
+    pending_cadence_.swap(new_cadence);
     render_intervals_cadence_held_ = 1;
   }
 
   return false;
 }
 
-bool VideoCadenceEstimator::CalculateCadence(
+int VideoCadenceEstimator::GetCadenceForFrame(uint64_t frame_number) const {
+  DCHECK(has_cadence());
+  return cadence_[frame_number % cadence_.size()];
+}
+
+VideoCadenceEstimator::Cadence VideoCadenceEstimator::CalculateCadence(
     base::TimeDelta render_interval,
     base::TimeDelta frame_duration,
     base::TimeDelta max_acceptable_drift,
-    bool fractional,
-    int* cadence,
-    base::TimeDelta* time_until_glitch) {
+    base::TimeDelta* time_until_max_drift) const {
+  // See if we can find a cadence which fits the data.
+  Cadence result;
+  if (CalculateOneFrameCadence(render_interval, frame_duration,
+                               max_acceptable_drift, &result,
+                               time_until_max_drift)) {
+    DCHECK_EQ(1u, result.size());
+  } else if (CalculateFractionalCadence(render_interval, frame_duration,
+                                        max_acceptable_drift, &result,
+                                        time_until_max_drift)) {
+    DCHECK(!result.empty());
+  } else if (CalculateOneFrameCadence(render_interval, frame_duration * 2,
+                                      max_acceptable_drift, &result,
+                                      time_until_max_drift)) {
+    // By finding cadence for double the frame duration, we're saying there
+    // exist two integers a and b, where a > b and a + b = |result|; this
+    // matches all patterns which regularly have half a frame per render
+    // interval; i.e. 24fps in 60hz.
+    DCHECK_EQ(1u, result.size());
+
+    // Two pattern cadence is always an odd number.
+    DCHECK(result[0] & 1);
+
+    result[0] = std::ceil(result[0] / 2.0);
+    result.push_back(result[0] - 1);
+  }
+  return result;
+}
+
+bool VideoCadenceEstimator::CalculateOneFrameCadence(
+    base::TimeDelta render_interval,
+    base::TimeDelta frame_duration,
+    base::TimeDelta max_acceptable_drift,
+    Cadence* cadence,
+    base::TimeDelta* time_until_max_drift) const {
+  DCHECK(cadence->empty());
+
   // The perfect cadence is the number of render intervals per frame, while the
-  // clamped cadence is the nearest matching integer cadence.
-  //
-  // Fractional cadence is checked to see if we have a cadence which would look
-  // best if we consistently drop the same frames.
+  // clamped cadence is the nearest matching integer value.
   //
   // As mentioned in the introduction, |perfect_cadence| is the ratio of the
   // frame duration to render interval length; while |clamped_cadence| is the
-  // nearest integer value to |perfect_cadence|.  When computing a fractional
-  // cadence (1/|perfect_cadence|), |fractional| must be set to true to ensure
-  // the rendered and actual frame durations are computed correctly.
+  // nearest integer value to |perfect_cadence|.
   const double perfect_cadence =
-      fractional ? render_interval.InSecondsF() / frame_duration.InSecondsF()
-                 : frame_duration.InSecondsF() / render_interval.InSecondsF();
+      frame_duration.InSecondsF() / render_interval.InSecondsF();
   const int clamped_cadence = perfect_cadence + 0.5;
   if (!clamped_cadence)
     return false;
 
-  // Calculate the drift in microseconds for each frame we render at cadence
-  // instead of for its real duration.
+  // For cadence based rendering the actual frame duration is just the frame
+  // duration, while the |rendered_frame_duration| is how long the frame would
+  // be displayed for if we rendered it |clamped_cadence| times.
   const base::TimeDelta rendered_frame_duration =
-      fractional ? render_interval : clamped_cadence * render_interval;
+      clamped_cadence * render_interval;
+  if (!IsAcceptableCadence(rendered_frame_duration, frame_duration,
+                           max_acceptable_drift, time_until_max_drift)) {
+    return false;
+  }
 
-  // When computing a fractional drift, we render the first of |clamped_cadence|
-  // frames and drop |clamped_cadence| - 1 frames.  To make the calculations
-  // below work we need to project out the timestamp of the frame which would be
-  // rendered after accounting for those |clamped_cadence| frames.
-  const base::TimeDelta actual_frame_duration =
-      fractional ? clamped_cadence * frame_duration : frame_duration;
-  if (rendered_frame_duration == actual_frame_duration) {
-    *cadence = clamped_cadence;
+  cadence->push_back(clamped_cadence);
   return true;
+}
+
+bool VideoCadenceEstimator::CalculateFractionalCadence(
+    base::TimeDelta render_interval,
+    base::TimeDelta frame_duration,
+    base::TimeDelta max_acceptable_drift,
+    Cadence* cadence,
+    base::TimeDelta* time_until_max_drift) const {
+  DCHECK(cadence->empty());
+
+  // Fractional cadence allows us to see if we have a cadence which would look
+  // best if we consistently drop the same frames.
+  //
+  // In this case, the perfect cadence is the number of frames per render
+  // interval, while the clamped cadence is the nearest integer value.
+  const double perfect_cadence =
+      render_interval.InSecondsF() / frame_duration.InSecondsF();
+  const int clamped_cadence = perfect_cadence + 0.5;
+  if (!clamped_cadence)
+    return false;
+
+  // For fractional cadence, the rendered duration of each frame is just the
+  // render interval.  While the actual frame duration is the total duration of
+  // all the frames we would end up dropping during that time.
+  const base::TimeDelta actual_frame_duration =
+      clamped_cadence * frame_duration;
+  if (!IsAcceptableCadence(render_interval, actual_frame_duration,
+                           max_acceptable_drift, time_until_max_drift)) {
+    return false;
   }
 
-  // Compute how long it'll take to exhaust the drift using |clamped_cadence|.
+  // Fractional cadence means we render the first of |clamped_cadence| frames
+  // and drop |clamped_cadence| - 1 frames.
+  cadence->insert(cadence->begin(), clamped_cadence, 0);
+  (*cadence)[0] = 1;
+  return true;
+}
+
+std::string VideoCadenceEstimator::CadenceToString(
+    const Cadence& cadence) const {
+  if (cadence.empty())
+    return std::string("[]");
+
+  std::ostringstream os;
+  os << "[";
+  std::copy(cadence.begin(), cadence.end() - 1,
+            std::ostream_iterator<int>(os, ":"));
+  os << cadence.back() << "]";
+  return os.str();
+}
+
+bool VideoCadenceEstimator::IsAcceptableCadence(
+    base::TimeDelta rendered_frame_duration,
+    base::TimeDelta actual_frame_duration,
+    base::TimeDelta max_acceptable_drift,
+    base::TimeDelta* time_until_max_drift) const {
+  if (rendered_frame_duration == actual_frame_duration)
+    return true;
+
+  // Compute how long it'll take to exhaust the drift if frames are rendered for
+  // |rendered_frame_duration| instead of |actual_frame_duration|.
   const double duration_delta =
       (rendered_frame_duration - actual_frame_duration)
           .magnitude()
           .InMicroseconds();
   const int64 frames_until_drift_exhausted =
       std::ceil(max_acceptable_drift.InMicroseconds() / duration_delta);
-  *time_until_glitch = rendered_frame_duration * frames_until_drift_exhausted;
-
-  if (*time_until_glitch >= minimum_time_until_glitch_) {
-    *cadence = clamped_cadence;
-    return true;
-  }
-
-  return false;
-}
-
-int VideoCadenceEstimator::GetCadenceForFrame(int index) const {
-  DCHECK(has_cadence());
-  DCHECK_GE(index, 0);
-
-  if (fractional_cadence_)
-    return index % fractional_cadence_ == 0 ? 1 : 0;
 
-  return cadence_;
+  // If the time until a frame would be repeated or dropped is greater than our
+  // limit of acceptability, the cadence is acceptable.
+  *time_until_max_drift =
+      rendered_frame_duration * frames_until_drift_exhausted;
+  return *time_until_max_drift >= minimum_time_until_max_drift_;
 }
 
 }  // namespace media

media/filters/video_cadence_estimator.h

@@ -5,6 +5,8 @@
 #ifndef MEDIA_FILTERS_VIDEO_CADENCE_ESTIMATOR_H_
 #define MEDIA_FILTERS_VIDEO_CADENCE_ESTIMATOR_H_
 
+#include <vector>
+
 #include "base/time/time.h"
 #include "media/base/media_export.h"
 
@@ -14,30 +16,45 @@ namespace media {
 // render cadence which would allow for optimal uniformity of displayed frame
 // durations over time.
 //
-// Cadence is the ratio of the frame duration to render interval length.  I.e.
-// for 30fps in 60Hz the cadence would be (1/30) / (1/60) = 60 / 30 = 2.  It's
-// common that this is not an exact integer, e.g., 29.974fps in 60Hz which
-// would have a cadence of (1/29.974) / (1/60) = ~2.0029.
+// Cadence is the ideal repeating frame pattern for a group of frames; currently
+// VideoCadenceEstimator supports 1-frame ([N]), 2-frame ([3:2]), and N-frame
+// fractional ([1:0:...:0]) cadences.  Details on what this means are below.
+//
+// The perfect cadence of a set of frames is the ratio of the frame duration to
+// render interval length.  I.e. for 30fps in 60Hz the cadence would be (1/30) /
+// (1/60) = 60 / 30 = 2.  It's common that this is not an exact integer, e.g.,
+// 29.974fps in 60Hz which would have a cadence of (1/29.974) / (1/60) =
+// ~2.0029.
+//
+// The perfect cadence is always a real number.  All N-cadences [a1:a2:..:aN]
+// where aK is an integer are an approximation of the perfect cadence; i.e. the
+// average of [a1:..:aN] will approximately equal the perfect cadence.  When N=1
+// we have a 1-frame cadence, when N=2, we have a 2-frame cadence, etc.
 //
-// Clamped integer cadence means we round the actual cadence (~2.0029 in the
-// pending example) to the nearest integer value (2 in this case).  If the
-// delta between those values is small, we can choose to render frames for the
-// integer number of render intervals; shortening or lengthening the actual
-// rendered frame duration.  Doing so ensures each frame gets an optimal amount
-// of display time.
+// For single frame cadence we just round the perfect cadence (~2.0029 in the
+// previous example) to the nearest integer value (2 in this case; which is
+// denoted as a cadence of [2]).  If the delta between those values is small we
+// can choose to render frames for the integer number of render intervals;
+// shortening or lengthening the actual rendered frame duration.  Doing so
+// ensures each frame gets an optimal amount of display time.
 //
-// Obviously clamping cadence like that leads to drift over time of the actual
-// VideoFrame timestamp relative to its rendered time, so we perform some
-// calculations to ensure we only clamp cadence when it will take some time to
-// drift an undesirable amount; see CalculateCadence() for details on how this
-// calculation is made.
+// The delta between the perfect cadence and the rounded cadence leads to drift
+// over time of the actual VideoFrame timestamp relative to its rendered time,
+// so we perform some calculations to ensure we only use a cadence when it will
+// take some time to drift an undesirable amount; see CalculateCadence() for
+// details on how this calculation is made.
 //
-// Notably, this concept can be extended to include fractional cadence when the
-// frame duration is shorter than the render interval; e.g. 120fps in 60Hz.  In
-// this case, the first frame in each group of N frames is displayed once, while
-// the next N - 1 frames are dropped; where N is the fractional cadence of the
-// frame group.  Using the pending example N = 120/60 = 2. See implementations
-// of CalculateCadence() and UpdateCadenceEstimate() for more details.
+// 2-frame cadence is an extension of 1-frame cadence.  Consider the case of
+// 24fps in 60Hz, which has a perfect cadence of 2.5; rounding up to a cadence
+// of 3 would cause drift to accumulate unusably fast.  A better approximation
+// of this cadence would be [3:2].
+//
+// Fractional cadence is a special case of N-frame cadence which can be used
+// when the frame duration is shorter than the render interval; e.g. 120fps in
+// 60Hz.  In this case, the first frame in each group of N frames is displayed
+// once, while the next N - 1 frames are dropped; i.e. the cadence is of the
+// form [1:0:..:0].  Using the previous example N = 120/60 = 2, which means the
+// cadence would be [1:0].  See CalculateFractionalCadence() for more details.
 //
 // In practice this works out to the following for common setups if we use
 // cadence based selection:
@@ -55,18 +72,18 @@ class MEDIA_EXPORT VideoCadenceEstimator {
   // As mentioned in the introduction, the determination of whether to clamp to
   // a given cadence is based on how long it takes before a frame would have to
   // be dropped or repeated to compensate for reaching the maximum acceptable
-  // drift; this time length is controlled by |minimum_time_until_glitch|.
-  explicit VideoCadenceEstimator(base::TimeDelta minimum_time_until_glitch);
+  // drift; this time length is controlled by |minimum_time_until_max_drift|.
+  explicit VideoCadenceEstimator(base::TimeDelta minimum_time_until_max_drift);
   ~VideoCadenceEstimator();
 
   // Clears stored cadence information.
   void Reset();
 
-  // Updates the estimates for |cadence_| and |fractional_cadence_| based on the
-  // given values as described in the introduction above.
+  // Updates the estimates for |cadence_| based on the given values as described
+  // in the introduction above.
   //
   // Clients should call this and then update the cadence for all frames via the
-  // GetCadenceForFrame() method.
+  // GetCadenceForFrame() method if the cadence changes.
   //
   // Cadence changes will not take affect until enough render intervals have
   // elapsed.  For the purposes of hysteresis, each UpdateCadenceEstimate() call
@@ -78,67 +95,90 @@ class MEDIA_EXPORT VideoCadenceEstimator {
                              base::TimeDelta max_acceptable_drift);
 
   // Returns true if a useful cadence was found.
-  bool has_cadence() const { return cadence_ > 0; }
+  bool has_cadence() const { return !cadence_.empty(); }
 
-  // Given a frame |index|, where zero is the most recently rendered frame,
+  // Given a |frame_number|, where zero is the most recently rendered frame,
   // returns the ideal cadence for that frame.
-  int GetCadenceForFrame(int index) const;
+  //
+  // Note: Callers must track the base |frame_number| relative to all frames
+  // rendered or removed after the first frame for which cadence is detected.
+  // The first frame after cadence is detected has a |frame_number| of 0.
+  //
+  // Frames which come in before the last rendered frame should be ignored in
+  // terms of impact to the base |frame_number|.
+  int GetCadenceForFrame(uint64_t frame_number) const;
 
   void set_cadence_hysteresis_threshold_for_testing(base::TimeDelta threshold) {
     cadence_hysteresis_threshold_ = threshold;
   }
 
-  int get_cadence_for_testing() const {
-    return cadence_ && fractional_cadence_ ? fractional_cadence_ : cadence_;
-  }
+  size_t cadence_size_for_testing() const { return cadence_.size(); }
+  std::string GetCadenceForTesting() const { return CadenceToString(cadence_); }
 
  private:
+  using Cadence = std::vector<int>;
+
+  // Attempts to find a 1-frame, 2-frame, or N-frame fractional cadence; returns
+  // the cadence vector if cadence is found and sets |time_until_max_drift| for
+  // the computed cadence.
+  Cadence CalculateCadence(base::TimeDelta render_interval,
+                           base::TimeDelta frame_duration,
+                           base::TimeDelta max_acceptable_drift,
+                           base::TimeDelta* time_until_max_drift) const;
+
   // Calculates the clamped cadence for the given |render_interval| and
   // |frame_duration|, then calculates how long that cadence can be used before
   // exhausting |max_acceptable_drift|.  If the time until exhaustion is greater
-  // than |minimum_time_until_glitch_|, returns true and sets |cadence| to the
-  // clamped cadence.  If the clamped cadence is unusable, |cadence| will be set
-  // to zero.
-  //
-  // If |fractional| is true, GetCadence() will calculate the clamped cadence
-  // using the ratio of the |render_interval| to |frame_duration| instead of
-  // vice versa.
+  // than |minimum_time_until_max_drift_|, returns true and sets |cadence| to
+  // the clamped cadence.  If the clamped cadence is unusable, |cadence| will be
+  // set to zero.
   //
-  // Sets |time_until_glitch| to the computed glitch time.  Set to zero if the
-  // clamped cadence is unusable.
-  bool CalculateCadence(base::TimeDelta render_interval,
+  // Sets |time_until_max_drift| to the computed glitch time.  Set to zero if
+  // the clamped cadence is unusable.
+  bool CalculateOneFrameCadence(base::TimeDelta render_interval,
+                                base::TimeDelta frame_duration,
+                                base::TimeDelta max_acceptable_drift,
+                                Cadence* cadence,
+                                base::TimeDelta* time_until_max_drift) const;
+
+  // Similar to CalculateCadence() except it tries to find the ideal number of
+  // frames which can fit into a |render_interval|; which means doing the same
+  // calculations as CalculateCadence() but with the ratio of |render_interval|
+  // to |frame_duration| instead of the other way around.
+  bool CalculateFractionalCadence(base::TimeDelta render_interval,
                                   base::TimeDelta frame_duration,
                                   base::TimeDelta max_acceptable_drift,
-                        bool fractional,
-                        int* cadence,
-                        base::TimeDelta* time_until_glitch);
+                                  Cadence* cadence,
+                                  base::TimeDelta* time_until_max_drift) const;
+
+  // Converts a cadence vector into a human readable string of the form
+  // "[a, b, ..., z]".
+  std::string CadenceToString(const Cadence& cadence) const;
+
+  // Returns true if the drift of the rendered frame duration versus its actual
+  // frame duration take longer than |minimum_time_until_max_drift_| to exhaust
+  // |max_acceptable_drift|.  |time_until_max_drift| is set to how long it will
+  // take before a glitch (frame drop or repeat occurs).
+  bool IsAcceptableCadence(base::TimeDelta rendered_frame_duration,
+                           base::TimeDelta actual_frame_duration,
+                           base::TimeDelta max_acceptable_drift,
+                           base::TimeDelta* time_until_max_drift) const;
 
-  // The idealized cadence for all frames seen thus far; updated based upon the
-  // ratio of |frame_duration| to |render_interval|, or vice versa, as given to
-  // UpdateCadenceEstimate().  Zero if no integer cadence could be detected.
-  //
-  // Fractional cadences are handled by strongly preferring the first frame in
-  // a series if it fits within acceptable drift. E.g., with 120fps content on
-  // a 60Hz monitor we'll strongly prefer the first frame of every 2 frames.
-  //
-  // |fractional_cadence_| is the number of frames per render interval; the
-  // first of which would be rendered and the rest dropped.
-  int cadence_;
-  int fractional_cadence_;
-
-  // Used as hysteresis to prevent oscillation between cadence and coverage
-  // based rendering methods.  Pending values are updated upon each new cadence
-  // detected by UpdateCadenceEstimate().
+  // The approximate best N-frame cadence for all frames seen thus far; updated
+  // by UpdateCadenceEstimate().  Empty when no cadence has been detected.
+  Cadence cadence_;
+
+  // Used as hysteresis to prevent oscillation between cadence approximations
+  // for spurious blips in the render interval or frame duration.
   //
   // Once a new cadence is detected, |render_intervals_cadence_held_| is
-  // incremented for each UpdateCadenceEstimate() call where the cadence matches
-  // one of the pending values.  |render_intervals_cadence_held_| is cleared
-  // when a "new" cadence matches |cadence_| or |pending_cadence_|.
+  // incremented for each UpdateCadenceEstimate() call where |cadence_| matches
+  // |pending_cadence_|.  |render_intervals_cadence_held_| is cleared when a
+  // "new" cadence matches |cadence_| or |pending_cadence_|.
   //
   // Once |kMinimumCadenceDurationMs| is exceeded in render intervals, the
-  // detected cadence is set in |cadence_| and |fractional_cadence_|.
-  int pending_cadence_;
-  int pending_fractional_cadence_;
+  // detected cadence is set in |cadence_|.
+  Cadence pending_cadence_;
   int render_intervals_cadence_held_;
   base::TimeDelta cadence_hysteresis_threshold_;
 
@@ -149,7 +189,7 @@ class MEDIA_EXPORT VideoCadenceEstimator {
 
   // The minimum amount of time allowed before a glitch occurs before confirming
   // cadence for a given render interval and frame duration.
-  const base::TimeDelta minimum_time_until_glitch_;
+  const base::TimeDelta minimum_time_until_max_drift_;
 
   DISALLOW_COPY_AND_ASSIGN(VideoCadenceEstimator);
 };

media/filters/video_cadence_estimator_unittest.cc

@@ -3,6 +3,8 @@
 // found in the LICENSE file.
 
 #include "base/memory/scoped_ptr.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/strings/string_split.h"
 #include "base/strings/stringprintf.h"
 #include "media/filters/video_cadence_estimator.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -22,33 +24,47 @@ static base::TimeDelta Interval(double hertz) {
   return base::TimeDelta::FromSecondsD(1.0 / hertz);
 }
 
-static void VerifyCadence(VideoCadenceEstimator* estimator,
+std::vector<int> CreateCadenceFromString(const std::string& cadence) {
+  std::vector<std::string> tokens;
+  CHECK_EQ('[', cadence[0]);
+  CHECK_EQ(']', cadence[cadence.length() - 1]);
+  base::SplitString(cadence.substr(1, cadence.length() - 2), ':', &tokens);
+
+  std::vector<int> result;
+  for (const auto& token : tokens) {
+    int cadence_value = 0;
+    CHECK(base::StringToInt(token, &cadence_value)) << token;
+    result.push_back(cadence_value);
+  }
+
+  return result;
+}
+
+static void VerifyCadenceVector(VideoCadenceEstimator* estimator,
                                 double frame_hertz,
                                 double render_hertz,
-                          int expected_cadence) {
+                                const std::string& expected_cadence) {
   SCOPED_TRACE(base::StringPrintf("Checking %.03f fps into %0.03f", frame_hertz,
                                   render_hertz));
+
+  const std::vector<int> expected_cadence_vector =
+      CreateCadenceFromString(expected_cadence);
+
   estimator->Reset();
   const base::TimeDelta acceptable_drift = Interval(frame_hertz) / 2;
   const bool cadence_changed = estimator->UpdateCadenceEstimate(
       Interval(render_hertz), Interval(frame_hertz), acceptable_drift);
   EXPECT_EQ(cadence_changed, estimator->has_cadence());
-  EXPECT_EQ(!!expected_cadence, estimator->has_cadence());
+  EXPECT_EQ(expected_cadence_vector.empty(), !estimator->has_cadence());
 
   // Nothing further to test.
-  if (!expected_cadence)
+  if (expected_cadence_vector.empty())
     return;
 
-  // Spot check a few frame indices.
-  if (frame_hertz <= render_hertz) {
-    EXPECT_EQ(expected_cadence, estimator->GetCadenceForFrame(0));
-    EXPECT_EQ(expected_cadence, estimator->GetCadenceForFrame(1));
-    EXPECT_EQ(expected_cadence, estimator->GetCadenceForFrame(2));
-  } else {
-    EXPECT_EQ(1, estimator->GetCadenceForFrame(0));
-    EXPECT_EQ(0, estimator->GetCadenceForFrame(1));
-    EXPECT_EQ(1, estimator->GetCadenceForFrame(expected_cadence));
-    EXPECT_EQ(0, estimator->GetCadenceForFrame(expected_cadence + 1));
+  // Spot two cycles of the cadence.
+  for (size_t i = 0; i < expected_cadence_vector.size() * 2; ++i) {
+    ASSERT_EQ(expected_cadence_vector[i % expected_cadence_vector.size()],
+              estimator->GetCadenceForFrame(i));
   }
 }
 
@@ -58,28 +74,31 @@ TEST(VideoCadenceEstimatorTest, CadenceCalculations) {
       base::TimeDelta::FromSeconds(kMinimumAcceptableTimeBetweenGlitchesSecs));
   estimator.set_cadence_hysteresis_threshold_for_testing(base::TimeDelta());
 
-  VerifyCadence(&estimator, 24, 60, 0);
-  VerifyCadence(&estimator, NTSC(24), 60, 0);
-  VerifyCadence(&estimator, 25, 60, 0);
-  VerifyCadence(&estimator, NTSC(30), 60, 2);
-  VerifyCadence(&estimator, 30, 60, 2);
-  VerifyCadence(&estimator, 50, 60, 0);
-  VerifyCadence(&estimator, NTSC(60), 60, 1);
-  VerifyCadence(&estimator, 120, 60, 2);
+  const std::string kEmptyCadence = "[]";
+  VerifyCadenceVector(&estimator, 24, 60, "[3:2]");
+  VerifyCadenceVector(&estimator, NTSC(24), 60, "[3:2]");
+
+  VerifyCadenceVector(&estimator, 25, 60, kEmptyCadence);
+  VerifyCadenceVector(&estimator, NTSC(30), 60, "[2]");
+  VerifyCadenceVector(&estimator, 30, 60, "[2]");
+  VerifyCadenceVector(&estimator, 50, 60, kEmptyCadence);
+  VerifyCadenceVector(&estimator, NTSC(60), 60, "[1]");
+  VerifyCadenceVector(&estimator, 120, 60, "[1:0]");
+  VerifyCadenceVector(&estimator, 120, 24, "[1:0:0:0:0]");
 
   // 50Hz is common in the EU.
-  VerifyCadence(&estimator, NTSC(24), 50, 0);
-  VerifyCadence(&estimator, 24, 50, 0);
-  VerifyCadence(&estimator, NTSC(25), 50, 2);
-  VerifyCadence(&estimator, 25, 50, 2);
-  VerifyCadence(&estimator, NTSC(30), 50, 0);
-  VerifyCadence(&estimator, 30, 50, 0);
-  VerifyCadence(&estimator, NTSC(60), 50, 0);
-  VerifyCadence(&estimator, 60, 50, 0);
-
-  VerifyCadence(&estimator, 25, NTSC(60), 0);
-  VerifyCadence(&estimator, 120, NTSC(60), 0);
-  VerifyCadence(&estimator, 1, NTSC(60), 60);
+  VerifyCadenceVector(&estimator, NTSC(24), 50, kEmptyCadence);
+  VerifyCadenceVector(&estimator, 24, 50, kEmptyCadence);
+  VerifyCadenceVector(&estimator, NTSC(25), 50, "[2]");
+  VerifyCadenceVector(&estimator, 25, 50, "[2]");
+  VerifyCadenceVector(&estimator, NTSC(30), 50, kEmptyCadence);
+  VerifyCadenceVector(&estimator, 30, 50, kEmptyCadence);
+  VerifyCadenceVector(&estimator, NTSC(60), 50, kEmptyCadence);
+  VerifyCadenceVector(&estimator, 60, 50, kEmptyCadence);
+
+  VerifyCadenceVector(&estimator, 25, NTSC(60), kEmptyCadence);
+  VerifyCadenceVector(&estimator, 120, NTSC(60), kEmptyCadence);
+  VerifyCadenceVector(&estimator, 1, NTSC(60), "[60]");
 }
 
 TEST(VideoCadenceEstimatorTest, CadenceVariesWithAcceptableDrift) {
@@ -101,7 +120,7 @@ TEST(VideoCadenceEstimatorTest, CadenceVariesWithAcceptableDrift) {
   EXPECT_TRUE(estimator.UpdateCadenceEstimate(render_interval, frame_interval,
                                               acceptable_drift));
   EXPECT_TRUE(estimator.has_cadence());
-  EXPECT_EQ(2, estimator.get_cadence_for_testing());
+  EXPECT_EQ("[1:0]", estimator.GetCadenceForTesting());
 }
 
 TEST(VideoCadenceEstimatorTest, CadenceVariesWithAcceptableGlitchTime) {
@@ -125,7 +144,7 @@ TEST(VideoCadenceEstimatorTest, CadenceVariesWithAcceptableGlitchTime) {
   EXPECT_TRUE(estimator->UpdateCadenceEstimate(render_interval, frame_interval,
                                                acceptable_drift));
   EXPECT_TRUE(estimator->has_cadence());
-  EXPECT_EQ(2, estimator->get_cadence_for_testing());
+  EXPECT_EQ("[1:0]", estimator->GetCadenceForTesting());
 }
 
 TEST(VideoCadenceEstimatorTest, CadenceHystersisPreventsOscillation) {

media/filters/video_renderer_algorithm.cc

@@ -94,10 +94,12 @@ scoped_refptr<VideoFrame> VideoRendererAlgorithm::Render(
   base::TimeDelta selected_frame_drift;
 
   // Step 4: Attempt to find the best frame by cadence.
+  bool was_frame_selected_by_cadence = false;
   int cadence_overage = 0;
   int frame_to_render =
       FindBestFrameByCadence(first_frame_ ? nullptr : &cadence_overage);
   if (frame_to_render >= 0) {
+    was_frame_selected_by_cadence = true;
     selected_frame_drift =
         CalculateAbsoluteDriftForFrame(deadline_min, frame_to_render);
   }
@@ -121,7 +123,12 @@ scoped_refptr<VideoFrame> VideoRendererAlgorithm::Render(
     }
 
     if (frame_to_render >= 0) {
+      if (was_frame_selected_by_cadence) {
         cadence_overage = 0;
+        was_frame_selected_by_cadence = false;
+        DVLOG(2) << "Overriding frame selected by cadence because of drift: "
+                 << selected_frame_drift;
+      }
       selected_frame_drift =
           CalculateAbsoluteDriftForFrame(deadline_min, frame_to_render);
     }
@@ -132,7 +139,10 @@ scoped_refptr<VideoFrame> VideoRendererAlgorithm::Render(
   // selection is going to be bad because it means no suitable frame has any
   // coverage of the deadline interval.
   if (frame_to_render < 0 || selected_frame_drift > max_acceptable_drift_) {
+    if (was_frame_selected_by_cadence) {
       cadence_overage = 0;
+      was_frame_selected_by_cadence = false;
+    }
     frame_to_render = FindBestFrameByDrift(deadline_min, &selected_frame_drift);
   }
 
@@ -189,11 +199,14 @@ scoped_refptr<VideoFrame> VideoRendererAlgorithm::Render(
       }
     }
 
+    // Increment the frame counter for all frames removed after the last
+    // rendered frame.
+    cadence_frame_counter_ += frame_to_render - last_frame_index_;
     frame_queue_.erase(frame_queue_.begin(),
                        frame_queue_.begin() + frame_to_render);
   }
 
-  if (last_render_had_glitch_) {
+  if (last_render_had_glitch_ && !first_frame_) {
     DVLOG(2) << "Deadline: [" << deadline_min.ToInternalValue() << ", "
              << deadline_max.ToInternalValue()
              << "], Interval: " << render_interval_.InMicroseconds()
@@ -215,6 +228,14 @@ scoped_refptr<VideoFrame> VideoRendererAlgorithm::Render(
       deadline_min >= frame_queue_.front().start_time - render_interval_ / 2) {
     frame_queue_.front().render_count += cadence_overage + 1;
     frame_queue_.front().drop_count += cadence_overage;
+
+    // Once we reach a glitch in our cadence sequence, reset the base frame
+    // number used for defining the cadence sequence.
+    if (!was_frame_selected_by_cadence && cadence_estimator_.has_cadence()) {
+      cadence_frame_counter_ = 0;
+      UpdateCadenceForFrames();
+    }
+
     first_frame_ = false;
   }
 
@@ -235,8 +256,9 @@ size_t VideoRendererAlgorithm::RemoveExpiredFrames(base::TimeTicks deadline) {
 
   // Finds and removes all frames which are too old to be used; I.e., the end of
   // their render interval is further than |max_acceptable_drift_| from the
-  // given |deadline|.
-  size_t frames_to_expire = 0;
+  // given |deadline|.  We also always expire anything inserted before the last
+  // rendered frame.
+  size_t frames_to_expire = last_frame_index_;
   const base::TimeTicks minimum_start_time =
       deadline - max_acceptable_drift_ - average_frame_duration_;
   for (; frames_to_expire < frame_queue_.size() - 1; ++frames_to_expire) {
@@ -247,6 +269,7 @@ size_t VideoRendererAlgorithm::RemoveExpiredFrames(base::TimeTicks deadline) {
   if (!frames_to_expire)
     return 0;
 
+  cadence_frame_counter_ += frames_to_expire - last_frame_index_;
   frame_queue_.erase(frame_queue_.begin(),
                      frame_queue_.begin() + frames_to_expire);
 
@@ -282,6 +305,7 @@ void VideoRendererAlgorithm::Reset() {
   cadence_estimator_.Reset();
   frame_duration_calculator_.Reset();
   first_frame_ = true;
+  cadence_frame_counter_ = 0;
 
   // Default to ATSC IS/191 recommendations for maximum acceptable drift before
   // we have enough frames to base the maximum on frame duration.
@@ -474,6 +498,7 @@ bool VideoRendererAlgorithm::UpdateFrameStatistics() {
   if (!cadence_changed)
     return true;
 
+  cadence_frame_counter_ = 0;
   UpdateCadenceForFrames();
 
   // Thus far there appears to be no need for special 3:2 considerations, the
@@ -489,7 +514,8 @@ void VideoRendererAlgorithm::UpdateCadenceForFrames() {
     // cadence selection.
     frame_queue_[i].ideal_render_count =
         cadence_estimator_.has_cadence()
-            ? cadence_estimator_.GetCadenceForFrame(i - last_frame_index_)
+            ? cadence_estimator_.GetCadenceForFrame(cadence_frame_counter_ +
+                                                    (i - last_frame_index_))
             : 0;
   }
 }

media/filters/video_renderer_algorithm.h

@@ -189,7 +189,8 @@ class MEDIA_EXPORT VideoRendererAlgorithm {
   bool UpdateFrameStatistics();
 
   // Updates the ideal render count for all frames in |frame_queue_| based on
-  // the cadence returned by |cadence_estimator_|.
+  // the cadence returned by |cadence_estimator_|.  Cadence is assigned based
+  // on |frame_counter_|.
   void UpdateCadenceForFrames();
 
   // If |cadence_estimator_| has detected a valid cadence, attempts to find the
@@ -292,6 +293,13 @@ class MEDIA_EXPORT VideoRendererAlgorithm {
   // until the first frame has reached its presentation time.
   bool first_frame_;
 
+  // The frame number of the last rendered frame; incremented for every frame
+  // rendered and every frame dropped or expired since the last rendered frame.
+  //
+  // Given to |cadence_estimator_| when assigning cadence values to the
+  // ReadyFrameQueue.  Cleared when a new cadence is detected.
+  uint64_t cadence_frame_counter_;
+
   DISALLOW_COPY_AND_ASSIGN(VideoRendererAlgorithm);
 };
 

media/filters/video_renderer_algorithm_unittest.cc

@@ -109,7 +109,7 @@ class VideoRendererAlgorithmTest : public testing::Test {
 
   size_t frames_queued() const { return algorithm_.frame_queue_.size(); }
 
-  int GetCadence(double frame_rate, double display_rate) {
+  std::string GetCadence(double frame_rate, double display_rate) {
     TickGenerator display_tg(tick_clock_->NowTicks(), display_rate);
     TickGenerator frame_tg(base::TimeTicks(), frame_rate);
     time_source_.StartTicking();
@@ -122,7 +122,8 @@ class VideoRendererAlgorithmTest : public testing::Test {
     EXPECT_TRUE(RenderAndStep(&display_tg, &frames_dropped));
 
     // Store cadence before reseting the algorithm.
-    const int cadence = algorithm_.cadence_estimator_.get_cadence_for_testing();
+    const std::string cadence =
+        algorithm_.cadence_estimator_.GetCadenceForTesting();
     time_source_.StopTicking();
     algorithm_.Reset();
     return cadence;
@@ -230,7 +231,7 @@ class VideoRendererAlgorithmTest : public testing::Test {
         // The frame estimate should be off by at most one frame.
         const size_t estimated_frames_queued =
             frames_queued() /
-            algorithm_.cadence_estimator_.get_cadence_for_testing();
+            algorithm_.cadence_estimator_.cadence_size_for_testing();
         ASSERT_NEAR(algorithm_.EffectiveFramesQueued(), estimated_frames_queued,
                     1);
       }
@@ -888,10 +889,10 @@ TEST_F(VideoRendererAlgorithmTest, BestFrameByFractionalCadence) {
   }
 }
 
-// Verify a 3:2 frame pattern for 23.974fps in 60Hz; doubles as a test for best
-// frame by coverage.
+// Verify a 3:2 frame pattern for 23.974fps and 24fps in 60Hz.
 TEST_F(VideoRendererAlgorithmTest, FilmCadence) {
   const double kTestRates[] = {NTSC(24), 24};
+  disable_cadence_hysteresis();
 
   for (double frame_rate : kTestRates) {
     scoped_refptr<VideoFrame> current_frame;
@@ -916,7 +917,7 @@ TEST_F(VideoRendererAlgorithmTest, FilmCadence) {
           }
 
           current_frame = frame;
-          ASSERT_FALSE(is_using_cadence());
+          ASSERT_TRUE(is_using_cadence());
         });
 
     if (HasFatalFailure())
@@ -926,28 +927,28 @@ TEST_F(VideoRendererAlgorithmTest, FilmCadence) {
 
 // Spot check common display and frame rate pairs for correctness.
 TEST_F(VideoRendererAlgorithmTest, CadenceCalculations) {
-  ASSERT_FALSE(GetCadence(24, 60));
-  ASSERT_FALSE(GetCadence(NTSC(24), 60));
-  ASSERT_FALSE(GetCadence(25, 60));
-  ASSERT_EQ(2, GetCadence(NTSC(30), 60));
-  ASSERT_EQ(2, GetCadence(30, 60));
-  ASSERT_FALSE(GetCadence(50, 60));
-  ASSERT_EQ(1, GetCadence(NTSC(60), 60));
-  ASSERT_EQ(2, GetCadence(120, 60));
+  ASSERT_EQ("[3:2]", GetCadence(24, 60));
+  ASSERT_EQ("[3:2]", GetCadence(NTSC(24), 60));
+  ASSERT_EQ("[]", GetCadence(25, 60));
+  ASSERT_EQ("[2]", GetCadence(NTSC(30), 60));
+  ASSERT_EQ("[2]", GetCadence(30, 60));
+  ASSERT_EQ("[]", GetCadence(50, 60));
+  ASSERT_EQ("[1]", GetCadence(NTSC(60), 60));
+  ASSERT_EQ("[1:0]", GetCadence(120, 60));
 
   // 50Hz is common in the EU.
-  ASSERT_FALSE(GetCadence(NTSC(24), 50));
-  ASSERT_FALSE(GetCadence(24, 50));
-  ASSERT_EQ(2, GetCadence(NTSC(25), 50));
-  ASSERT_EQ(2, GetCadence(25, 50));
-  ASSERT_FALSE(GetCadence(NTSC(30), 50));
-  ASSERT_FALSE(GetCadence(30, 50));
-  ASSERT_FALSE(GetCadence(NTSC(60), 50));
-  ASSERT_FALSE(GetCadence(60, 50));
-
-  ASSERT_FALSE(GetCadence(25, NTSC(60)));
-  ASSERT_EQ(2, GetCadence(120, NTSC(60)));
-  ASSERT_EQ(60, GetCadence(1, NTSC(60)));
+  ASSERT_EQ("[]", GetCadence(NTSC(24), 50));
+  ASSERT_EQ("[]", GetCadence(24, 50));
+  ASSERT_EQ("[2]", GetCadence(NTSC(25), 50));
+  ASSERT_EQ("[2]", GetCadence(25, 50));
+  ASSERT_EQ("[]", GetCadence(NTSC(30), 50));
+  ASSERT_EQ("[]", GetCadence(30, 50));
+  ASSERT_EQ("[]", GetCadence(NTSC(60), 50));
+  ASSERT_EQ("[]", GetCadence(60, 50));
+
+  ASSERT_EQ("[]", GetCadence(25, NTSC(60)));
+  ASSERT_EQ("[1:0]", GetCadence(120, NTSC(60)));
+  ASSERT_EQ("[60]", GetCadence(1, NTSC(60)));
 }
 
 TEST_F(VideoRendererAlgorithmTest, RemoveExpiredFrames) {