Tab Capture: AnimatedContentSampler subsampling and phase fixes in tests

content/browser/media/capture/animated_content_sampler.h

Index: content/browser/media/capture/animated_content_sampler.h
diff --git a/content/browser/media/capture/animated_content_sampler.h b/content/browser/media/capture/animated_content_sampler.h
index de7b3016d84e31208456ab7699871203bc3f21b4..f8b187be2c03ec3ca83bb066e2e34c0f9f3400cd 100644
--- a/content/browser/media/capture/animated_content_sampler.h
+++ b/content/browser/media/capture/animated_content_sampler.h
@@ -27,6 +27,13 @@ class CONTENT_EXPORT AnimatedContentSampler {
   explicit AnimatedContentSampler(base::TimeDelta min_capture_period);
   ~AnimatedContentSampler();
 
+  // Get/Set the target sampling period.  This is used to determine whether to
+  // subsample the frames of animated content.
+  base::TimeDelta target_sampling_period() const {
+    return target_sampling_period_;
+  }
+  void SetTargetSamplingPeriod(base::TimeDelta period);
+
   // Examines the given presentation event metadata, along with recent history,
   // to detect animated content, updating the state of this sampler.
   // |damage_rect| is the region of a frame about to be drawn, while
@@ -42,9 +49,14 @@ class CONTENT_EXPORT AnimatedContentSampler {
   bool ShouldSample() const;
 
   // Returns a frame timestamp to provide to consumers of the sampled frame.
-  // Only valid when should_sample() returns true.
+  // Only valid when ShouldSample() returns true.
   base::TimeTicks frame_timestamp() const { return frame_timestamp_; }
 
+  // Returns the current sampling period.  This can be treated as the estimated
+  // duration of the frame to be sampled.  Only valid when ShouldSample()
+  // returns true.
+  base::TimeDelta sampling_period() const { return sampling_period_; }
+
   // Accessors to currently-detected animating region/period, for logging.
   const gfx::Rect& detected_region() const { return detected_region_; }
   base::TimeDelta detected_period() const { return detected_period_; }
@@ -86,7 +98,12 @@ class CONTENT_EXPORT AnimatedContentSampler {
 
   // Called by ConsiderPresentationEvent() when the current event is part of a
   // detected animation, to update |frame_timestamp_|.
-  void UpdateFrameTimestamp(base::TimeTicks event_time);
+  base::TimeTicks ComputeNextFrameTimestamp(base::TimeTicks event_time) const;
+
+  static base::TimeDelta ComputeSamplingPeriod(

[hubbe, 2015/05/06 19:53:18]

Comment?

[miu, 2015/05/09 20:57:39]

Done.

+      base::TimeDelta animation_period,
+      base::TimeDelta target_sampling_period,
+      base::TimeDelta min_capture_period);
 
   // The client expects frame timestamps to be at least this far apart.
   const base::TimeDelta min_capture_period_;
@@ -103,23 +120,25 @@ class CONTENT_EXPORT AnimatedContentSampler {
   // that means "not detected."
   base::TimeDelta detected_period_;
 
-  // The rewritten frame timestamp for the latest event.
-  base::TimeTicks frame_timestamp_;
+  // Target period between sampled frames.  This can be changed by the client at
+  // any time (e.g., to sample high frame rate content at a lower rate).
+  base::TimeDelta target_sampling_period_;
+
+  // The sampling period computed during the last call to
+  // ConsiderPresentationEvent().
+  base::TimeDelta sampling_period_;
 
-  // The frame timestamp provided in the last call to RecordSample().  This
-  // timestamp may or may not have been one proposed by AnimatedContentSampler.
-  base::TimeTicks recorded_frame_timestamp_;
+  // Indicates whether the last event caused animated content to be detected and
+  // whether the current event should be sampled.
+  enum { NOT_SAMPLING, START_SAMPLING,

[hubbe, 2015/05/06 19:53:18]

one per line?

[miu, 2015/05/09 20:57:38]

Done.

+         SHOULD_NOT_SAMPLE, SHOULD_SAMPLE } sampling_state_;
 
-  // Accumulates all the time advancements since the last call to
-  // RecordSample().  When this is greater than zero, there have been one or
-  // more events proposed for sampling, but not yet recorded.  This accounts for
-  // the cases where AnimatedContentSampler indicates a frame should be sampled,
-  // but the client chooses not to do so.
-  base::TimeDelta sequence_offset_;
+  // A token bucket that is used to decide which subset of the frames containing

[hubbe, 2015/05/06 19:53:18]

Wait, what?
How exactly is TimeDelta == token bucket?

[miu, 2015/05/09 20:57:38]

One microsecond == one token.  When you can take, say, 33333 tokens, you get to
sample an event.  I updated the comment to clarify.

+  // the animated content should be sampled.
+  base::TimeDelta token_bucket_;
 
-  // A token bucket that is used to decide which frames to drop whenever
-  // |detected_period_| is less than |min_capture_period_|.
-  base::TimeDelta borrowed_time_;
+  // The rewritten frame timestamp for the latest event.
+  base::TimeTicks frame_timestamp_;
 };
 
 }  // namespace content