Support configuring the audio buffer duration in the Pepper MediaStream API.

content/renderer/pepper/pepper_media_stream_audio_track_host.cc

Index: content/renderer/pepper/pepper_media_stream_audio_track_host.cc
diff --git a/content/renderer/pepper/pepper_media_stream_audio_track_host.cc b/content/renderer/pepper/pepper_media_stream_audio_track_host.cc
index d972c2c061e2882eb1257d92fc9981e18a296ba5..aa3caf3b5d15fda78b0768cd89ec57e7a37b548e 100644
--- a/content/renderer/pepper/pepper_media_stream_audio_track_host.cc
+++ b/content/renderer/pepper/pepper_media_stream_audio_track_host.cc
@@ -26,8 +26,9 @@ using ppapi::MediaStreamAudioTrackShared;
 
 namespace {
 
-// Max audio buffer duration in milliseconds.
-const uint32_t kMaxDuration = 10;
+// Audio buffer durations in milliseconds.
+const uint32_t kMinDuration = 10;
+const uint32_t kDefaultDuration = 10;
 
 const int32_t kDefaultNumberOfBuffers = 4;
 const int32_t kMaxNumberOfBuffers = 1000;  // 10 sec
@@ -65,10 +66,14 @@ PepperMediaStreamAudioTrackHost::AudioSink::AudioSink(
     PepperMediaStreamAudioTrackHost* host)
     : host_(host),
       buffer_data_size_(0),
+      active_buffer_index_(-1),
+      active_buffer_offset_(0),
+      init_buffers_count_(0),
       main_message_loop_proxy_(base::MessageLoopProxy::current()),
       weak_factory_(this),
       number_of_buffers_(kDefaultNumberOfBuffers),
-      bytes_per_second_(0) {}
+      bytes_per_second_(0),
+      user_buffer_duration_(kDefaultDuration) {}
 
 PepperMediaStreamAudioTrackHost::AudioSink::~AudioSink() {
   DCHECK_EQ(main_message_loop_proxy_, base::MessageLoopProxy::current());
@@ -83,39 +88,63 @@ void PepperMediaStreamAudioTrackHost::AudioSink::EnqueueBuffer(int32_t index) {
 }
 
 void PepperMediaStreamAudioTrackHost::AudioSink::Configure(
-    int32_t number_of_buffers) {
+    int32_t number_of_buffers, int32_t duration) {
   DCHECK_EQ(main_message_loop_proxy_, base::MessageLoopProxy::current());
   bool changed = false;
   if (number_of_buffers != number_of_buffers_)
     changed = true;
+  if (duration != 0 && duration != user_buffer_duration_) {
+    user_buffer_duration_ = duration;
+    changed = true;
+  }
   number_of_buffers_ = number_of_buffers;
 
   // Initialize later in OnSetFormat if bytes_per_second_ is not know yet.
-  if (changed && bytes_per_second_ > 0)
+  if (changed && bytes_per_second_ > 0 && bytes_per_frame_ > 0)
     InitBuffers();
 }
 
 void PepperMediaStreamAudioTrackHost::AudioSink::SetFormatOnMainThread(
-    int bytes_per_second) {
+    int bytes_per_second, int bytes_per_frame) {
   bytes_per_second_ = bytes_per_second;
+  bytes_per_frame_ = bytes_per_frame;
   InitBuffers();
 }
 
 void PepperMediaStreamAudioTrackHost::AudioSink::InitBuffers() {
   DCHECK_EQ(main_message_loop_proxy_, base::MessageLoopProxy::current());
+  {
+    base::AutoLock lock(lock_);
+    // Clear |buffers_|, so the audio thread will drop all incoming audio data.
+    buffers_.clear();
+    init_buffers_count_++;
+  }
+  int32_t frame_rate = bytes_per_second_ / bytes_per_frame_;
+  base::CheckedNumeric<int32_t> frames_per_buffer = user_buffer_duration_;
+  frames_per_buffer *= frame_rate;
+  frames_per_buffer /= base::Time::kMillisecondsPerSecond;
+  base::CheckedNumeric<int32_t> buffer_audio_size =
+      frames_per_buffer * bytes_per_frame_;
   // The size is slightly bigger than necessary, because 8 extra bytes are
-  // added into the struct. Also see |MediaStreamBuffer|.
-  base::CheckedNumeric<int32_t> buffer_size = bytes_per_second_;
-  buffer_size *= kMaxDuration;
-  buffer_size /= base::Time::kMillisecondsPerSecond;
+  // added into the struct. Also see |MediaStreamBuffer|. Also, the size of the
+  // buffer may be larger than requested, since the size of each buffer will be
+  // 4-byte aligned.
+  base::CheckedNumeric<int32_t> buffer_size = buffer_audio_size;
   buffer_size += sizeof(ppapi::MediaStreamBuffer::Audio);
+  DCHECK_GT(buffer_size.ValueOrDie(), 0);

[dmichael (off chromium), 2014/07/29 16:41:24]

Can you use IsValid() somewhere before ValidOrDie? Since these numbers can come
from the plugin process, it would be nice to fail gracefully if possible.

[Anand Mistry (off Chromium), 2014/07/30 00:22:05]

The only number that comes from the plugin, and is used by this calculation, is
the buffer duration. And that is bounded to 10,000 by a call to
VerifyAttributes(). There'll be an overflow if somehow, our source is >2
channels of 96KHz audio. In which case, we probably have other problems.

But more generally, it would be nice if we could return errors from this, back
to the plugin. But I'd prefer to do that in a follow-up CL.

[dmichael (off chromium), 2014/07/30 23:09:22]

Okay, thanks for explaining. Fine to do in a follow up.

+
+  // We don't need hold |lock_| during |host->InitBuffers()| call, because
+  // we just cleared |buffers_| , so the audio thread will drop all incoming
+  // audio data, and not use buffers in |host_|.
   bool result = host_->InitBuffers(number_of_buffers_,
                                    buffer_size.ValueOrDie(),
                                    kRead);
   // TODO(penghuang): Send PP_ERROR_NOMEMORY to plugin.
   CHECK(result);
+
+  // Fill the |buffers_|, so the audio thread can continue receiving audio data.
   base::AutoLock lock(lock_);
-  buffers_.clear();
+  output_buffer_size_ = buffer_audio_size.ValueOrDie();
   for (int32_t i = 0; i < number_of_buffers_; ++i) {
     int32_t index = host_->buffer_manager()->DequeueBuffer();
     DCHECK_GE(index, 0);
@@ -124,9 +153,15 @@ void PepperMediaStreamAudioTrackHost::AudioSink::InitBuffers() {
 }
 
 void PepperMediaStreamAudioTrackHost::AudioSink::
-    SendEnqueueBufferMessageOnMainThread(int32_t index) {
+    SendEnqueueBufferMessageOnMainThread(int32_t index,
+                                         int32_t init_buffers_count) {
   DCHECK_EQ(main_message_loop_proxy_, base::MessageLoopProxy::current());
-  host_->SendEnqueueBufferMessageToPlugin(index);
+  // If |InitBuffers()| is called after this task being posted from the audio
+  // thread, the buffer should become invalid already. We should ignore it.  And
+  // because only the main thread modifies the |init_buffers_count_|, so we
+  // don't need lock |lock_| here (main thread).
+  if (init_buffers_count == init_buffers_count_)
+    host_->SendEnqueueBufferMessageToPlugin(index);
 }
 
 void PepperMediaStreamAudioTrackHost::AudioSink::OnData(const int16* audio_data,
@@ -138,33 +173,70 @@ void PepperMediaStreamAudioTrackHost::AudioSink::OnData(const int16* audio_data,
   DCHECK_EQ(sample_rate, audio_params_.sample_rate());
   DCHECK_EQ(number_of_channels, audio_params_.channels());
   DCHECK_EQ(number_of_frames, audio_params_.frames_per_buffer());
-  int32_t index = -1;
-  {
-    base::AutoLock lock(lock_);
-    if (!buffers_.empty()) {
-      index = buffers_.front();
+
+  const uint32_t bytes_per_frame = number_of_channels *
+      audio_params_.bits_per_sample() / 8;
+
+  int frames_remaining = number_of_frames;
+  base::TimeDelta timestamp_offset;
+
+  base::AutoLock lock(lock_);
+  while (frames_remaining) {
+    if (active_init_buffers_count_ != init_buffers_count_) {
+      // Buffers have changed, so drop the active buffer.
+      active_buffer_index_ = -1;
+    }
+    if (active_buffer_index_ == -1 && !buffers_.empty()) {
+      active_init_buffers_count_ = init_buffers_count_;
+      active_buffer_offset_ = 0;
+      active_buffer_index_ = buffers_.front();
       buffers_.pop_front();
     }
-  }
+    if (active_buffer_index_ == -1) {
+      // Eek! We're dropping frames. Bad, bad, bad!
+      break;
+    }
 
-  if (index != -1) {
     // TODO(penghuang): support re-sampling, etc.
     ppapi::MediaStreamBuffer::Audio* buffer =
-        &(host_->buffer_manager()->GetBufferPointer(index)->audio);
-    buffer->header.size = host_->buffer_manager()->buffer_size();
-    buffer->header.type = ppapi::MediaStreamBuffer::TYPE_AUDIO;
-    buffer->timestamp = timestamp_.InMillisecondsF();
-    buffer->sample_rate = static_cast<PP_AudioBuffer_SampleRate>(sample_rate);
-    buffer->number_of_channels = number_of_channels;
-    buffer->number_of_samples = number_of_channels * number_of_frames;
-    buffer->data_size = buffer_data_size_;
-    memcpy(buffer->data, audio_data, buffer_data_size_);
-
-    main_message_loop_proxy_->PostTask(
-        FROM_HERE,
-        base::Bind(&AudioSink::SendEnqueueBufferMessageOnMainThread,
-                   weak_factory_.GetWeakPtr(),
-                   index));
+        &(host_->buffer_manager()->GetBufferPointer(active_buffer_index_)
+          ->audio);
+    if (active_buffer_offset_ == 0) {
+      // The active buffer is new, so initialise the header and metadata fields.
+      buffer->header.size = host_->buffer_manager()->buffer_size();
+      buffer->header.type = ppapi::MediaStreamBuffer::TYPE_AUDIO;
+      buffer->timestamp = (timestamp_ + timestamp_offset).InMillisecondsF();
+      buffer->sample_rate = static_cast<PP_AudioBuffer_SampleRate>(sample_rate);
+      buffer->data_size = output_buffer_size_;
+      buffer->number_of_channels = number_of_channels;
+      buffer->number_of_samples = buffer->data_size / bytes_per_frame;
+    }
+    uint32_t buffer_bytes_remaining =
+        buffer->data_size - active_buffer_offset_;
+    DCHECK_EQ(buffer_bytes_remaining % bytes_per_frame, 0U);
+    uint32_t incoming_bytes_remaining = frames_remaining * bytes_per_frame;
+    uint32_t bytes_to_copy = std::min(buffer_bytes_remaining,
+                                      incoming_bytes_remaining);
+    uint32_t frames_to_copy = bytes_to_copy / bytes_per_frame;
+    DCHECK_EQ(bytes_to_copy % bytes_per_frame, 0U);
+    memcpy(buffer->data + active_buffer_offset_,
+           audio_data, bytes_to_copy);
+    active_buffer_offset_ += bytes_to_copy;
+    audio_data += bytes_to_copy / sizeof(*audio_data);
+    frames_remaining -= frames_to_copy;
+    timestamp_offset += base::TimeDelta::FromMilliseconds(
+        frames_to_copy * base::Time::kMillisecondsPerSecond / sample_rate);
+
+    DCHECK_LE(active_buffer_offset_, buffer->data_size);
+    if (active_buffer_offset_ == buffer->data_size) {
+      main_message_loop_proxy_->PostTask(
+          FROM_HERE,
+          base::Bind(&AudioSink::SendEnqueueBufferMessageOnMainThread,
+                     weak_factory_.GetWeakPtr(),
+                     active_buffer_index_,
+                     init_buffers_count_));
+      active_buffer_index_ = -1;
+    }
   }
   timestamp_ += buffer_duration_;
 }
@@ -172,7 +244,12 @@ void PepperMediaStreamAudioTrackHost::AudioSink::OnData(const int16* audio_data,
 void PepperMediaStreamAudioTrackHost::AudioSink::OnSetFormat(
     const AudioParameters& params) {
   DCHECK(params.IsValid());
-  DCHECK_LE(params.GetBufferDuration().InMilliseconds(), kMaxDuration);
+  // TODO(amistry): How do you handle the case where the user configures a
+  // duration that's shorter than the received buffer duration? One option is to
+  // double buffer, where the size of the intermediate ring buffer is at least
+  // max(user requested duration, received buffer duration). There are other
+  // ways of dealing with it, but which one is "correct"?
+  DCHECK_LE(params.GetBufferDuration().InMilliseconds(), kMinDuration);
   DCHECK_EQ(params.bits_per_sample(), 16);
   DCHECK_NE(GetPPSampleRate(params.sample_rate()),
             PP_AUDIOBUFFER_SAMPLERATE_UNKNOWN);
@@ -192,11 +269,13 @@ void PepperMediaStreamAudioTrackHost::AudioSink::OnSetFormat(
     audio_thread_checker_.DetachFromThread();
     original_audio_params_ = params;
 
+    int bytes_per_frame = params.channels() * params.bits_per_sample() / 8;
     main_message_loop_proxy_->PostTask(
         FROM_HERE,
         base::Bind(&AudioSink::SetFormatOnMainThread,
                    weak_factory_.GetWeakPtr(),
-                   params.GetBytesPerSecond()));
+                   params.GetBytesPerSecond(),
+                   bytes_per_frame));
   }
 }
 
@@ -236,7 +315,7 @@ int32_t PepperMediaStreamAudioTrackHost::OnHostMsgConfigure(
   int32_t buffers = attributes.buffers
                         ? std::min(kMaxNumberOfBuffers, attributes.buffers)
                         : kDefaultNumberOfBuffers;
-  audio_sink_.Configure(buffers);
+  audio_sink_.Configure(buffers, attributes.duration);
 
   context->reply_msg = PpapiPluginMsg_MediaStreamAudioTrack_ConfigureReply();
   return PP_OK;