Fix start/stop of html5 audio stream and race condition when pausing.

content/renderer/media/audio_device.cc

 // Copyright (c) 2011 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 "content/renderer/media/audio_device.h"
 
 #include "base/bind.h"
 #include "base/debug/trace_event.h"
 #include "base/message_loop.h"
 #include "base/time.h"
@@ skipping 269 matching lines @@
 
 void AudioDevice::Send(IPC::Message* message) {
   filter_->Send(message);
 }
 
 // Our audio thread runs here.
 void AudioDevice::Run() {
   audio_thread_->SetThreadPriority(base::kThreadPriority_RealtimeAudio);
 
   base::SharedMemory shared_memory(shared_memory_handle_, false);
-  shared_memory.Map(memory_length_);
-  // Allow the client to pre-populate the buffer.
-  FireRenderCallback(reinterpret_cast<int16*>(shared_memory.memory()));
-
+  shared_memory.Map(media::TotalSharedMemorySizeInBytes(memory_length_));
   base::SyncSocket socket(socket_handle_);

[no longer working on chromium, 2011/12/16 12:21:13]

I am actually thinking about the opposite solution.
In the current AudioOutputController::DoPlay() we ask for first packet by
sync_reader_->UpdatePendingBytes(0); then we wait 3ms for the packet to be
ready, which means we will have at least 3ms delay in start playing.
I don't think it is a good approach, 3ms is not much but I don't see why we give
it up here.
An easy fix is that we still do the pre-buffer here, and in DoPlay() we check if
sync_reader_->DataReady(), if it is, start the stream immediately, otherwise
wait for 3ms.

And inside the loop, when pending_data ==
media::AudioOutputController::kPauseMark, we should call
media::SetUnknownDataSize(shared_memory_, memory_length_);

[enal1, 2011/12/16 16:48:23]

I believe I answered once. Pre-buffering does not work for <audio>, because we
can play the same stream again and again, so Play() does not know if it is the
first DoPlay() ever called -- in this case there should be pre-buffered data --
or if it is resuming after pause, and in this case there is no pre-buffered
data. Of course I can set up flag in AudioOutputController, etc., but current
code is much cleaner. If those 3ms ever become an issue we can use polling loop
with yield() there.

Pre-buffering can hurt when stopping <audio> as well. We are stopping audio
streams using "earliest ending time", and in case of very short stream it would
be "time at the moment of pre-buffering", not "time when we started playback".
It is bad right now, we are losing several last milliseconds, as "time now" is
time for 1st buffer, and we would call OnMoreData() up to 2 more times before
starting playback, with wait if data is not ready. My pending CL should improve
things a lot, but pre-buffering would hurt again.

And we'll change to software mixing soon, and have to modify that code anyways.

 
   int pending_data;
   const int samples_per_ms = static_cast<int>(sample_rate_) / 1000;
   const int bytes_per_ms = channels_ * (bits_per_sample_ / 8) * samples_per_ms;
 
   while (sizeof(pending_data) ==
       socket.Receive(&pending_data, sizeof(pending_data))) {
     if (pending_data == media::AudioOutputController::kPauseMark) {
       memset(shared_memory.memory(), 0, memory_length_);
+      media::SetActualDataSizeInBytes(&shared_memory, memory_length_, 0);
       continue;
     } else if (pending_data < 0) {
       break;
     }
     // Convert the number of pending bytes in the render buffer
     // into milliseconds.
     audio_delay_milliseconds_ = pending_data / bytes_per_ms;
-    FireRenderCallback(reinterpret_cast<int16*>(shared_memory.memory()));
+    size_t num_frames = FireRenderCallback(
+        reinterpret_cast<int16*>(shared_memory.memory()));
+    // Let the host know we are done.

[henrika (OOO until Aug 14), 2011/12/16 10:52:17]

Shouldn't comments have an empty line above when they are not first in scope?

[enal1, 2011/12/16 16:48:23]

Done.

+    media::SetActualDataSizeInBytes(&shared_memory,
+                                    memory_length_,
+                                    num_frames * channels_ * sizeof(int16));
   }
 }
 
-void AudioDevice::FireRenderCallback(int16* data) {
+size_t AudioDevice::FireRenderCallback(int16* data) {
   TRACE_EVENT0("audio", "AudioDevice::FireRenderCallback");
 
+  size_t num_frames = 0;
   if (callback_) {
     // Update the audio-delay measurement then ask client to render audio.
-    callback_->Render(audio_data_, buffer_size_, audio_delay_milliseconds_);
+    num_frames = callback_->Render(audio_data_,
+                                   buffer_size_,
+                                   audio_delay_milliseconds_);
 
     // Interleave, scale, and clip to int16.
     // TODO(crogers): avoid converting to integer here, and pass the data
     // to the browser process as float, so we don't lose precision for
     // audio hardware which has better than 16bit precision.
     media::InterleaveFloatToInt16(audio_data_,
                                   data,
                                   buffer_size_);
   }
+  return num_frames;
 }
 
 void AudioDevice::ShutDownAudioThread() {
   // Synchronize with OnLowLatencyCreated().
   base::AutoLock auto_lock(lock_);
   if (audio_thread_.get()) {
     // Close the socket handler to terminate the main thread function in the
     // audio thread.
     {
       base::SyncSocket socket(socket_handle_);
     }
     audio_thread_->Join();
     audio_thread_.reset(NULL);
   }
 }