Separate MediaCodecLoop from MediaCodecAudioDecoder

media/base/android/media_codec_loop.h

+// 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.
+
+#ifndef MEDIA_BASE_ANDROID_MEDIA_CODEC_LOOP_H_
+#define MEDIA_BASE_ANDROID_MEDIA_CODEC_LOOP_H_
+
+#include <deque>
+#include <memory>
+#include <utility>
+#include <vector>
+
+#include "base/macros.h"
+#include "base/memory/weak_ptr.h"
+#include "base/time/time.h"
+#include "base/timer/timer.h"
+#include "media/base/android/media_codec_bridge.h"
+#include "media/base/android/media_drm_bridge_cdm_context.h"
+#include "media/base/audio_decoder.h"
+#include "media/base/audio_decoder_config.h"
+#include "media/base/media_export.h"
+
+// MediaCodecLoop is based on Android's MediaCodec API.
+// The MediaCodec API is required to play encrypted (as in EME) content on
+// Android. It is also a way to employ hardware-accelerated decoding.
+// One MediaCodecLoop instance owns a single MediaCodec(Bridge) instance, and
+// drives it to perform decoding in conjunction with a MediaCodecLoop::Client.
+// The Client provides the input data and consumes the output data.  A typical
+// example is AndroidVideoDecodeAccelerator.
+
+// Implementation notes.
+//
+// The MediaCodec
+// (http://developer.android.com/reference/android/media/MediaCodec.html) works
+// by exchanging buffers between the client and the codec itself. On the input
+// side an "empty" buffer has to be dequeued from the codec, filled with data
+// and queued back. On the output side a "full" buffer with data should be
+// dequeued, the data is to be used somehow (copied out, or rendered to a pre-
+// defined texture for video) and the buffer has to be returned back (released).
+// Neither input nor output dequeue operations are guaranteed to succeed: the
+// codec might not have available input buffers yet, or not every encoded buffer
+// has arrived to complete an output frame. In such case the client should try
+// to dequeue a buffer again at a later time.
+//
+// There is also a special situation related to an encrypted stream, where the
+// enqueuing of a filled input buffer might fail due to lack of the relevant key
+// in the CDM module.  While MediaCodecLoop does not handle the CDM directly,
+// it does understand the codec state.
+//
+// Much of that logic is common to all users of MediaCodec.  The MediaCodecLoop
+// provides the main driver loop to talk to MediaCodec.  Via the Client
+// interface, MediaCodecLoop can request input data, send output buffers, etc.
+// MediaCodecLoop does not construct a MediaCodec, but instead takes ownership
+// of one that it provided during construction.
+//
+// Although one can specify a delay in the MediaCodec's dequeue operations,
+// this implementation follows the simple logic which is similar to
+// AndroidVideoDecodeAccelerator: no delay for either input or output buffers,
+// the processing is initated by the timer with short period (10 ms). Using no
+// delay for enqueue operations has an extra benefit of not blocking the current
+// thread.
+//
+// This implementation detects the MediaCodec idle run (no input or output
+// buffer processing) and after being idle for a predefined time the timer
+// stops.  The client is responsible for signalling us to wake up via a call
+// to DoIOTask.  It is okay for the client to call this even when the timer is
+// already running.
+//
+// The current implementation is single threaded. Every method is supposed to
+// run on the same thread.
+//
+// State diagram.
+//
+//  [Ready]
+//     |
+// (MediaCodec error)
+//     |
+//   [Error]
+//
+//  [Ready]
+//     |
+// (EOS enqueued)
+//     |
+//  [Draining]
+//     |
+// (EOS dequeued on output)
+//     |
+//   [Drained]
+//
+//      [Ready]
+//         |
+// (MEDIA_CODEC_NO_KEY)
+//         |
+//   [WaitingForKey]
+//         |
+//    (OnKeyAdded)
+//         |
+//      [Ready]
+//
+//     -[Any state]-
+//    |             |
+// (Flush ok) (Flush fails)
+//    |             |
+// [Ready]       [Error]
+
+namespace base {
+class SingleThreadTaskRunner;
+}
+
+namespace media {
+
+class MEDIA_EXPORT MediaCodecLoop {
+ public:
+  // TODO(liberato): this exists in video_decoder.h and audio_decoder.h too.
+  using InitCB = base::Callback<void(bool success)>;
+  using DecodeCB = base::Callback<void(DecodeStatus status)>;
+
+  // Information about dequeued input buffer.
+  struct InputBufferInfo {

[watk, 2016/06/09 02:04:34]

I think I like this better if you drop the Info and call them InputBuffer and
OutputBuffer.

InputBuffer input_buffer = DequeueInputBuffer();
if (input_buffer.index != kInvalid)
  EnqueueInputBuffer(input_buffer);

[liberato (no reviews please), 2016/06/10 21:34:10]

Done.  also moved to protected since the client never sees it.

+    const int

[DaleCurtis, 2016/06/08 22:55:01]

Odd formatting. Did clang-format do this?

[liberato (no reviews please), 2016/06/10 21:34:09]

yeah.  i moved the comment above it, and now it's what one would expect.

+        buf_index;  // The codec input buffers are referred to by this index.

[watk, 2016/06/09 02:04:34]

nit: "buf_" feels redundant now

[liberato (no reviews please), 2016/06/10 21:34:09]

Done.

+    const bool is_pending;  // True if we tried to enqueue this buffer before.
+    InputBufferInfo(int i, bool p) : buf_index(i), is_pending(p) {}
+  };
+
+  // Data that the client wants to put into an input buffer.
+  struct InputBufferData {

[watk, 2016/06/09 02:04:34]

InputBufferInfo and InputBufferData are pretty similar, but I'm struggling to
think of a better name. InputData?

[liberato (no reviews please), 2016/06/10 21:34:10]

Done.

+    InputBufferData();
+    InputBufferData(const InputBufferData&);
+    InputBufferData(const uint8_t* m, size_t l, const DecodeCB& cb);
+    ~InputBufferData();

[watk, 2016/06/09 02:04:34]

You might be able to get away with the default constructor/destructor/copy
constructor and fiddle with the fields directly, unless I've missed some calls
to these.

[liberato (no reviews please), 2016/06/10 21:34:09]

i've removed the unused constructor.  the copy constructor needs to be present
because chromium-style yells at me when i don't.

+
+    const uint8_t* memory;
+    size_t length;
+
+    std::string key_id;
+    std::string iv;
+    std::vector<SubsampleEntry> subsamples;
+
+    base::TimeDelta presentation_time;
+
+    // Called when this is queued.
+    DecodeCB completion_cb;
+
+    bool is_eos;
+    bool is_encrypted;
+  };
+
+  // Information about the MediaCodec's output buffer.

[watk, 2016/06/09 02:04:33]

Maybe: "Information about a MediaCodec output buffer."

[liberato (no reviews please), 2016/06/10 21:34:10]

Done.

+  struct OutputBufferInfo {
+    int buf_index;  // The codec output buffers are referred to by this index.

[watk, 2016/06/09 02:04:34]

Same comment about "buf_"

[liberato (no reviews please), 2016/06/10 21:34:09]

Done.

+    size_t offset;  // Position in the buffer where data starts.
+    size_t size;    // The size of the buffer (includes offset).
+    base::TimeDelta pts;  // Presentation timestamp.
+    bool is_eos;          // true if this buffer is the end of stream.
+    bool is_key_frame;    // true if this buffer is a key frame.

[watk, 2016/06/09 02:04:33]

These two comments should be capitalized. (Or deleted).

I wonder if MediaCodecBridge::DequeueOutputBuffer should return a struct like
this?

[liberato (no reviews please), 2016/06/10 21:34:10]

Caps: done.  not sure about MediaCodecBridge.

+  };
+
+  class Client {

[watk, 2016/06/09 02:04:34]

Protected non-virtual destructor? And DISALLOW_COPY_AND_ASSIGN.

[liberato (no reviews please), 2016/06/10 21:34:09]

destructor: good idea.  done.
DISALLOW_COPY_AND_ASSIGN: there's no data to copy or assign.

+   public:
+    // Return true if and only if there is input that is pending to be
+    // queued with MediaCodec.  ProvideInputData() will not be called more than
+    // once in response to this returning true once.  It is not guaranteed that
+    // ProvideInputData will be called at all.
+    virtual bool IsAnyInputPending() = 0;
+
+    // Fills and returns an input buffer for MediaCodecLoop to queue.  It is
+    // assumed that IsAnyInputPending() has already verified that there is data
+    // to be queued.

[watk, 2016/06/09 02:04:34]

Could be more explicit: "It is an error to call this if !IsAnyInputPending()."?

[liberato (no reviews please), 2016/06/10 21:34:10]

that's too strict, since IsAnyInputPending is not const.  it could change the
result of the next call, but it would still be okay to get the input data that
was pending on the original.

actually, i'll make it const and update the docs as you suggest.

+    virtual InputBufferData ProvideInputData() = 0;
+
+    // Called when an EOS buffer is dequeued from the output.
+    virtual void OnDecodedEos(const OutputBufferInfo& out) = 0;
+
+    // Processes the output buffer after it comes from MediaCodec.  The client
+    // has the responsibility to release the codec buffer, though it doesn't
+    // need to do so before this call returns.  If it does not do so before
+    // returning, then the client must call DoIOTask when it does release it.
+    // If this returns false, then we transition to STATE_ERROR.
+    virtual bool OnDecodedFrame(const OutputBufferInfo& out) = 0;
+
+    // Processes the output format change on |media_codec|.  Returns true on
+    // success, or false to transition to the error state.
+    virtual bool OnOutputFormatChanged() = 0;
+
+    // Notify the client when our state transitions to STATE_ERROR.
+    virtual void OnError() = 0;
+  };
+
+  // Handy enum for "no buffer".
+  enum { kInvalidBufferIndex = -1 };
+
+  // We will take ownership of |media_codec|.  We will not destroy it until
+  // we are destructed.
+  MediaCodecLoop(Client* client,
+                 std::unique_ptr<MediaCodecBridge>&& media_codec);
+  ~MediaCodecLoop();
+
+  // Does the MediaCodec processing cycle: enqueues an input buffer, then
+  // dequeues output buffers.  This should be called by the client when more
+  // work becomes available, such as when new input data arrives.  If codec
+  // output buffers are freed after OnDecodedFrame returns, then this should
+  // also be called.
+  void DoIOTask();
+
+  // Try to flush this media codec.  Returns true on success, false on failure.
+  // Failures can result in a state change to the Error state.  If this returns
+  // false but the state is still READY, then the codec may continue to be used.
+  bool TryFlush();
+
+  // Callback called by the client when a new key is available.  This allows
+  // decoding to resume if it was stopped in the WAITING_FOR_KEY state.
+  void OnKeyAdded();
+
+  // Return our codec.  It is guaranteed that this will be non-null, if we were
+  // given a non-null codec during construction.

[watk, 2016/06/09 02:04:33]

Can we explicitly require that media_codec is not null in the constructor with a
DCHECK so we don't have to consider this returning null? 

[liberato (no reviews please), 2016/06/10 21:34:09]

Done.

+  MediaCodecBridge* GetCodec() const;
+
+ protected:
+  // Possible states.

[watk, 2016/06/09 02:04:33]

Comment seems redundant

[liberato (no reviews please), 2016/06/10 21:34:10]

Done.

+  enum State {
+    STATE_READY,
+    STATE_WAITING_FOR_KEY,
+    STATE_DRAINING,
+    STATE_DRAINED,
+    STATE_ERROR,
+  };
+
+  // Enqueues pending input buffers into MediaCodec as long as it can happen
+  // without delay in dequeuing and enqueueing input buffers.
+  // Returns true if any input was processed.
+  bool QueueInput();
+
+  // Enqueues one pending input buffer into MediaCodec if MediaCodec has room.
+  // Returns true if any input was processed.
+  bool QueueOneInputBuffer();
+
+  // Get data for |input_info| from the client and queue it.
+  void EnqueueInputBuffer(const InputBufferInfo& input_info);
+
+  // A helper method for QueueInput(). Dequeues an empty input buffer from the
+  // codec and returns the information about it. OutputBufferInfo.buf_index is
+  // the index of the dequeued buffer or -1 if the codec is busy or an error
+  // occured.  OutputBufferInfo.is_pending is set to true if we tried to enqueue
+  // this buffer before. In this case the buffer is already filled with data.
+  // In the case of an error sets STATE_ERROR.
+  InputBufferInfo DequeueInputBuffer();
+
+  // Dequeues all output buffers from MediaCodec that are immediately available.
+  // Returns true if any output buffer was received from MediaCodec.
+  bool DequeueOutput();
+
+  // Start the timer immediately if |start| is true or stop it based on elapsed
+  // idle time if |start| is false.
+  void ManageTimer(bool start);
+
+  // Helper method to change the state.
+  void SetState(State new_state);
+
+  // A helper function for logging.
+  static const char* AsString(State state);
+
+  // Used to post tasks. This class is single threaded and every method should
+  // run on this task runner.
+  scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
+
+  State state_;
+
+  // The client that we notify about MediaCodec events.
+  Client* client_;
+
+  // The MediaCodec instance that we're using.
+  std::unique_ptr<MediaCodecBridge> media_codec_;
+
+  // Repeating timer that kicks MediaCodec operation.
+  base::RepeatingTimer io_timer_;
+
+  // Time at which we last did useful work on |io_timer_|.
+  base::TimeTicks idle_time_begin_;
+
+  // Index of the dequeued and filled buffer that we keep trying to enqueue.
+  // Such buffer appears in MEDIA_CODEC_NO_KEY processing. The -1 value means
+  // there is no such buffer.
+  int pending_input_buf_index_;
+
+  // When processing a pending input bfufer, this is the data that was returned
+  // to us by the client.  |memory| has been cleared, since the codec has it.
+  InputBufferData pending_input_buf_data_;
+
+  // When an EOS is queued, we defer its completion callback until the EOS
+  // arrives at the output queue.  This is valid when we're in STATE_DRAINING.
+  DecodeCB pending_eos_completion_cb_;
+
+  base::WeakPtrFactory<MediaCodecLoop> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(MediaCodecLoop);
+};
+
+}  // namespace media
+
+#endif  // MEDIA_BASE_ANDROID_MEDIA_CODEC_LOOP_H_