Pass delay and timestamp to AudioSourceCallback::OnMoreData.

media/audio/audio_output_resampler.cc

Index: media/audio/audio_output_resampler.cc
diff --git a/media/audio/audio_output_resampler.cc b/media/audio/audio_output_resampler.cc
index c643b3e672c24ed086c07bf5d19262d396b66697..71e64d03809b5e6b7583c23b0f749b7e6baa78a8 100644
--- a/media/audio/audio_output_resampler.cc
+++ b/media/audio/audio_output_resampler.cc
@@ -6,6 +6,9 @@
 
 #include <stdint.h>
 
+#include <algorithm>
+#include <string>
+
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/compiler_specific.h"
@@ -31,9 +34,9 @@ class OnMoreDataConverter
   ~OnMoreDataConverter() override;
 
   // AudioSourceCallback interface.
-  int OnMoreData(AudioBus* dest,
-                 uint32_t total_bytes_delay,
-                 uint32_t frames_skipped) override;
+  int OnMoreData(base::TimeTicks target_playout_time,
+                 int prior_frames_skipped,
+                 AudioBus* dest) override;
   void OnError(AudioOutputStream* stream) override;
 
   // Sets |source_callback_|.  If this is not a new object, then Stop() must be
@@ -58,11 +61,11 @@ class OnMoreDataConverter
   // Source callback.
   AudioOutputStream::AudioSourceCallback* source_callback_;
 
-  // Last |total_bytes_delay| received via OnMoreData(), used to correct
-  // playback delay by ProvideInput() and passed on to |source_callback_|.
-  uint32_t current_total_bytes_delay_;
+  // Last |target_playout_time| received via OnMoreData() and passed on to
+  // |source_callback_|.
+  base::TimeTicks target_playout_time_;
 
-  const int input_bytes_per_frame_;
+  const int input_samples_per_second_;
 
   // Handles resampling, buffering, and channel mixing between input and output
   // parameters.
@@ -346,7 +349,7 @@ OnMoreDataConverter::OnMoreDataConverter(const AudioParameters& input_params,
     : io_ratio_(static_cast<double>(input_params.GetBytesPerSecond()) /
                 output_params.GetBytesPerSecond()),
       source_callback_(nullptr),
-      input_bytes_per_frame_(input_params.GetBytesPerFrame()),
+      input_samples_per_second_(input_params.sample_rate()),
       audio_converter_(input_params, output_params, false),
       error_occurred_(false) {}
 
@@ -373,10 +376,10 @@ void OnMoreDataConverter::Stop() {
   audio_converter_.RemoveInput(this);
 }
 
-int OnMoreDataConverter::OnMoreData(AudioBus* dest,
-                                    uint32_t total_bytes_delay,
-                                    uint32_t frames_skipped) {
-  current_total_bytes_delay_ = total_bytes_delay;
+int OnMoreDataConverter::OnMoreData(base::TimeTicks target_playout_time,
+                                    int /* prior_frames_skipped */,
+                                    AudioBus* dest) {
+  target_playout_time_ = target_playout_time;
   audio_converter_.Convert(dest);
 
   // Always return the full number of frames requested, ProvideInput()
@@ -386,16 +389,14 @@ int OnMoreDataConverter::OnMoreData(AudioBus* dest,
 
 double OnMoreDataConverter::ProvideInput(AudioBus* dest,
                                          uint32_t frames_delayed) {
-  // Adjust playback delay to include |frames_delayed|.
-  // TODO(dalecurtis): Stop passing bytes around, it doesn't make sense since
-  // AudioBus is just float data.  Use TimeDelta instead.
-  uint32_t new_total_bytes_delay = base::saturated_cast<uint32_t>(
-      io_ratio_ *
-      (current_total_bytes_delay_ + frames_delayed * input_bytes_per_frame_));
-
+  base::TimeTicks new_target_playout_time =

[chcunningham1, 2016/08/27 00:35:00]

This new math looks good to me, but I'd like to have Dale double check. 

Dale, I think the original math was wrong to multiply the entire thing by
io_ratio_. We used to have:

new_bytes_delay = io_ratio_ * (current_total_bytes_delay_ + frames_delayed *
input_bytes_per_frame_);

I think this should have been:

new_bytes_delay = (io_ratio_ * current_total_bytes_delay_) + (frames_delayed *
input_bytes_per_frame_);

extra parens added for clarity ... my point being frames_delayed is already
given in terms of input frames (hence the scaling by input_bytes_per_frame). 

If I'm missing something subtle I may need to re-assess the math in this latest
ps.

[miu, 2016/08/31 23:26:54]

Actually, Hubbe originally wrote this code. I'll add him to this code review to
check.

[DaleCurtis, 2016/09/01 18:46:13]

Looking at the code that does seem correct. 

[James West, 2016/09/13 07:40:50]

Acknowledged.

+      target_playout_time_ +
+      base::TimeDelta::FromMicroseconds(frames_delayed *
+                                        base::Time::kMicrosecondsPerSecond /
+                                        input_samples_per_second_);
   // Retrieve data from the original callback.
   const int frames =
-      source_callback_->OnMoreData(dest, new_total_bytes_delay, 0);
+      source_callback_->OnMoreData(new_target_playout_time, 0, dest);
 
   // Zero any unfilled frames if anything was filled, otherwise we'll just
   // return a volume of zero and let AudioConverter drop the output.