Add new class AudioBufferQueue.

media/base/seekable_audio_buffer.cc

Index: media/base/seekable_audio_buffer.cc
diff --git a/media/base/seekable_audio_buffer.cc b/media/base/seekable_audio_buffer.cc
new file mode 100644
index 0000000000000000000000000000000000000000..2542ff97edc121458168aa11682fd431a13056d5
--- /dev/null
+++ b/media/base/seekable_audio_buffer.cc
@@ -0,0 +1,181 @@
+// Copyright 2013 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/base/seekable_audio_buffer.h"
+
+#include <algorithm>
+
+#include "base/logging.h"
+#include "media/base/audio_bus.h"
+#include "media/base/buffers.h"
+
+namespace media {
+
+SeekableAudioBuffer::SeekableAudioBuffer(int forward_capacity)
+    : current_buffer_offset_(0),
+      forward_capacity_(forward_capacity),
+      forward_frames_(0),
+      current_time_(kNoTimestamp()) {
+  current_buffer_ = buffers_.begin();
+}
+
+SeekableAudioBuffer::~SeekableAudioBuffer() {}
+
+void SeekableAudioBuffer::Clear() {

[scherkus (not reviewing), 2013/06/19 23:38:06]

do we ever call this method in production?

can we get away with re-allocating the object, or is that crazy talk?

if we decide with keeping it, one tiny improvement we can do in this file is to
have the ctor call Clear() instead of duplicating initialization code

[jrummell, 2013/06/20 21:47:01]

Clear() is called by alsa_output.cc, pulse_input.cc, pulse_unified.cc,  and
audio_renderer_algorithm.cc. ctor now calls Clear().

+  buffers_.clear();
+  current_buffer_ = buffers_.begin();
+  current_buffer_offset_ = 0;
+  forward_frames_ = 0;
+  current_time_ = kNoTimestamp();
+}
+
+bool SeekableAudioBuffer::Append(const scoped_refptr<AudioBuffer>& buffer_in) {
+  // Since the forward capacity is only used to check the criteria for buffer
+  // full, we always append data to the buffer.
+  buffers_.push_back(buffer_in);

[DaleCurtis, 2013/06/20 00:49:49]

Does this not invalidate the current_buffer_ iterator?

http://stackoverflow.com/questions/6438086/iterator-invalidation-rules/644282...

[jrummell, 2013/06/20 21:47:01]

For list, no. But for deque it does, so fixed.

+
+  // After we have written the first buffer, update |current_buffer_| to point
+  // to it.
+  if (current_buffer_ == buffers_.end()) {
+    DCHECK_EQ(0, forward_frames_);

[DaleCurtis, 2013/06/20 00:49:49]

no yoda conditions.

[scherkus (not reviewing), 2013/06/20 00:57:42]

I feel I should elaborate on this.

The gtest EXPECT_EQ() macro __is the only ones__ where we do yoda comparisons
since its API actually wants the expected value first so that it can print out
very pretty and well-formatted text.

All other macros ([D]CHECK_XX(), EXPECT_LT, EXPECT_NE) don't discriminate, so we
like to have them not be all yoda-y.

For reference, here's the most recent discussion we had on the topic:
http://goto.google.com/videostack-yoda

[jrummell, 2013/06/20 21:47:01]

Done. However, test code updated to have lots of yoda conditions :)

+    current_buffer_ = buffers_.begin();
+    current_buffer_offset_ = 0;
+    current_time_ = buffer_in->timestamp();
+  }
+
+  // Update the |forward_frames_| counter since we have added frames.
+  forward_frames_ += buffer_in->frame_count();
+
+  // Advise the user to stop append if the amount of forward frames exceeds the
+  // forward capacity. A false return value means the user should stop appending
+  // more data to this buffer.
+  return forward_frames_ < forward_capacity_;
+}
+
+int SeekableAudioBuffer::ReadFrames(int frames, AudioBus* dest) {
+  DCHECK(dest);

[scherkus (not reviewing), 2013/06/19 23:38:06]

don't need to DCHECK pointers we immediately dereference

here + rest of file

[jrummell, 2013/06/20 21:47:01]

Done.

+  DCHECK_GE(dest->frames(), frames);
+  return InternalRead(frames, true, 0, dest);
+}
+
+int SeekableAudioBuffer::PeekFrames(int frames,
+                                    int forward_offset,
+                                    AudioBus* dest) {
+  DCHECK(dest);
+  DCHECK_GE(dest->frames(), frames);
+  return InternalRead(frames, false, forward_offset, dest);
+}
+
+bool SeekableAudioBuffer::SeekFrames(int frames) {
+  // Perform seeking forward only if we have enough bytes in the queue.
+  if (frames > forward_frames_)

[scherkus (not reviewing), 2013/06/19 23:38:06]

think this can be a CHECK()?

I like APIs that fail horribly if you do something stupid. It's too easy to not
check the return value of a function returning a bool :)

