Pass delay and timestamp to AudioSourceCallback::OnMoreData.

media/audio/audio_output_controller.cc

 // Copyright (c) 2012 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 "media/audio/audio_output_controller.h"
 
 #include <stdint.h>
 
+#include <algorithm>
 #include <limits>
 
 #include "base/bind.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/numerics/safe_conversions.h"
 #include "base/task_runner_util.h"
 #include "base/threading/platform_thread.h"
 #include "base/time/time.h"
 #include "base/trace_event/trace_event.h"
 
@@ skipping 266 matching lines @@
   if (stream_ != diverting_to_stream_)
     OnDeviceChange();
 }
 
 void AudioOutputController::DoReportError() {
   DCHECK(message_loop_->BelongsToCurrentThread());
   if (state_ != kClosed)
     handler_->OnError();
 }
 
-int AudioOutputController::OnMoreData(AudioBus* dest,
-                                      uint32_t total_bytes_delay,
-                                      uint32_t frames_skipped) {
+int AudioOutputController::OnMoreData(base::TimeTicks target_playout_time,
+                                      int prior_frames_skipped,
+                                      AudioBus* dest) {
   TRACE_EVENT0("audio", "AudioOutputController::OnMoreData");
 
   // Indicate that we haven't wedged (at least not indefinitely, WedgeCheck()
   // may have already fired if OnMoreData() took an abnormal amount of time).
   // Since this thread is the only writer of |on_more_io_data_called_| once the
   // thread starts, its safe to compare and then increment.
   if (base::AtomicRefCountIsZero(&on_more_io_data_called_))
     base::AtomicRefCountInc(&on_more_io_data_called_);
 
   sync_reader_->Read(dest);
 
+  const base::TimeDelta delay =

[chcunningham, 2016/08/27 00:36:18]

Can you add a comment explaining why this guard is needed. Something like the
explanation you already gave in the CL discussion is great.

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

Looks like this API change should propagate through the IPC interface. If you're
not going to tackle that in this CL, can you put a TODO comment here and file a
bug at crbug.com for tracking purposes?

BTW--If we fix the IPC interface, there would be no reason to call
TimeTicks::Now() at all. ;-)

[jameswest, 2016/09/07 21:52:16]

When the number of bytes is passed to UpdatePendingBytes, where does that
ultimately go? I see it gets sent to a SyncSocket in AudioSyncReader, but I
can't figure out where that goes.

[miu, 2016/09/07 22:19:36]

The consumer is media::AudioDeviceThread:
https://cs.chromium.org/chromium/src/media/audio/audio_device_thread.cc?rcl=1...

That's a real-time thread, and when it calls callback_->Process(), the
implementation in media::AudioOutputDevice is invoked:
https://cs.chromium.org/chromium/src/media/audio/audio_output_device.cc?rcl=0...

From there, AudioOutputDevice calls AudioRendererSink::Render(), which should be
changed to identically match the OnMoreData() API:
https://cs.chromium.org/chromium/src/media/base/audio_renderer_sink.h?rcl=147...

It's up to you whether you want to do these things. On the one hand, they really
are needed now (by https://codereview.chromium.org/2060833002/), and I'm sure
Mikhail would really appreciate you doing the work since, otherwise, he'd have
to. On the other hand, changing the AudioRendererSink interface would require
tweaking the audio handling code in about 10-15 other classes, and so it's not
just a quick thing for you.

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

I'm happy to do this, but I think it should be a separate change to avoid
greatly expanding the size of this one. I added a TODO.

[miu, 2016/09/16 18:35:58]

SGTM.

+      std::max(target_playout_time - base::TimeTicks::Now(), base::TimeDelta());
+  const int total_bytes_delay = delay.InSeconds() * params_.GetBytesPerSecond();

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

s/InSeconds/InSecondsF/ to prevent serious loss of precision!

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

Done.

   const int frames = dest->frames();
   sync_reader_->UpdatePendingBytes(
-      total_bytes_delay + frames * params_.GetBytesPerFrame(), frames_skipped);
+      total_bytes_delay + frames * params_.GetBytesPerFrame(),
+      prior_frames_skipped);
 
   bool need_to_duplicate = false;
   {
     base::AutoLock lock(duplication_targets_lock_);
     need_to_duplicate = !duplication_targets_.empty();
   }
   if (need_to_duplicate) {
-    const base::TimeTicks reference_time =
-        base::TimeTicks::Now() +
-        base::TimeDelta::FromMicroseconds(base::Time::kMicrosecondsPerSecond *
-                                          total_bytes_delay /
-                                          params_.GetBytesPerSecond());
     std::unique_ptr<AudioBus> copy(AudioBus::Create(params_));
     dest->CopyTo(copy.get());
     message_loop_->PostTask(
         FROM_HERE,
         base::Bind(&AudioOutputController::BroadcastDataToDuplicationTargets,
-                   this, base::Passed(&copy), reference_time));
+                   this, base::Passed(&copy), target_playout_time));
   }
 
   if (will_monitor_audio_levels())
     power_monitor_.Scan(*dest, frames);
 
   return frames;
 }
 
 void AudioOutputController::BroadcastDataToDuplicationTargets(
     std::unique_ptr<AudioBus> audio_bus,
@@ skipping 165 matching lines @@
   DCHECK(message_loop_->BelongsToCurrentThread());
 
   // If we should be playing and we haven't, that's a wedge.
   if (state_ == kPlaying) {
     UMA_HISTOGRAM_BOOLEAN("Media.AudioOutputControllerPlaybackStartupSuccess",
                           base::AtomicRefCountIsOne(&on_more_io_data_called_));
   }
 }
 
 }  // namespace media