Add new class AudioBufferQueue.

media/base/seekable_audio_buffer.h

+// 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.
+
+// SeekableAudioBuffer to support forward seeking in a buffer for reading a

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

this should go by the class as opposed to top of file

yes, chrome as a whole in inconsistent but in media/ we try to be consistent :)

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

Done.

+// media data source.
+//
+// In order to support forward seeking, this class buffers data in the forward
+// direction only. The current read position can be forwarded to anywhere in the
+// buffered data.
+//
+// In the case of appending data to the buffer, there is an advisory limit of
+// how many frames can be kept, regulated by |forward_capacity|. The append
+// operation (by calling Append()) that caused frames to exceed
+// |forward_capacity| will have a return value that advises a halt of append
+// operation. Further append operations are allowed but are not advised. Since
+// this class is used as a backend buffer for caching media files downloaded
+// from network we cannot afford losing data, we can only advise a halt of
+// further writing to this buffer.
+//
+// This class is not inherently thread-safe. Concurrent access must be
+// externally serialized.
+
+#ifndef MEDIA_BASE_SEEKABLE_AUDIO_BUFFER_H_
+#define MEDIA_BASE_SEEKABLE_AUDIO_BUFFER_H_
+
+#include <list>
+
+#include "base/basictypes.h"
+#include "base/memory/scoped_ptr.h"
+#include "media/base/audio_buffer.h"

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

can we fwd decl AudioBuffer?

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

scoped_ptr (for list/deque) needs the class defined.

+#include "media/base/media_export.h"
+
+namespace media {
+
+class AudioBus;
+
+class MEDIA_EXPORT SeekableAudioBuffer {

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

I've never particularly liked the "seekable" name, and with this
AudioBuffer-specific version it's even less seekable since we can only ever move
forward!

WDYT about AudioBufferQueue?

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

Done.

+ public:
+  // Constructs an instance with |forward_capacity|. The value is in frames.
+  SeekableAudioBuffer(int forward_capacity);

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

one thing I've always wanted to consider is whether we could remove the capacity
logic from this class and have the owner (e.g., AudioRendererImpl) manage its
own capacity logic, since its the one that ultimately cares and manages capacity

then this class is simply a queue of buffers to facilitate reading across many
AudioBuffers

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

+1 if we can. AudioRendererAlgorithm, AudioRendererImpl, and SeekableBuffer all
have a sad coupling around capacity. Ideally exposing a frames() variable should
be enough for SeekableAudioBuffer, AudioRendererAlgorithm or AudioRendererImpl
can then maintain the proper watermarks.

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

Done.

+
+  ~SeekableAudioBuffer();
+
+  // Clears the buffer queue.
+  void Clear();
+
+  // Appends |buffer_in| to this buffer. Returns false if forward_bytes() is
+  // greater than or equals to forward_capacity(), true otherwise. The data is
+  // added to the buffer in any case.
+  bool Append(const scoped_refptr<AudioBuffer>& buffer_in);
+
+  // Reads a maximum of |frames| frames into |dest| from the current position.
+  // Returns the number of frames read. The current position will advance by the
+  // amount of frames read.
+  int ReadFrames(int frames, AudioBus* dest);
+
+  // Copies up to |frames| frames from current position to |dest|. Returns
+  // number of frames copied. Doesn't advance current position. Optionally
+  // starts at a |forward_offset| from current position.
+  int PeekFrames(int frames, AudioBus* dest) {
+    return PeekFrames(frames, 0, dest);
+  }
+  int PeekFrames(int frames, int forward_offset, AudioBus* dest);
+
+  // Moves the current position forward by |frames| frames. If |frames| exceeds
+  // frames available, the seek operation will fail and return value will be
+  // false. If the seek operation fails, the current position will not be
+  // updated.
+  bool SeekFrames(int frames);
+
+  // Returns the suggested maximum number of frames that should be kept in the
+  // forward direction.
+  int forward_capacity() const { return forward_capacity_; }

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

should we drop the forward prefix considering there's only one direction?

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

Removed completely.

+
+  // Sets the forward_capacity to |new_forward_capacity| frames.
+  void set_forward_capacity(int new_forward_capacity) {
+    forward_capacity_ = new_forward_capacity;
+  }
+
+  // Returns the number of frames buffered beyond the current position.
+  int forward_frames() const { return forward_frames_; }
+
+  // Returns the current timestamp, taking into account current offset. The
+  // value calculated based on the timestamp of the current buffer. If timestamp
+  // for the current buffer is set to 0, then returns value that corresponds to
+  // the last position in a buffer that had timestamp set. kNoTimestamp() is
+  // returned if no buffers we read from had timestamp set.
+  base::TimeDelta current_time() const { return current_time_; }
+
+ private:
+  // Definition of the buffer queue.
+  typedef std::list<scoped_refptr<AudioBuffer> > BufferQueue;

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

this (and the regular SeekableBuffer!) should be a deque given the typical usage
of this class (pops off the front + pushes on the back, no random
insertion/deletion in middle, tiny element size)

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

Done.

+
+  // An internal method shared by ReadFrames() and SeekFrames() that actually
+  // does reading. It reads a maximum of |frames| frames into |dest|. Returns
+  // the number of frames read. The current position will be moved forward by
+  // the number of frames read if |advance_position| is set. If |dest| is NULL,
+  // only the current position will advance but no data will be copied.
+  // |forward_offset| can be used to skip frames before reading.
+  int InternalRead(int frames,
+                   bool advance_position,
+                   int forward_offset,
+                   AudioBus* dest);
+
+  // Updates |current_time_| with the time that corresponds to the specified
+  // position in the buffer.
+  void UpdateCurrentTime(BufferQueue::iterator buffer, int offset);
+
+  BufferQueue::iterator current_buffer_;
+  BufferQueue buffers_;
+  int current_buffer_offset_;
+
+  int forward_capacity_;
+  int forward_frames_;
+
+  // Keeps track of the most recent time we've seen in case the |buffers_| is
+  // empty when our owner asks what time it is.
+  base::TimeDelta current_time_;
+
+  DISALLOW_COPY_AND_ASSIGN(SeekableAudioBuffer);

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

IMPLICIT as we don't define the default ctor

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

There is now a default ctor.

+};
+
+}  // namespace media
+
+#endif  // MEDIA_BASE_SEEKABLE_AUDIO_BUFFER_H_