Introducing DecoderBuffer and general Buffer cleanup.

media/base/decoder_buffer.h

+// Copyright (c) 2012 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.
+
+// A specialized buffer for interfacing with audio / video decoders.
+//
+// Specifically ensures that data is aligned and padded as necessary by the
+// underlying decoding framework.  On desktop platforms this means memory is
+// allocated using FFmpeg with particular alignment and padding requirements.
+//
+// Also includes decoder specific functionality for decryption.
+
+#ifndef MEDIA_BASE_DECODER_BUFFER_H_
+#define MEDIA_BASE_DECODER_BUFFER_H_
+
+#include "base/memory/scoped_ptr.h"
+#include "media/base/buffers.h"
+#include "media/base/decrypt_config.h"
+
+namespace media {
+
+class MEDIA_EXPORT DecoderBuffer : public Buffer {
+ public:
+  // Allocates buffer of size |buffer_size| >= 0.  Buffer will be padded and
+  // aligned as necessary.
+  explicit DecoderBuffer(int buffer_size);

[scherkus (not reviewing), 2012/05/30 02:48:42]

while doing a final review pass it appears that there are no real production
users of this public ctor, which makes me wonder whether it should remain public

Don't let that stop you from committing the change -- just something to think
about!

Side-bar: my dealings with Go has me on a big "ctors (especially overloaded
ones) don't produce self-documenting code!" kick.  It's sort of a contrived
case, but imagine if you were new to the code and had to compare these two
snippets of code:
  1) db = new DecoderBuffer(1024);
  2) db = DecoderBuffer::Allocate(1024);

[DaleCurtis, 2012/05/30 03:20:11]

We should keep this constructor since it's impossible to allocate a buffer and
then write data into it otherwise.  You would always need to allocate a
secondary buffer, read your data into it, and then use the CopyFrom()
constructor to copy the data into DecoderBuffer.

Neither method is any clearer than the other to me.  Without context of what a
DecoderBuffer is I don't know if it's allocating 1024 buffers, a 1024 byte
buffer, or some random other quantity.

You could start using Java-style DecoderBuffer::AllocateNewBufferInBytes(1024)
style naming, but I suspect that would only be helpful in trivial cases like
this.

[DaleCurtis, 2012/05/30 03:50:52]

Also, Ronald has a use case for this constructor coming up in
http://codereview.chromium.org/10451051/

[scherkus (not reviewing), 2012/05/30 04:32:27]

Gah I think you're reading too much into my sidebar / musings on ctors!

I even refrained from mentioning Java because I didn't want to give the
impression that I'd prefer something like DB::ANBIB()!!! (FWIW that name would
be un-Go-like as well)

On 2012/05/30 03:20:11, DaleCurtis wrote:

+
+  // Create a DecoderBuffer whose |data_| is copied from |data|.  Buffer will be
+  // padded and aligned as necessary.  |data| must not be NULL and |size| >= 0.
+  static scoped_refptr<DecoderBuffer> CopyFrom(const uint8* data, int size);
+
+  // Create a DecoderBuffer indicating we've reached end of stream.  GetData()
+  // and GetWritableData() will return NULL and GetDataSize() will return 0.
+  static scoped_refptr<DecoderBuffer> CreateEOSBuffer();
+
+  // Buffer implementation.
+  virtual const uint8* GetData() const OVERRIDE;
+  virtual int GetDataSize() const OVERRIDE;
+
+  // Returns a read-write pointer to the buffer data.
+  virtual uint8* GetWritableData();
+
+  virtual const DecryptConfig* GetDecryptConfig() const;
+  virtual void SetDecryptConfig(scoped_ptr<DecryptConfig> decrypt_config);
+
+ protected:
+  DecoderBuffer(const uint8* data, int size);
+  virtual ~DecoderBuffer();
+
+ private:
+  int buffer_size_;
+  uint8* data_;
+  scoped_ptr<DecryptConfig> decrypt_config_;
+
+  // Constructor helper method for memory allocations.
+  void Initialize();
+
+  DISALLOW_COPY_AND_ASSIGN(DecoderBuffer);
+};
+
+}  // namespace media
+
+#endif  // MEDIA_BASE_DECODER_BUFFER_H_