Use double microseconds for tracking back/front timestamp in AudioClock.

Back timestamp is computed by summing the new frames_written for every
call to WroteAudio. The number of microseconds per frame is often not
a whole number (e.g. 20.833 mu for sample rate of 48Khz). Prior to this
change, using TimeDelta to do the summing of frames_written meant we
truncated to the nearest microsecond with every call to WroteAudio. The
truncation error slowly accumulates in the back timestamp. After 2
hours of playback this error causes noticeable audio/video sync drift.

Having front_timestamp be a double is less critical. Front timestamp
is computed using back_timestamp at every call to WroteAudio, so fixing
back implicitly fixes front. Still, I've changed them both to double
for the sake of consistency and a slight improvement in accuracy.

BUG=564604

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

Cr-Commit-Position: refs/heads/master@{#376287}
(cherry picked from commit 2ed08018de0593b905c18500c1784464dcbe5468)

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

Cr-Commit-Position: refs/branch-heads/2623@{#503}
Cr-Branched-From: 92d77538a86529ca35f9220bd3cd512cbea1f086-refs/heads/master@{#369907}

3 files changed, 83 insertions, 41 deletions

diff --git a/media/filters/audio_clock.cc b/media/filters/audio_clock.cc
index 19c5c2c..0bd0a6c 100644
--- a/media/filters/audio_clock.cc
+++ b/media/filters/audio_clock.cc
@@ -8,6 +8,7 @@
 #include <stddef.h>
 
 #include <algorithm>
+#include <cmath>
 
 #include "base/logging.h"
 
@@ -19,9 +20,8 @@ AudioClock::AudioClock(base::TimeDelta start_timestamp, int sample_rate)
           static_cast<double>(base::Time::kMicrosecondsPerSecond) /
           sample_rate),
       total_buffered_frames_(0),
-      front_timestamp_(start_timestamp),
-      back_timestamp_(start_timestamp) {
-}
+      front_timestamp_micros_(start_timestamp.InMicroseconds()),
+      back_timestamp_micros_(start_timestamp.InMicroseconds()) {}
 
 AudioClock::~AudioClock() {
 }
@@ -36,8 +36,10 @@ void AudioClock::WroteAudio(int frames_written,
   DCHECK_GE(playback_rate, 0);
 
   // First write: initialize buffer with silence.
-  if (start_timestamp_ == front_timestamp_ && buffered_.empty())
+  if (start_timestamp_.InMicroseconds() == front_timestamp_micros_ &&
+      buffered_.empty()) {
     PushBufferedAudioData(delay_frames, 0.0);
+  }
 
   // Move frames from |buffered_| into the computed timestamp based on
   // |delay_frames|.
@@ -53,14 +55,16 @@ void AudioClock::WroteAudio(int frames_written,
   // Update our front and back timestamps.  The back timestamp is considered the
   // authoritative source of truth, so base the front timestamp on range of data
   // buffered.  Doing so avoids accumulation errors on the front timestamp.
-  back_timestamp_ += base::TimeDelta::FromMicroseconds(
-      frames_written * playback_rate * microseconds_per_frame_);
+  back_timestamp_micros_ +=
+      frames_written * playback_rate * microseconds_per_frame_;
+
   // Don't let front timestamp move earlier in time, as could occur due to delay
   // frames pushed in the first write, above.
-  front_timestamp_ = std::max(front_timestamp_,
-                              back_timestamp_ - ComputeBufferedMediaDuration());
-  DCHECK_GE(front_timestamp_, start_timestamp_);
-  DCHECK_LE(front_timestamp_, back_timestamp_);
+  front_timestamp_micros_ =
+      std::max(front_timestamp_micros_,
+               back_timestamp_micros_ - ComputeBufferedMediaDurationMicros());
+  DCHECK_GE(front_timestamp_micros_, start_timestamp_.InMicroseconds());
+  DCHECK_LE(front_timestamp_micros_, back_timestamp_micros_);
 }
 
 void AudioClock::CompensateForSuspendedWrites(base::TimeDelta elapsed,
@@ -81,12 +85,15 @@ void AudioClock::CompensateForSuspendedWrites(base::TimeDelta elapsed,
 }
 
 base::TimeDelta AudioClock::TimeUntilPlayback(base::TimeDelta timestamp) const {
-  DCHECK_GE(timestamp, front_timestamp_);
-  DCHECK_LE(timestamp, back_timestamp_);
+  // Use front/back_timestamp() methods rather than internal members. The public
+  // methods round to the nearest microsecond for conversion to TimeDelta and
+  // the rounded value will likely be used by the caller.
+  DCHECK_GE(timestamp, front_timestamp());
+  DCHECK_LE(timestamp, back_timestamp());
 
   int64_t frames_until_timestamp = 0;
   double timestamp_us = timestamp.InMicroseconds();
-  double media_time_us = front_timestamp_.InMicroseconds();
+  double media_time_us = front_timestamp().InMicroseconds();
 
   for (size_t i = 0; i < buffered_.size(); ++i) {
     // Leading silence is always accounted prior to anything else.
@@ -113,8 +120,8 @@ base::TimeDelta AudioClock::TimeUntilPlayback(base::TimeDelta timestamp) const {
     frames_until_timestamp += buffered_[i].frames;
   }
 
-  return base::TimeDelta::FromMicroseconds(frames_until_timestamp *
-                                           microseconds_per_frame_);
+  return base::TimeDelta::FromMicroseconds(
+      std::round(frames_until_timestamp * microseconds_per_frame_));
 }
 
 void AudioClock::ContiguousAudioDataBufferedForTesting(
@@ -179,12 +186,11 @@ void AudioClock::PopBufferedAudioData(int64_t frames) {
   }
 }
 
-base::TimeDelta AudioClock::ComputeBufferedMediaDuration() const {
+double AudioClock::ComputeBufferedMediaDurationMicros() const {
   double scaled_frames = 0;
   for (const auto& buffer : buffered_)
     scaled_frames += buffer.frames * buffer.playback_rate;
-  return base::TimeDelta::FromMicroseconds(scaled_frames *
-                                           microseconds_per_frame_);
+  return scaled_frames * microseconds_per_frame_;
 }
 
 }  // namespace media
diff --git a/media/filters/audio_clock.h b/media/filters/audio_clock.h
index 024c791..42ee2b6 100644
--- a/media/filters/audio_clock.h
+++ b/media/filters/audio_clock.h
@@ -7,6 +7,7 @@
 
 #include <stdint.h>
 
+#include <cmath>
 #include <deque>
 
 #include "base/macros.h"
@@ -89,8 +90,14 @@ class MEDIA_EXPORT AudioClock {
   // |start_timestamp| since no                  amount of media frames tracked
   // media data has been played yet.             by AudioClock, which would be
   //                                             1000 + 500 + 250 = 1750 ms.
-  base::TimeDelta front_timestamp() const { return front_timestamp_; }
-  base::TimeDelta back_timestamp() const { return back_timestamp_; }
+  base::TimeDelta front_timestamp() const {
+    return base::TimeDelta::FromMicroseconds(
+        std::round(front_timestamp_micros_));
+  }
+  base::TimeDelta back_timestamp() const {
+    return base::TimeDelta::FromMicroseconds(
+        std::round(back_timestamp_micros_));
+  }
 
   // Returns the amount of wall time until |timestamp| will be played by the
   // audio hardware.
@@ -117,7 +124,7 @@ class MEDIA_EXPORT AudioClock {
   // Helpers for operating on |buffered_|.
   void PushBufferedAudioData(int64_t frames, double playback_rate);
   void PopBufferedAudioData(int64_t frames);
-  base::TimeDelta ComputeBufferedMediaDuration() const;
+  double ComputeBufferedMediaDurationMicros() const;
 
   const base::TimeDelta start_timestamp_;
   const double microseconds_per_frame_;
@@ -125,8 +132,14 @@ class MEDIA_EXPORT AudioClock {
   std::deque<AudioData> buffered_;
   int64_t total_buffered_frames_;
 
-  base::TimeDelta front_timestamp_;
-  base::TimeDelta back_timestamp_;
+  // Use double rather than TimeDelta to avoid loss of partial microseconds when
+  // converting between frames-written/delayed and time-passed (see conversion
+  // in WroteAudio()). Particularly for |back_timestamp|, which accumulates more
+  // time with each call to WroteAudio(), the loss of precision can accumulate
+  // to create noticeable audio/video sync drift for longer (2-3 hr) videos.
+  // See http://crbug.com/564604.
+  double front_timestamp_micros_;
+  double back_timestamp_micros_;
 
   DISALLOW_COPY_AND_ASSIGN(AudioClock);
 };
diff --git a/media/filters/audio_clock_unittest.cc b/media/filters/audio_clock_unittest.cc
index 303e8e3..312ba61 100644
--- a/media/filters/audio_clock_unittest.cc
+++ b/media/filters/audio_clock_unittest.cc
@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "base/macros.h"
+#include "base/memory/scoped_ptr.h"
 #include "media/base/audio_timestamp_helper.h"
 #include "media/filters/audio_clock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -11,8 +12,7 @@ namespace media {
 
 class AudioClockTest : public testing::Test {
  public:
-  AudioClockTest()
-      : sample_rate_(10), clock_(base::TimeDelta(), sample_rate_) {}
+  AudioClockTest() { SetupClock(base::TimeDelta(), 10); }
 
   ~AudioClockTest() override {}
 
@@ -20,45 +20,51 @@ class AudioClockTest : public testing::Test {
                   int frames_requested,
                   int delay_frames,
                   double playback_rate) {
-    clock_.WroteAudio(
-        frames_written, frames_requested, delay_frames, playback_rate);
+    clock_->WroteAudio(frames_written, frames_requested, delay_frames,
+                       playback_rate);
   }
 
-  int FrontTimestampInDays() { return clock_.front_timestamp().InDays(); }
+  void SetupClock(base::TimeDelta start_time, int sample_rate) {
+    sample_rate_ = sample_rate;
+    clock_.reset(new AudioClock(start_time, sample_rate_));
+  }
+
+  int FrontTimestampInDays() { return clock_->front_timestamp().InDays(); }
 
   int FrontTimestampInMilliseconds() {
-    return clock_.front_timestamp().InMilliseconds();
+    return clock_->front_timestamp().InMilliseconds();
   }
 
   int BackTimestampInMilliseconds() {
-    return clock_.back_timestamp().InMilliseconds();
+    return clock_->back_timestamp().InMilliseconds();
   }
 
   int TimeUntilPlaybackInMilliseconds(int timestamp_ms) {
-    return clock_.TimeUntilPlayback(base::TimeDelta::FromMilliseconds(
-                                        timestamp_ms)).InMilliseconds();
+    return clock_
+        ->TimeUntilPlayback(base::TimeDelta::FromMilliseconds(timestamp_ms))
+        .InMilliseconds();
   }
 
   int ContiguousAudioDataBufferedInDays() {
     base::TimeDelta total, same_rate_total;
-    clock_.ContiguousAudioDataBufferedForTesting(&total, &same_rate_total);
+    clock_->ContiguousAudioDataBufferedForTesting(&total, &same_rate_total);
     return total.InDays();
   }
 
   int ContiguousAudioDataBufferedInMilliseconds() {
     base::TimeDelta total, same_rate_total;
-    clock_.ContiguousAudioDataBufferedForTesting(&total, &same_rate_total);
+    clock_->ContiguousAudioDataBufferedForTesting(&total, &same_rate_total);
     return total.InMilliseconds();
   }
 
   int ContiguousAudioDataBufferedAtSameRateInMilliseconds() {
     base::TimeDelta total, same_rate_total;
-    clock_.ContiguousAudioDataBufferedForTesting(&total, &same_rate_total);
+    clock_->ContiguousAudioDataBufferedForTesting(&total, &same_rate_total);
     return same_rate_total.InMilliseconds();
   }
 
-  const int sample_rate_;
-  AudioClock clock_;
+  int sample_rate_;
+  scoped_ptr<AudioClock> clock_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(AudioClockTest);
@@ -337,8 +343,8 @@ TEST_F(AudioClockTest, CompensateForSuspendedWrites) {
   // Elapsing frames less than we have buffered should do nothing.
   const int kDelayFrames = 2;
   for (int i = 1000; i <= kBaseTimeMs; i += 1000) {
-    clock_.CompensateForSuspendedWrites(base::TimeDelta::FromMilliseconds(i),
-                                        kDelayFrames);
+    clock_->CompensateForSuspendedWrites(base::TimeDelta::FromMilliseconds(i),
+                                         kDelayFrames);
     EXPECT_EQ(kBaseTimeMs - (i - 1000), TimeUntilPlaybackInMilliseconds(0));
 
     // Write silence to simulate maintaining a 7s output buffer.
@@ -347,9 +353,26 @@ TEST_F(AudioClockTest, CompensateForSuspendedWrites) {
 
   // Exhausting all frames should advance timestamps and prime the buffer with
   // our delay frames value.
-  clock_.CompensateForSuspendedWrites(base::TimeDelta::FromMilliseconds(7000),
-                                      kDelayFrames);
+  clock_->CompensateForSuspendedWrites(base::TimeDelta::FromMilliseconds(7000),
+                                       kDelayFrames);
   EXPECT_EQ(kDelayFrames * 100, TimeUntilPlaybackInMilliseconds(1000));
 }
 
+TEST_F(AudioClockTest, FramesToTimePrecision) {
+  SetupClock(base::TimeDelta(), 48000);
+  double micros_per_frame = base::Time::kMicrosecondsPerSecond / 48000.0;
+  int frames_written = 0;
+
+  // Write ~2 hours of data to clock to give any error a significant chance to
+  // accumulate.
+  while (clock_->back_timestamp() <= base::TimeDelta::FromHours(2)) {
+    frames_written += 1024;
+    WroteAudio(1024, 1024, 0, 1);
+  }
+
+  // Verify no error accumulated.
+  EXPECT_EQ(std::round(frames_written * micros_per_frame),
+            clock_->back_timestamp().InMicroseconds());
+}
+
 }  // namespace media