Pass delay and timestamp to AudioSourceCallback::OnMoreData.

chromecast/media/audio/cast_audio_mixer.cc

Index: chromecast/media/audio/cast_audio_mixer.cc
diff --git a/chromecast/media/audio/cast_audio_mixer.cc b/chromecast/media/audio/cast_audio_mixer.cc
index fc73fbbf4d8013581545959bac4df8248c6db9e9..2eb5097c424991969663072b6d84cf142828b240 100644
--- a/chromecast/media/audio/cast_audio_mixer.cc
+++ b/chromecast/media/audio/cast_audio_mixer.cc
@@ -4,6 +4,7 @@
 
 #include "chromecast/media/audio/cast_audio_mixer.h"
 
+#include "base/logging.h"
 #include "chromecast/media/audio/cast_audio_manager.h"
 #include "chromecast/media/audio/cast_audio_output_stream.h"
 #include "media/base/channel_layout.h"
@@ -140,8 +141,10 @@ class CastAudioMixer::MixerProxyStream
                       uint32_t frames_delayed) override {
     DCHECK(source_callback_);
 
-    uint32_t bytes_delay = frames_delayed * input_params_.GetBytesPerFrame();
-    source_callback_->OnMoreData(audio_bus, bytes_delay, 0);
+    const base::TimeDelta delay = base::TimeDelta::FromMicroseconds(
+        frames_delayed * base::Time::kMicrosecondsPerSecond /
+        input_params_.sample_rate());

[chcunningham, 2016/09/23 20:53:29]

int division

[jameswest, 2016/09/29 00:52:24]

Moved to utility function.

+    source_callback_->OnMoreData(delay, base::TimeTicks::Now(), 0, audio_bus);
     return volume_;
   }
 
@@ -245,13 +248,13 @@ void CastAudioMixer::RemoveInput(
     output_stream_->Stop();
 }
 
-int CastAudioMixer::OnMoreData(::media::AudioBus* dest,
-                               uint32_t total_bytes_delay,
-                               uint32_t frames_skipped) {
+int CastAudioMixer::OnMoreData(base::TimeDelta delay,
+                               base::TimeTicks /* delay_timestamp */,
+                               int /* prior_frames_skipped */,
+                               ::media::AudioBus* dest) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
-  uint32_t frames_delayed =
-      total_bytes_delay / output_params_.GetBytesPerFrame();
+  uint32_t frames_delayed = delay.InSecondsF() * output_params_.sample_rate();

[chcunningham, 2016/09/23 20:53:29]

What do you think about delay in frames instead of TimeDelta? We could skip this
conversion and get a little more precision. Its late feedback for this review
though - I don't care enough to block you further if you feel this has gone on
long enough ;)

[miu, 2016/09/28 23:04:41]

I'm torn between both, myself. But, overall, I suppose I'm slightly in favor of
the way we have it.

FWICT, here are the pros/cons of each:

* TimeDelta delay pros: Easier math in OnMoreData() impls, when computing target
playout time, and no need for them to look-up sample rate to map from # frames
to # microseconds. Also, resamplers do not need to recompute delay (unless
stretching a signal).

* TimeDelta delay cons: If client code needs to know an exact # frames, it may
occasionally be off-by-one unless we are more careful about the math. For
example, at 48kHz, one sample delay is equal to 20.8333... microseconds. As the
code is right now, if there was a one sample delay, our calls to OnMoreData()
would report the delay as 20 microseconds, not 21 microseconds. 21 is more
accurate, so maybe the caller of OnMoreData() should do rounding. OR, it would
also work for the impl of OnMoreData() to do rounding (20 usec is clearly one
frame of delay, not zero frames). OR, maybe we do NO rounding because such
accuracy is not necessary?

* frame delay pros: An exact delay measurement (because the timebase is the
sample rate).

* frame delay cons: Harder to compute target playout time. Also, depending on
client code to do the correct math downstream.

[jameswest, 2016/09/29 00:52:24]

I'm personally ambivalent. One thing I'll add is that frames are not always more
accurate because on some platforms, such as Chrome OS, the underlying audio
library gives us the delay in time rather than frames. I'll leave it as
TimeDelta for this change since that's the way I already have it. It shouldn't
be too difficult to change it later if we decide frames are better.

I didn't do any rounding because I don't think we should add an operation
without evidence that it actually helps, but I don't mind adding it if someone
thinks we should. We only have to change it in one place now if we need to
change it.

   mixer_->ConvertWithDelay(frames_delayed, dest);
   return dest->frames();
 }
@@ -282,4 +285,4 @@ void CastAudioMixer::OnError(::media::AudioOutputStream* stream) {
 }
 
 }  // namespace media
-}  // namespace chromecast
+}  // namespace chromecast