Implementation of MediaCodecPlayer stage 1

media/base/android/access_unit_queue.cc

Index: media/base/android/access_unit_queue.cc
diff --git a/media/base/android/access_unit_queue.cc b/media/base/android/access_unit_queue.cc
new file mode 100644
index 0000000000000000000000000000000000000000..21022a4050086c7f90e9fe1f0487e48474a90ad5
--- /dev/null
+++ b/media/base/android/access_unit_queue.cc
@@ -0,0 +1,101 @@
+// Copyright 2015 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/android/access_unit_queue.h"
+
+#include "base/logging.h"
+#include "media/base/demuxer_stream.h"
+
+namespace media {
+
+AccessUnitQueue::AccessUnitQueue() : has_eos_(false) {}
+
+AccessUnitQueue::~AccessUnitQueue() {}
+
+void AccessUnitQueue::PushBack(const DemuxerData& data) {
+  // TODO(timav): The lock is expensive here because it lasts
+  // for the whole duration of data copying. We need to figure out
+  // whet to do with this.

[wolenetz, 2015/05/19 21:58:37]

nit: s/whet/what/

[Tima Vaisburd, 2015/05/22 22:48:54]

Done.

+
+  // Media thread

[wolenetz, 2015/05/19 21:58:37]

nit: here and elsewhere, might be good to have DCHECKs in place to enforce the
method is called on the expected thread.

[Tima Vaisburd, 2015/05/22 22:48:54]

I tried to do it everywhere in higher-level classes: MediaCodecPlayer,
MediaCodecDecoder and derivatives. This queue is a part of MediaCodecDecoder and
the methods are called from there. I made an attempt to have these DCHECKs here,
too, but it did not look good. I think the checks in decoder classes are enough.

+  base::AutoLock lock(lock_);
+

[wolenetz, 2015/05/19 21:58:37]

nit: should probably do some DCHECK sanity here like:
DCHECK(!has_eos_);
and maybe something like DCHECK(unit.data.access_units.length > 0).

[Tima Vaisburd, 2015/05/22 22:48:54]

Done.

+  for (const AccessUnit& unit : data.access_units) {
+    access_units_.push_back(unit);
+    if (unit.is_end_of_stream) {
+      DVLOG(1) << "AccessUnitQueue::" << __FUNCTION__ << ": EOS detected";
+      has_eos_ = true;
+    }
+  }
+
+  // We assume that the rule "one DemuxerConfigs for one AU
+  // with |kConfigChanged|" holds and do not check anything.
+  for (const DemuxerConfigs& conf : data.demuxer_configs)

[wolenetz, 2015/05/19 21:58:36]

Hmm. Seems simple enough to DCHECK(data.demuxer_configs.length <= 1), or even
better, have a local bool that is set true in the loop in l.24 if a unit is
kConfigChanged, and then adjust the DCHECK here based on that bool. In general,
protecting borders, especially ones on each side of IPC, SGTM.

[Tima Vaisburd, 2015/05/22 22:48:54]

Done.

+    demuxer_configs_.push_back(conf);
+}
+
+void AccessUnitQueue::PopFront() {
+  // Decoder thread
+  base::AutoLock lock(lock_);
+
+  if (access_units_.front().status == DemuxerStream::kConfigChanged)
+    demuxer_configs_.pop_front();
+
+  if (access_units_.front().is_end_of_stream)
+    has_eos_ = false; // assume there os only one AU with EOS

[wolenetz, 2015/05/19 21:58:37]

hmm. Once EOS has been consumed from the queue, does the queue really no longer
care whether or not it previously reached EOS?

[Tima Vaisburd, 2015/05/22 22:48:54]

The idea was to self-clean after the last access unit pops. I changed to
  if (access_units_.empty()) has_eos_ = false;

+
+  access_units_.pop_front();
+}
+
+void AccessUnitQueue::Flush() {
+  // Media thread
+  base::AutoLock lock(lock_);
+
+  access_units_.clear();
+  demuxer_configs_.clear();
+  has_eos_ = false;
+}
+
+bool AccessUnitQueue::SkipToKeyFrame() {
+  // Media thread
+  base::AutoLock lock(lock_);
+  // We assume there cannot be |demuxer_configs_| elements for non-key frames.
+
+  std::deque<AccessUnit>::iterator it;
+  for (it = access_units_.begin(); it!= access_units_.end(); ++it) {
+    if (it->is_key_frame) {
+      access_units_.erase(access_units_.begin(), it);
+      return true;
+    }
+  }
+
+  return false;
+}
+
+void AccessUnitQueue::GetInfo(Info* info) const {

[wolenetz, 2015/05/19 21:58:37]

nit: do we need this Peek-like method, or can PopFront return the necessary
information?

[Tima Vaisburd, 2015/05/22 22:48:54]

I think we do. The Android MediaCodec might return "try again later" when asked
for input buffer, so we need to keep AU around. We could pop it from the queue,
but could not remove it and would need to put aside, and remove after a
successful MediaCodec.queueInputBuffer() only. On the other hand, as far as I
know, after the MediaCodec returned an input buffer, it is impossible to cancel:
I must provide a valid encoded frame. This fact determines the order: first
check the AU queue for a frame, then ask MediaDecoder for a buffer.

+  // Media thread, Decoder thread
+  base::AutoLock lock(lock_);
+
+  info->length = access_units_.size();
+  info->has_eos = has_eos_;
+  info->front_unit = nullptr;
+  info->configs = nullptr;
+
+  if (info->length > 0) {
+    info->front_unit = &access_units_.front();
+    if (access_units_.front().status == DemuxerStream::kConfigChanged)
+      info->configs = &demuxer_configs_.front();

[wolenetz, 2015/05/19 21:58:36]

Sanity check that we have a config to return, please.

[Tima Vaisburd, 2015/05/22 22:48:54]

Done.

+  }
+}
+
+}  // namespace media
+
+std::ostream& operator<<(std::ostream& os, const media::AccessUnit& au) {
+  os << "status:" << au.status
+     << (au.is_end_of_stream ? " EOS" : "")
+     << (au.is_key_frame ? " KEY_FRAME" : "")
+     << " pts:" << au.timestamp
+     << " size:" << au.data.size();
+  return os;
+}