Interfaces for encoded video sources

media/base/encoded_bitstream_buffer.cc

Index: media/base/encoded_bitstream_buffer.cc
diff --git a/media/base/encoded_bitstream_buffer.cc b/media/base/encoded_bitstream_buffer.cc
new file mode 100644
index 0000000000000000000000000000000000000000..8d7d98694a25b201b97738109f660e594c69bcbc
--- /dev/null
+++ b/media/base/encoded_bitstream_buffer.cc
@@ -0,0 +1,133 @@
+// Copyright (c) 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/encoded_bitstream_buffer.h"
+
+#include <iomanip>
+
+#include "base/logging.h"
+#include "base/md5.h"
+
+namespace media {
+
+EncodedBitstreamBuffer::EncodedBitstreamBuffer(size_t size,
+                                               base::Time timestamp,
+                                               int temporal_layer)
+    : buffer_(new uint8[size]),
+      size_(size),
+      timestamp_(timestamp),
+      key_frame_(false),
+      temporal_layer_(temporal_layer),
+      altref_frame_(false),
+      golden_frame_(false),
+      droppable_(false),
+      layer_sync_(false) {
+}
+
+EncodedBitstreamBuffer::EncodedBitstreamBuffer(uint8* buffer,

[tommi (sloooow) - chröme, 2013/03/01 16:03:43]

const uint8* buffer?

Actually, would it make sense to transfer ownership of an existing buffer here
instead of making a new copy?

[Ville-Mikko Rautio, 2013/03/04 14:02:25]

In the first instance of using this class the buffer will be buffer allocated by
the Browser process and this class is to be used in the Renderer process as
well. Thus, the ownership transfer would not be as straightforward as one could
think in the first place. I think constructor doing ownership transfer cannot be
at least the only constructor for the class.

Constness done.

+                                               size_t size,
+                                               base::Time timestamp,
+                                               int temporal_layer)
+    : buffer_(new uint8[size]),
+      size_(size),
+      timestamp_(timestamp),
+      key_frame_(false),
+      temporal_layer_(temporal_layer),
+      altref_frame_(false),
+      golden_frame_(false) {

[Ami GONE FROM CHROMIUM, 2013/03/02 10:47:16]

what about droppable_/layer_sync_?

This is why we don't write copy/pasta like this :)
If you have duplicated code, extract it into an Init() function.
(though, if you take my suggestion and take a metadata struct, this becomes
moot).

[Ville-Mikko Rautio, 2013/03/04 14:02:25]

Going for the Metadata.

+  memcpy(buffer_.get(), buffer, size);

[Ami GONE FROM CHROMIUM, 2013/03/02 10:47:16]

This is crazy, considering the IPC system just memcpy'd the buffer, too ;)
Why pass a SharedMemoryHandle in and avoid *both* copies?

[Ville-Mikko Rautio, 2013/03/14 13:19:20]

I'll add TODO as this requires coordination with posciak@ on the browser side of
things.

+}
+
+EncodedBitstreamBuffer::~EncodedBitstreamBuffer() {
+}
+
+uint8* EncodedBitstreamBuffer::buffer() const {
+  return buffer_.get();
+}
+
+size_t EncodedBitstreamBuffer::size() const {
+  return size_;
+}
+
+base::Time EncodedBitstreamBuffer::timestamp() const {
+  return timestamp_;
+}
+
+int EncodedBitstreamBuffer::temporal_layer() const {
+  return temporal_layer_;
+}
+
+bool EncodedBitstreamBuffer::key_frame() const {
+  return key_frame_;
+}
+
+bool EncodedBitstreamBuffer::altref_frame() const {
+  return altref_frame_;
+}
+
+bool EncodedBitstreamBuffer::golden_frame() const {
+  return golden_frame_;
+}
+
+bool EncodedBitstreamBuffer::droppable() const {
+  return droppable_;
+}
+
+bool EncodedBitstreamBuffer::layer_sync() const {
+  return layer_sync_;
+}
+
+void EncodedBitstreamBuffer::MarkKeyFrame() {
+  key_frame_ = true;
+  altref_frame_ = true;
+  golden_frame_ = true;

[Ami GONE FROM CHROMIUM, 2013/03/02 10:47:16]

Is this really how VP8 bitstreams work?

[Ville-Mikko Rautio, 2013/03/04 14:02:25]

Yes.

+}
+
+void EncodedBitstreamBuffer::MarkAltRef() {
+  altref_frame_ = true;
+}
+
+void EncodedBitstreamBuffer::MarkGolden() {
+  golden_frame_ = true;
+}
+
+void EncodedBitstreamBuffer::MarkDroppable() {
+  droppable_ = true;
+}
+
+void EncodedBitstreamBuffer::MarkLayerSync() {
+  layer_sync_ = true;
+}
+
+}  // namespace media
+
+std::ostream& operator<<(std::ostream& output,
+                         const media::EncodedBitstreamBuffer& b) {
+  base::MD5Digest digest;

[tommi (sloooow) - chröme, 2013/03/01 16:03:43]

this looks like debug only code, right?  Can we exclude it from release builds?

[Ville-Mikko Rautio, 2013/03/04 14:02:25]

See the comment on the header.

+  output << "[";
+  if (b.key_frame()) {
+    output << "I";
+  } else {
+    if (b.golden_frame())
+      output << "G";
+    if (b.altref_frame())
+      output << "A";
+    if (b.droppable())
+      output << "-";
+    if (!b.golden_frame() && !b.altref_frame() && !b.droppable())
+      output << "P";
+  }
+  output << "]";
+  output << " tl: [" << b.temporal_layer();
+  if (b.layer_sync())
+    output << "S";
+  output << "]";
+  output << " time: " << std::setprecision(2) << std::fixed
+      << b.timestamp().ToDoubleT();
+  output << " (" << b.size() << " bytes)";
+  base::MD5Sum(b.buffer(), b.size(), &digest);
+  output << " md5: " << base::MD5DigestToBase16(digest);
+  return output;
+}
+