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 c1edfddeb4eda4c772a5cb059bd4116b17a86669..3069988773563454c958d8eb78e19e2d6786a1c6 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,11 +66,15 @@ PepperMediaStreamAudioTrackHost::AudioSink::AudioSink(
     PepperMediaStreamAudioTrackHost* host)
     : host_(host),
       buffer_data_size_(0),
+      active_buffer_index_(-1),
+      active_buffers_generation_(0),
+      active_buffer_offset_(0),
       buffers_generation_(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());
@@ -84,21 +89,26 @@ 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();
 }
 
@@ -110,12 +120,19 @@ void PepperMediaStreamAudioTrackHost::AudioSink::InitBuffers() {
     buffers_.clear();
     buffers_generation_++;
   }
+  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);
 
   // We don't need to hold |lock_| during |host->InitBuffers()| call, because
   // we just cleared |buffers_| , so the audio thread will drop all incoming
@@ -128,6 +145,7 @@ void PepperMediaStreamAudioTrackHost::AudioSink::InitBuffers() {
 
   // Fill the |buffers_|, so the audio thread can continue receiving audio data.
   base::AutoLock lock(lock_);
+  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);
@@ -155,30 +173,75 @@ void PepperMediaStreamAudioTrackHost::AudioSink::OnData(const int16* audio_data,
   DCHECK(audio_data);
   DCHECK_EQ(sample_rate, audio_params_.sample_rate());
   DCHECK_EQ(number_of_channels, audio_params_.channels());
+  // Here, |number_of_frames| and |audio_params_.frames_per_buffer()| refer to
+  // the incomming audio buffer. However, this doesn't necessarily equal
+  // |buffer->number_of_samples|, which is configured by the user when they set
+  // buffer duration.
   DCHECK_EQ(number_of_frames, audio_params_.frames_per_buffer());
 
+  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_);
-  if (!buffers_.empty()) {
-    int index = buffers_.front();
-    buffers_.pop_front();
+  while (frames_remaining) {
+    if (active_buffers_generation_ != buffers_generation_) {
+      // Buffers have changed, so drop the active buffer.
+      active_buffer_index_ = -1;
+    }
+    if (active_buffer_index_ == -1 && !buffers_.empty()) {
+      active_buffers_generation_ = buffers_generation_;
+      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;
+    }
+
     // 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,
-                   buffers_generation_));
+        &(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;

[thembrown, 2014/08/15 15:01:23]

Shouldn't this read:
buffer->number_of_samples = buffer->data_size * number_of_channels /
bytes_per_frame;

instead? Now number of samples is set to the number of frames, while in the old
code it was set to number_of_frames * number_of_channels

[Anand Mistry (off Chromium), 2014/08/18 01:15:14]

Oops. I'll fix it right away.

+    }
+    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_,
+                     buffers_generation_));
+      active_buffer_index_ = -1;
+    }
   }
   timestamp_ += buffer_duration_;
 }
@@ -186,7 +249,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);
@@ -206,11 +274,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));
   }
 }
 
@@ -250,7 +320,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;