Separate MediaCodecLoop from MediaCodecAudioDecoder

media/base/android/media_codec_loop.cc

Index: media/base/android/media_codec_loop.cc
diff --git a/media/base/android/media_codec_loop.cc b/media/base/android/media_codec_loop.cc
new file mode 100644
index 0000000000000000000000000000000000000000..f6dc35ae3edcaf1e835bec947b192a1299320a11
--- /dev/null
+++ b/media/base/android/media_codec_loop.cc
@@ -0,0 +1,398 @@
+// Copyright 2016 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/media_codec_loop.h"
+
+#include "base/android/build_info.h"
+#include "base/bind.h"
+#include "base/callback_helpers.h"
+#include "base/logging.h"
+#include "base/threading/thread_task_runner_handle.h"
+#include "media/base/android/sdk_media_codec_bridge.h"
+#include "media/base/audio_buffer.h"
+#include "media/base/audio_timestamp_helper.h"
+#include "media/base/bind_to_current_loop.h"
+#include "media/base/timestamp_constants.h"
+
+namespace media {
+
+constexpr base::TimeDelta kDecodePollDelay =
+    base::TimeDelta::FromMilliseconds(10);
+constexpr base::TimeDelta kNoWaitTimeout = base::TimeDelta::FromMicroseconds(0);
+constexpr base::TimeDelta kIdleTimerTimeout = base::TimeDelta::FromSeconds(1);
+
+MediaCodecLoop::InputData::InputData()
+    : memory(nullptr), length(0), is_eos(false), is_encrypted(false) {}
+
+MediaCodecLoop::InputData::InputData(const InputData& other)
+    : memory(other.memory),
+      length(other.length),
+      key_id(other.key_id),
+      iv(other.iv),
+      subsamples(other.subsamples),
+      presentation_time(other.presentation_time),
+      completion_cb(other.completion_cb),
+      is_eos(other.is_eos),
+      is_encrypted(other.is_encrypted) {}
+
+MediaCodecLoop::InputData::~InputData() {}
+
+MediaCodecLoop::MediaCodecLoop(Client* client,
+                               std::unique_ptr<MediaCodecBridge>&& media_codec)
+    : state_(STATE_READY),
+      client_(client),
+      media_codec_(std::move(media_codec)),
+      pending_input_buf_index_(kInvalidBufferIndex),
+      weak_factory_(this) {
+  DVLOG(1) << __FUNCTION__;

[DaleCurtis, 2016/06/13 23:16:13]

I typically drop these from production code, but up to you.

[liberato (no reviews please), 2016/06/14 17:26:44]

Done.

+  DCHECK(media_codec_);
+}
+
+MediaCodecLoop::~MediaCodecLoop() {
+  DVLOG(1) << __FUNCTION__;
+}
+
+void MediaCodecLoop::OnKeyAdded() {
+  DVLOG(1) << __FUNCTION__;
+
+  if (state_ == STATE_WAITING_FOR_KEY)
+    SetState(STATE_READY);
+
+  DoIOTask();
+}
+
+bool MediaCodecLoop::TryFlush() {
+  DVLOG(1) << __FUNCTION__;
+
+  // We do not clear the input queue here.  It depends on the caller.
+  // For decoder reset, then it is appropriate.  Otherwise, the requests might
+  // simply be sent to us later, such as on a format change.
+
+  // Flush if we can, otherwise completely recreate and reconfigure the codec.
+  // Prior to JellyBean-MR2, flush() had several bugs (b/8125974, b/8347958) so
+  // we have to completely destroy and recreate the codec there.
+  bool success = false;

[DaleCurtis, 2016/06/13 23:16:13]

Reorder this so that we early return instead of using bool success. I also
wonder if we should have a static bool codec_reset_requires_destruction() {
return base::android::BuildInfo::GetInstance()->sdk_int() < 18; } helper method
for clarity.

[liberato (no reviews please), 2016/06/14 17:26:44]

Done, also renamed 'flush' pending rebase onto watk's recent MediaCodecBridge
s/Reset/Flush/

+  if (state_ != STATE_ERROR && state_ != STATE_DRAINED &&
+      base::android::BuildInfo::GetInstance()->sdk_int() >= 18) {
+    io_timer_.Stop();
+
+    // media_codec_->Reset() calls MediaCodec.flush().
+    success = media_codec_->Reset() == MEDIA_CODEC_OK;
+
+    // Transition to the error state if the flush failed.
+    SetState(success ? STATE_READY : STATE_ERROR);
+  }
+
+  return success;
+}
+
+void MediaCodecLoop::DoIOTask() {
+  if (state_ == STATE_ERROR)
+    return;
+
+  bool did_work = false, did_input = false, did_output = false;
+  do {
+    did_input = ProcessOneInputBuffer();
+    did_output = ProcessOneOutputBuffer();
+    if (did_input || did_output)
+      did_work = true;
+  } while (did_input || did_output);
+
+  // TODO(liberato): add "start_timer" for AVDA.
+  ManageTimer(did_work);
+}
+
+bool MediaCodecLoop::ProcessOneInputBuffer() {
+  DVLOG(2) << __FUNCTION__;
+
+  // We can only queue a buffer if there is input from the client, or if we
+  // tried previously but had to wait for a key.  In the latter case, MediaCodec
+  // already has the data.
+  if (pending_input_buf_index_ == kInvalidBufferIndex &&

[DaleCurtis, 2016/06/13 23:16:13]

A single pending_input_buf_index_ isn't going to work for video right? We queue
multiple inputs at once where as audio only allowed one to be queued at a time.

[liberato (no reviews please), 2016/06/14 17:26:44]

this is only if we're waiting for a key, which limits us to one buffer.  the
same logic is in avda:

https://cs.chromium.org/chromium/src/media/gpu/android_video_decode_accelerat...

+      !client_->IsAnyInputPending()) {
+    return false;
+  }
+
+  if (state_ == STATE_WAITING_FOR_KEY || state_ == STATE_DRAINING ||
+      state_ == STATE_DRAINED) {

[DaleCurtis, 2016/06/13 23:16:13]

STATE_ERROR ?

[liberato (no reviews please), 2016/06/14 17:26:44]

Done.

+    return false;
+  }
+
+  // DequeueInputBuffer() may set STATE_ERROR.
+  InputBuffer input_buffer = DequeueInputBuffer();

[DaleCurtis, 2016/06/13 23:16:13]

Instead of return by copy should this take a InputBuffer* and return a
true/false for error? That allows us to avoid mixing kInvalidBufferIndex
everywhere as our error signal.

[liberato (no reviews please), 2016/06/14 17:26:44]

this sounded like a good idea, but i un-did it after trying it out.

the problem is that "try again later" isn't an error, but we want to do the same
thing as the error cases.  we really do want "if we have no input buffer".

otherwise, it turns into

if (Dequeue...())
  return false;  // error
if (input_buffer.input_buf_index == kInvalidBufferIndex)
  return false;  // no work to do

i tried the middle ground of "return buf_index != kInvalidBufferIndex", but it
didn't seem to add any clarity.  probably made the call less clear.  plus, i had
to remove the InputBuffer consts to allow the assignments that were okay during
initialization.

so, i just put it back this way.

+
+  if (input_buffer.index == kInvalidBufferIndex)
+    return false;
+
+  // EnqueueInputBuffer() may set STATE_DRAINING, STATE_WAITING_FOR_KEY or
+  // STATE_ERROR.
+  EnqueueInputBuffer(input_buffer);
+  return state_ == STATE_READY;
+}
+
+MediaCodecLoop::InputBuffer MediaCodecLoop::DequeueInputBuffer() {
+  DVLOG(2) << __FUNCTION__;
+
+  // Do not dequeue a new input buffer if we failed with MEDIA_CODEC_NO_KEY.
+  // That status does not return the input buffer back to the pool of
+  // available input buffers. We have to reuse it in QueueSecureInputBuffer().
+  if (pending_input_buf_index_ != kInvalidBufferIndex) {
+    InputBuffer result(pending_input_buf_index_, true);
+    pending_input_buf_index_ = kInvalidBufferIndex;
+    return result;
+  }
+
+  int input_buf_index = kInvalidBufferIndex;
+
+  media::MediaCodecStatus status =
+      media_codec_->DequeueInputBuffer(kNoWaitTimeout, &input_buf_index);
+  switch (status) {
+    case media::MEDIA_CODEC_DEQUEUE_INPUT_AGAIN_LATER:
+      DVLOG(2) << __FUNCTION__ << ": MEDIA_CODEC_DEQUEUE_INPUT_AGAIN_LATER";
+      break;
+
+    case media::MEDIA_CODEC_ERROR:
+      DVLOG(1) << __FUNCTION__ << ": MEDIA_CODEC_ERROR from DequeInputBuffer";

[DaleCurtis, 2016/06/13 23:16:13]

DLOG(ERROR) ?

[liberato (no reviews please), 2016/06/14 17:26:44]

Done.

+      SetState(STATE_ERROR);
+      break;
+
+    case media::MEDIA_CODEC_OK:
+      break;
+
+    default:
+      NOTREACHED() << "Unknown DequeueInputBuffer status " << status;
+      SetState(STATE_ERROR);
+      break;
+  }
+
+  return InputBuffer(input_buf_index, false);
+}
+
+void MediaCodecLoop::EnqueueInputBuffer(const InputBuffer& input_buffer) {
+  DVLOG(2) << __FUNCTION__ << ": index:" << input_buffer.index;
+
+  DCHECK_NE(input_buffer.index, kInvalidBufferIndex);
+
+  InputData input_data;
+

[DaleCurtis, 2016/06/13 23:16:13]

Remove line so it's clear input_data goes with conditional.

[liberato (no reviews please), 2016/06/14 17:26:45]

Done.

+  if (input_buffer.is_pending) {
+    // A pending buffer is already filled with data, no need to copy it again.
+    DVLOG(2) << __FUNCTION__ << ": QueueSecureInputBuffer (pending):"
+             << " index:" << input_buffer.index
+             << " pts:" << pending_input_buf_data_.presentation_time
+             << " size:" << pending_input_buf_data_.length;
+
+    input_data = pending_input_buf_data_;
+  } else {
+    input_data = client_->ProvideInputData();
+  }
+
+  // Process this buffer.

[DaleCurtis, 2016/06/13 23:16:13]

Not really useful.

[liberato (no reviews please), 2016/06/14 17:26:44]

Done.

+
+  if (input_data.is_eos) {
+    media_codec_->QueueEOS(input_buffer.index);
+    SetState(STATE_DRAINING);
+
+    // For EOS, the completion callback is called when the EOS arrives at the
+    // output queue.
+    pending_eos_completion_cb_ = input_data.completion_cb;
+    return;
+  }
+
+  media::MediaCodecStatus status = MEDIA_CODEC_OK;
+
+  if (input_data.is_encrypted) {
+    // Note that input_data might not have a valid memory ptr if this is a
+    // re-send of a buffer that was sent before decryption keys arrived.
+
+    DVLOG(2) << __FUNCTION__ << ": QueueSecureInputBuffer:"
+             << " index:" << input_buffer.index
+             << " pts:" << input_data.presentation_time
+             << " size:" << input_data.length;
+
+    status = media_codec_->QueueSecureInputBuffer(
+        input_buffer.index, input_data.memory, input_data.length,
+        input_data.key_id, input_data.iv, input_data.subsamples,
+        input_data.presentation_time);
+
+  } else {
+    DVLOG(2) << __FUNCTION__ << ": QueueInputBuffer:"
+             << " index:" << input_buffer.index
+             << " pts:" << input_data.presentation_time
+             << " size:" << input_data.length;
+
+    status = media_codec_->QueueInputBuffer(
+        input_buffer.index, input_data.memory, input_data.length,
+        input_data.presentation_time);
+  }
+
+  switch (status) {
+    case MEDIA_CODEC_ERROR:
+      DVLOG(0) << __FUNCTION__ << ": MEDIA_CODEC_ERROR from QueueInputBuffer";

[DaleCurtis, 2016/06/13 23:16:13]

DLOG(ERROR)

[liberato (no reviews please), 2016/06/14 17:26:44]

Done.

+      input_data.completion_cb.Run(DecodeStatus::DECODE_ERROR);
+      // Transition to the error state after running the completion cb, to keep
+      // it in order if the client chooses to flush its queue.
+      SetState(STATE_ERROR);
+      break;
+
+    case MEDIA_CODEC_NO_KEY:
+      DVLOG(1) << "QueueSecureInputBuffer failed: MEDIA_CODEC_NO_KEY";
+      // Do not call the completion cb here.  It will be called when we retry
+      // after getting the key.
+      pending_input_buf_index_ = input_buffer.index;
+      pending_input_buf_data_ = input_data;
+      // MediaCodec has a copy of the data already.  When we call again, be sure
+      // to send in nullptr for the source.  Note that the client doesn't
+      // guarantee that the pointer will remain valid after we return anyway.
+      pending_input_buf_data_.memory = nullptr;
+      SetState(STATE_WAITING_FOR_KEY);
+      break;
+
+    case MEDIA_CODEC_OK:
+      input_data.completion_cb.Run(DecodeStatus::OK);
+      break;
+
+    default:
+      NOTREACHED() << "Unknown Queue(Secure)InputBuffer status " << status;
+      input_data.completion_cb.Run(DecodeStatus::DECODE_ERROR);
+      SetState(STATE_ERROR);
+      break;
+  }
+}
+
+bool MediaCodecLoop::ProcessOneOutputBuffer() {
+  DVLOG(2) << __FUNCTION__;
+
+  MediaCodecStatus status;
+  OutputBuffer out;
+  bool did_work = false;
+
+  // TODO(liberato): When merging AVDA, we will also have to ask the client if
+  // it can accept another output buffer.
+
+  status = media_codec_->DequeueOutputBuffer(kNoWaitTimeout, &out.index,
+                                             &out.offset, &out.size, &out.pts,
+                                             &out.is_eos, &out.is_key_frame);
+
+  switch (status) {
+    case MEDIA_CODEC_OUTPUT_BUFFERS_CHANGED:
+      // Output buffers are replaced in MediaCodecBridge, nothing to do.
+      DVLOG(2) << __FUNCTION__ << " MEDIA_CODEC_OUTPUT_BUFFERS_CHANGED";
+      did_work = true;
+      break;
+
+    case MEDIA_CODEC_OUTPUT_FORMAT_CHANGED:
+      DVLOG(2) << __FUNCTION__ << " MEDIA_CODEC_OUTPUT_FORMAT_CHANGED";
+      if (!client_->OnOutputFormatChanged())
+        SetState(STATE_ERROR);
+      did_work = (state_ != STATE_ERROR);
+      break;
+
+    case MEDIA_CODEC_OK:
+      // We got the decoded frame.
+      if (out.is_eos) {
+        // Set state STATE_DRAINED after we have received EOS frame at the
+        // output. media_decoder_job.cc says: once output EOS has occurred, we
+        // should not be asked to decode again.
+        DCHECK_EQ(state_, STATE_DRAINING);
+        SetState(STATE_DRAINED);
+
+        DCHECK_NE(out.index, kInvalidBufferIndex);
+        DCHECK(media_codec_);
+
+        media_codec_->ReleaseOutputBuffer(out.index, false);
+
+        // Run the EOS completion callback now, since we deferred it until
+        // the EOS was completely processed.
+        pending_eos_completion_cb_.Run(DecodeStatus::OK);
+        pending_eos_completion_cb_ = DecodeCB();
+
+        client_->OnDecodedEos(out);
+      } else {
+        if (!client_->OnDecodedFrame(out))
+          SetState(STATE_ERROR);
+      }
+
+      did_work = true;
+      break;
+
+    case MEDIA_CODEC_DEQUEUE_OUTPUT_AGAIN_LATER:
+      // Nothing to do.
+      DVLOG(2) << __FUNCTION__ << " MEDIA_CODEC_DEQUEUE_OUTPUT_AGAIN_LATER";
+      break;
+
+    case MEDIA_CODEC_ERROR:
+      DVLOG(0) << __FUNCTION__

[DaleCurtis, 2016/06/13 23:16:13]

DLOG(ERROR)

[liberato (no reviews please), 2016/06/14 17:26:44]

Done.

+               << ": MEDIA_CODEC_ERROR from DequeueOutputBuffer";
+
+      // Next Decode() will report the error to the pipeline.
+      SetState(STATE_ERROR);
+      break;
+
+    default:
+      NOTREACHED() << "Unknown DequeueOutputBuffer status " << status;
+      // Next Decode() will report the error to the pipeline.
+      SetState(STATE_ERROR);
+      break;
+  }
+
+  return did_work;
+}
+
+void MediaCodecLoop::ManageTimer(bool did_work) {
+  bool should_be_running = true;
+
+  base::TimeTicks now = base::TimeTicks::Now();
+  if (did_work || idle_time_begin_ == base::TimeTicks()) {
+    idle_time_begin_ = now;
+  } else {
+    // Make sure that we have done work recently enough, else stop the timer.
+    if (now - idle_time_begin_ > kIdleTimerTimeout)
+      should_be_running = false;
+  }
+
+  if (should_be_running && !io_timer_.IsRunning()) {
+    io_timer_.Start(FROM_HERE, kDecodePollDelay, this,
+                    &MediaCodecLoop::DoIOTask);
+  } else if (!should_be_running && io_timer_.IsRunning()) {
+    io_timer_.Stop();
+  }
+}
+
+void MediaCodecLoop::SetState(State new_state) {
+  DVLOG(1) << __FUNCTION__ << ": " << AsString(state_) << "->"
+           << AsString(new_state);
+  if (state_ != new_state && state_ == STATE_ERROR)
+    client_->OnCodecLoopError();
+  state_ = new_state;
+}
+
+MediaCodecBridge* MediaCodecLoop::GetCodec() const {
+  return media_codec_.get();
+}
+
+#if defined(RETURN_STRING)

[DaleCurtis, 2016/06/13 23:16:13]

Hmm, actually, is this even necessary? I.e. won't the compiler complain about
these collisions already?

[liberato (no reviews please), 2016/06/14 17:26:44]

yes.

+#error Unexpected macro collision.
+#endif
+
+// static
+const char* MediaCodecLoop::AsString(State state) {
+#define RETURN_STRING(x) \
+  case x:                \
+    return #x;
+
+  switch (state) {
+    RETURN_STRING(STATE_READY);
+    RETURN_STRING(STATE_WAITING_FOR_KEY);
+    RETURN_STRING(STATE_DRAINING);
+    RETURN_STRING(STATE_DRAINED);
+    RETURN_STRING(STATE_ERROR);
+  }
+#undef RETURN_STRING
+
+  NOTREACHED() << "Unknown state " << state;
+  return nullptr;
+}
+
+}  // namespace media