Pass delay and timestamp to AudioSourceCallback::OnMoreData.

chromecast/media/audio/cast_audio_mixer.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #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"
 
 namespace {
 const int kBitsPerSample = 16;
 const int kFramesPerBuffer = 1024;
 const int kSampleRate = 48000;
 }  // namespace
 
@@ skipping 116 matching lines @@
     DCHECK(audio_manager_->GetTaskRunner()->BelongsToCurrentThread());
 
     *volume = volume_;
   }
 
   // ::media::AudioConverter::InputCallback implementation
   double ProvideInput(::media::AudioBus* audio_bus,
                       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_;
   }
 
   CastAudioManager* const audio_manager_;
   CastAudioMixer* const audio_mixer_;
 
   std::unique_ptr<ResamplerProxy> proxy_;
   AudioSourceCallback* source_callback_;
   const ::media::AudioParameters input_params_;
   const ::media::AudioParameters output_params_;
@@ skipping 83 matching lines @@
 void CastAudioMixer::RemoveInput(
     ::media::AudioConverter::InputCallback* input_callback) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   mixer_->RemoveInput(input_callback);
   // Stop |output_stream_| if there are no inputs and the stream is running.
   if (mixer_->empty() && output_stream_)
     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();
 }
 
 void CastAudioMixer::OnError(::media::AudioOutputStream* stream) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(output_stream_ == stream);
 
   // TODO(ameyak): Add rate limiting. If errors are seen to occur
   //               above some arbitrary value in a specified amount
@@ skipping 10 matching lines @@
     // Assume error state
     output_stream_->Close();
     output_stream_ = nullptr;
     error_ = true;
     for (auto it = proxy_streams_.begin(); it != proxy_streams_.end(); it++)
       (*it)->OnError();
   }
 }
 
 }  // namespace media
-}  // namespace chromecast
+}  // namespace chromecast