[jrummell, 2013/06/20 21:47:01]

Done.

+    return false;
+
+  // Do a read of |frames| frames.
+  int taken = InternalRead(frames, true, 0, NULL);
+  DCHECK_EQ(taken, frames);
+  return true;
+}
+
+int SeekableAudioBuffer::InternalRead(int frames,
+                                      bool advance_position,
+                                      int forward_offset,
+                                      AudioBus* dest) {
+  // Counts how many frames are actually read from the buffer queue.
+  int taken = 0;
+  BufferQueue::iterator current_buffer = current_buffer_;
+  int current_buffer_offset = current_buffer_offset_;
+
+  int frames_to_skip = forward_offset;
+  while (taken < frames) {
+    // |current_buffer| is valid since the first time this buffer is appended
+    // with data. Make sure there is data to be processed.
+    if (current_buffer == buffers_.end())
+      break;
+
+    scoped_refptr<AudioBuffer> buffer = *current_buffer;
+
+    int remaining_frames_in_buffer =
+        buffer->frame_count() - current_buffer_offset;
+
+    if (frames_to_skip > 0) {
+      // If there are frames to skip, do it first. May need to skip into
+      // subsequent buffers.
+      int skipped = std::min(remaining_frames_in_buffer, frames_to_skip);
+      current_buffer_offset += skipped;
+      frames_to_skip -= skipped;
+    } else {
+      // Find the right amount to copy from the current buffer. We shall copy no
+      // more than |frames| frames in total and each single step copies no more
+      // than the current buffer size.
+      int copied = std::min(frames - taken, remaining_frames_in_buffer);
+
+      // if |dest| is NULL, there's no need to copy.
+      if (dest)
+        buffer->ReadFrames(copied, current_buffer_offset, taken, dest);
+
+      // Increase total number of frames copied, which regulates when to end
+      // this loop.
+      taken += copied;
+
+      // We have read |copied| frames from the current buffer. Advance the
+      // offset.
+      current_buffer_offset += copied;
+    }
+
+    // Has the buffer has been consumed?
+    if (current_buffer_offset == buffer->frame_count()) {
+      if (advance_position) {
+        // Next buffer may not have timestamp, so we need to update current
+        // timestamp before switching to the next buffer.
+        UpdateCurrentTime(current_buffer, current_buffer_offset);
+      }
+
+      BufferQueue::iterator next = current_buffer;
+      ++next;
+      // If we are at the last buffer, no more data to be copied, so stop.
+      if (next == buffers_.end())
+        break;
+
+      // Advances the iterator.
+      current_buffer = next;
+      current_buffer_offset = 0;
+    }
+  }
+
+  if (advance_position) {
+    // Update the appropriate values since |taken| frames have been copied out.
+    forward_frames_ -= taken;
+    DCHECK_GE(forward_frames_, 0);
+    DCHECK(current_buffer_ != buffers_.end() || forward_frames_ == 0);
+
+    current_buffer_ = current_buffer;
+    current_buffer_offset_ = current_buffer_offset;
+
+    UpdateCurrentTime(current_buffer_, current_buffer_offset_);
+
+    // Remove any buffers before the current pointer as there is no going
+    // backwards.
+    while (true) {

[DaleCurtis, 2013/06/20 00:49:49]

buffers_.erase(buffers_.begin(), current_buffer_)?

Not sure if that will result in current_buffer_ being invalidated.

[jrummell, 2013/06/20 21:47:01]

Done.

+      BufferQueue::iterator i = buffers_.begin();
+      if (i == current_buffer_)
+        break;
+      buffers_.erase(i);
+    }
+  }
+
+  return taken;
+}
+
+void SeekableAudioBuffer::UpdateCurrentTime(BufferQueue::iterator buffer,
+                                            int offset) {
+  if (buffer != buffers_.end() && (*buffer)->timestamp() != kNoTimestamp()) {
+    int64 time_offset = ((*buffer)->duration().InMicroseconds() * offset) /

[DaleCurtis, 2013/06/20 00:49:49]

Use InMicrosecondsF() and round up by adding 0.5?

[jrummell, 2013/06/20 21:47:01]

There is no InMicrosecondsF(), since TimeDelta is stored as a microseconds
int64. Converted to a float operation to get the rounding.

+                        (*buffer)->frame_count();
+    current_time_ = (*buffer)->timestamp() +
+                    base::TimeDelta::FromMicroseconds(time_offset);
+  }
+}
+
+}  // namespace media