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/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.

[watk, 2016/06/15 00:29:18]

The "similar to AVDA" comment will be very soon out of date. I think the main
point is that we poll at 10ms using non-blocking calls because a) we don't want
to block the thread, and b) we can't tell in advance whether the calls will
succeed or block indefinitely.

[liberato (no reviews please), 2016/06/15 20:27:30]

Done.

+//
+// 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]

[watk, 2016/06/15 00:29:18]

Should this be any state? (not only Ready)?

[liberato (no reviews please), 2016/06/15 20:27:30]

Done.

+//     |
+// (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)
+//    |             |

[watk, 2016/06/15 00:29:17]

Should we have a flushing state in this diagram?

[liberato (no reviews please), 2016/06/15 20:27:30]

it has draining / drained.  is that what you mean?

+// [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)>;
+
+  // Data that the client wants to put into an input buffer.
+  struct InputData {
+    InputData();
+    InputData(const InputData&);
+    ~InputData();
+
+    const uint8_t* memory = nullptr;
+    size_t length = 0;
+
+    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 = false;
+    bool is_encrypted = false;
+  };
+
+  // Information about a MediaCodec output buffer.
+  struct OutputBuffer {

[DaleCurtis, 2016/06/14 19:29:03]

Assign defaults?

[liberato (no reviews please), 2016/06/14 21:56:45]

thanks, meant to do it in the last one.

+    int index;      // The codec output buffers are referred to by this index.
+    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.
+  };
+
+  class Client {
+   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() const = 0;
+
+    // Fills and returns an input buffer for MediaCodecLoop to queue.  It is
+    // an error for MediaCodecLoop to call this while !IsAnyInputPending().
+    virtual InputData ProvideInputData() = 0;
+
+    // Called when an EOS buffer is dequeued from the output.
+    virtual void OnDecodedEos(const OutputBuffer& 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 OutputBuffer& 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 OnCodecLoopError() = 0;
+
+   protected:
+    ~Client() {}

[DaleCurtis, 2016/06/14 19:29:03]

virtual per style guide.

[liberato (no reviews please), 2016/06/14 21:56:45]

Done.

[watk, 2016/06/15 00:29:17]

My bad Frank. I didn't realize the style guide says the destructor should be
virtual in this case.

+  };
+
+  // Handy enum for "no buffer".
+  enum { kInvalidBufferIndex = -1 };
+
+  // We will take ownership of |media_codec|.  We will not destroy it until
+  // we are destructed.  |media_codec| may not be null.
+  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();

[watk, 2016/06/15 00:29:17]

Should we rename this now? I'm trying to think of what the API would look like
if we created MCL from the start, and I'm pretty sure it wouldn't be called
DoIOTask().

Probably I would imagine something like:
MCL codec_loop = new MCL();
codec_loop->StartLooping();

[liberato (no reviews please), 2016/06/15 20:27:30]

agree that it should be renamed.  don't like 'start looping'.  it sounds too
much like it won't return from the call.  I'll try DoPendingWork().

+
+  // 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.

[watk, 2016/06/15 00:29:18]

This can return false if the state is DRAINED, but the codec is also still
usable? So the comment is a little imprecise.

[liberato (no reviews please), 2016/06/15 20:27:30]

i don't think that the codec is still usable after EOS without a flush or
start/stop.

+  bool TryFlush();
+
+  // Callback called by the client when a new key is available.  This allows

[watk, 2016/06/15 00:29:17]

Comment feels focused on the implementation rather than the contract. e.g., it's
not necessarily the client that has to call this. What about:

"This should be called when a new key is added. Decoding will resume if it was
stopped in the WAITING_FOR_KEY state."

[liberato (no reviews please), 2016/06/15 20:27:30]

Done.

+  // 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.

[watk, 2016/06/15 00:29:18]

Not technically true at the moment :) Maybe guaranteed non-null if state is not
error?

[liberato (no reviews please), 2016/06/15 20:27:30]

Done.

+  MediaCodecBridge* GetCodec() const;
+
+ protected:
+  enum State {
+    STATE_READY,
+    STATE_WAITING_FOR_KEY,
+    STATE_DRAINING,
+    STATE_DRAINED,
+    STATE_ERROR,
+  };
+
+  // Information about dequeued input buffer.
+  struct InputBuffer {
+    InputBuffer() : index(kInvalidBufferIndex), is_pending(false) {}

[DaleCurtis, 2016/06/14 19:29:03]

Drop and use inline values instead?

[liberato (no reviews please), 2016/06/14 21:56:45]

Done.

+    InputBuffer(int i, bool p) : index(i), is_pending(p) {}
+
+    // The codec input buffers are referred to by this index.
+    int index;
+    bool is_pending;  // True if we tried to enqueue this buffer before.
+  };
+
+  // Enqueues one pending input buffer into MediaCodec if MediaCodec has room,
+  // and if the client has any input to give us.
+  // Returns true if any input was processed.
+  bool ProcessOneInputBuffer();
+
+  // Get data for |input_buffer| from the client and queue it.
+  void EnqueueInputBuffer(const InputBuffer& input_buffer);
+
+  // Dequeues an empty input buffer from the codec and returns the information
+  // about it. InputBuffer.index is the index of the dequeued buffer or -1 if
+  // the codec is busy or an error occured.  InputBuffer.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.
+  InputBuffer DequeueInputBuffer();
+
+  // Dequeues one output buffer from MediaCodec if it is immediately available,
+  // and sends it to the client.
+  // Returns true if an output buffer was received from MediaCodec.
+  bool ProcessOneOutputBuffer();
+
+  // 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

[watk, 2016/06/15 00:29:18]

bfufer

[liberato (no reviews please), 2016/06/15 20:27:30]

Dnoe.

[watk, 2016/06/15 20:52:11]

s/Dnoe/Done :)

+  // to us by the client.  |memory| has been cleared, since the codec has it.
+  InputData 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_;

[DaleCurtis, 2016/06/14 19:29:03]

Annotate with comment indicating this must always be the last member to avoid
destruction races. See other classes for examples.

[liberato (no reviews please), 2016/06/14 21:56:44]

done.

also, i think clang now yells at you if it isn't.  at least, in some cases it
does.

+
+  DISALLOW_COPY_AND_ASSIGN(MediaCodecLoop);
+};
+
+}  // namespace media
+
+#endif  // MEDIA_BASE_ANDROID_MEDIA_CODEC_LOOP_H_