AndroidVDA by using Android's MediaCodec API.

content/common/gpu/media/android_video_decode_accelerator.cc

+// 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 "content/common/gpu/media/android_video_decode_accelerator.h"
+
+#include <jni.h>
+
+#include "base/android/jni_android.h"
+#include "base/android/scoped_java_ref.h"
+#include "base/bind.h"
+#include "base/logging.h"
+#include "base/message_loop.h"
+#include "content/common/android/surface_callback.h"
+#include "content/common/gpu/gpu_channel.h"
+#include "content/common/gpu/media/gles2_external_texture_copier.h"
+#include "media/base/bitstream_buffer.h"
+#include "media/video/picture.h"
+#include "third_party/angle/include/GLES2/gl2.h"
+#include "third_party/angle/include/GLES2/gl2ext.h"
+
+using base::android::MethodID;
+using base::android::ScopedJavaLocalRef;
+
+namespace content {
+
+// XXX: drop the below before submitting.
+#define LOG_LINE() LOG(INFO) << __FUNCTION__
+
+#undef DCHECK
+#define DCHECK CHECK
+
+// Helper macros for dealing with failure.  If |result| evaluates false, emit
+// |log| to ERROR, register |error| with the decoder, and return.
+#define RETURN_ON_FAILURE(result, log, error)                      \
+  do {                                                             \
+    if (!(result)) {                                               \
+      LOG(ERROR) << log;                                           \

[Ami GONE FROM CHROMIUM, 2013/01/28 19:49:45]

DLOG instead of LOG

[dwkang1, 2013/02/04 14:08:26]

Done.

+      client_->NotifyError(error);                                 \
+      state_ = ERROR;                                              \
+      return;                                                      \
+    }                                                              \
+  } while (0)
+
+enum { kNumPictureBuffers = 4 };
+
+// static
+const base::TimeDelta AndroidVideoDecodeAccelerator::kDecodePollDelay =
+    base::TimeDelta::FromMilliseconds(10);
+
+AndroidVideoDecodeAccelerator::AndroidVideoDecodeAccelerator(
+    media::VideoDecodeAccelerator::Client* client,
+    const base::Callback<bool(void)>& make_context_current)
+    : client_(client),
+      make_context_current_(make_context_current),
+      codec_(media::MediaCodecBridge::UNKNOWN),
+      state_(NO_ERROR),
+      surface_texture_id_(-1),
+      picturebuffers_requested_(false),
+      io_task_is_running_(false) {
+  LOG_LINE();
+}
+
+AndroidVideoDecodeAccelerator::~AndroidVideoDecodeAccelerator() {
+  LOG_LINE();
+  DCHECK(thread_checker_.CalledOnValidThread());
+}
+
+bool AndroidVideoDecodeAccelerator::Initialize(
+    media::VideoCodecProfile profile) {
+  LOG_LINE();
+  DCHECK(media_codec_ == NULL);
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  if (profile == media::VP8PROFILE_MAIN) {
+    codec_ = media::MediaCodecBridge::VIDEO_VP8;
+  } else if (profile >= media::H264PROFILE_MIN
+      && profile <= media::H264PROFILE_MAX) {

[Ami GONE FROM CHROMIUM, 2013/01/28 19:49:45]

&& operator goes on previous line (here and elsewhere)

[dwkang1, 2013/02/04 14:08:26]

Done.

+    codec_ = media::MediaCodecBridge::VIDEO_H264;
+  } else {
+    codec_ = media::MediaCodecBridge::UNKNOWN;

[Ami GONE FROM CHROMIUM, 2013/01/28 19:49:45]

unnecessary

[dwkang1, 2013/02/04 14:08:26]

Done.

+    LOG(ERROR) << "Unsupported profile: " << profile;
+    return false;
+  }
+
+  if (!make_context_current_.Run()) {
+    LOG(ERROR) << "Failed to make this decoder's GL context current.";
+    return false;
+  }
+  // XXX: apply the scheme for GL access. http://crbug.com/169433
+  glGenTextures(1, &surface_texture_id_);
+  glActiveTexture(GL_TEXTURE0);
+  glBindTexture(GL_TEXTURE_EXTERNAL_OES, surface_texture_id_);
+
+  glTexParameteri(GL_TEXTURE_EXTERNAL_OES, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+  glTexParameteri(GL_TEXTURE_EXTERNAL_OES, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+  glTexParameteri(GL_TEXTURE_EXTERNAL_OES,
+                  GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
+  glTexParameteri(GL_TEXTURE_EXTERNAL_OES,
+                  GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
+
+  surface_texture_ = new SurfaceTextureBridge(surface_texture_id_);
+
+  ConfigureMediaCodec();
+
+  MessageLoop::current()->PostTask(FROM_HERE, base::Bind(
+      &AndroidVideoDecodeAccelerator::NotifyInitializeDone,
+      base::AsWeakPtr(this)));
+  return true;
+}
+
+void AndroidVideoDecodeAccelerator::DoIOTask() {
+  if (state_ == NO_ERROR) {

[Ami GONE FROM CHROMIUM, 2013/01/28 19:49:45]

If this is false, then you don't want to enqueue more work, either.
I think this just wants to be
if (state_ == ERROR)
  return;
instead.

[dwkang1, 2013/02/04 14:08:26]

Done.

+    DequeueOutput();
+    QueueInput();
+  }
+
+  if (!pending_bitstream_buffers_.empty()
+      || !bitstream_buffer_ids_in_decoder_.empty()) {

[Ami GONE FROM CHROMIUM, 2013/01/28 19:49:45]

operator goes on previous line

[dwkang1, 2013/02/04 14:08:26]

Done.

+    MessageLoop::current()->PostDelayedTask(
+        FROM_HERE,
+        base::Bind(
+            &AndroidVideoDecodeAccelerator::DoIOTask, base::AsWeakPtr(this)),
+            kDecodePollDelay);
+  } else {
+    io_task_is_running_ = false;

[Ami GONE FROM CHROMIUM, 2013/01/28 19:49:45]

IMO this would be more clear as io_task_is_posted_, which you'd set to true each
time you Post'd DoIOTask, and which you cleared as the very first thing that
DoIOTask did.

[dwkang1, 2013/02/04 14:08:26]

Good idea. Thanks,

+  }
+}
+
+void AndroidVideoDecodeAccelerator::QueueInput() {
+  if (pending_bitstream_buffers_.empty()) {
+    return;
+  }
+  int input_buf_index =
+      media_codec_->DequeueInputBuffer(0);

[Ami GONE FROM CHROMIUM, 2013/01/28 19:49:45]

wrapping necessary?

[dwkang1, 2013/02/04 14:08:26]

Done.

+  if (input_buf_index < 0) {
+    return;

[Ami GONE FROM CHROMIUM, 2013/01/28 19:49:45]

DCHECK_EQ TRY_AGAIN_LATER?

[dwkang1, 2013/02/04 14:08:26]

Done.

+  }
+  media::BitstreamBuffer& bitstream_buffer =
+      pending_bitstream_buffers_.front();
+  pending_bitstream_buffers_.pop();
+
+  int flags = 0;
+  if (bitstream_buffer.id() == -1) {
+    flags |= media::MediaCodecBridge::BUFFER_FLAG_END_OF_STREAM;
+  }
+  if (bitstream_buffer.size() > 0) {
+    scoped_ptr<base::SharedMemory> shm(
+        new base::SharedMemory(bitstream_buffer.handle(), true));
+
+    RETURN_ON_FAILURE(shm->Map(bitstream_buffer.size()),
+                      "Failed to SharedMemory::Map()",
+                      UNREADABLE_INPUT);
+
+    media_codec_->PutToInputBuffer(
+        input_buf_index,
+        static_cast<const uint8*>(shm->memory()),
+        bitstream_buffer.size());
+  }
+  // Abuse the presentation time argument to propagate the bitstream
+  // buffer ID to the output, so we can report it back to the client in
+  // PictureReady().
+  int64 timestamp = bitstream_buffer.id();
+  media_codec_->QueueInputBuffer(
+      input_buf_index, 0, bitstream_buffer.size(), timestamp, flags);
+
+  bitstream_buffer_ids_in_decoder_.push(bitstream_buffer.id());
+  if (bitstream_buffer.id() != -1) {
+    client_->NotifyEndOfBitstreamBuffer(bitstream_buffer.id());

[Ami GONE FROM CHROMIUM, 2013/01/28 19:49:45]

This is wrong - NEOBB() must only be called once its contents have been decoded
(here, the buffer has only been handed to the decoder).
In particular, clients are likely to keep a table of bitstreambufferid ->
presentation-timestamp (and other metadata) and purge entries as
bitstreambufferid's are notified done.  But the entry is needed when a picture
is delivered with a given bitstreambufferid, so NEOBB should only be called once
you know no more pictures will be sent back with a given bitstreambufferid.

Given the MediaCodec API, it may be that it is only correct to call NEOBB once
input_buf_index is returned from DequeueInputBuffer() next.

OVDA is not wrong - EmptyBufferDoneTask is a callback that the underlying
OpenMAX library calls once it is done decoding from a buffer.

[dwkang1, 2013/01/29 03:58:48]

Got it. Seems that I misunderstood the meaning of EmptyBufferDone. Will fix in
the next patch set. Thanks for the explanation!

[dwkang1, 2013/02/04 14:08:26]

I checked GpuVideoDecoder implementation and have a question on it. Is the table
of bitstreambufferid ->
presentation-timestamp |input_buffer_data_| in GpuVideoDecoder?

IIRC, I don't see any relationship between |input_buffer_data_| and NEOBB. It
was just keeping last 128 entries to provide the mapping and be reset on
Reset(). And, GpuVideoDecoder::NotifyEndOfBitstreamBuffer() did nothing with
|input_buffer_data_|.

Actually, What happened in GpuVideoDecoder::NotifyEndOfBitstreamBuffer() was
remove the matching id in |bitstream_buffers_in_decoder_| and # of
|bitstream_buffers_in_decoder_| was limited by 4(kMaxInFlightDecodes).

When you say "once its contents have been decoded", does that mean "once we get
decoded output from the contents"? If so, it sounds to me that the decoder
should be able to start outputting decoded frames after getting 4 bitstream
buffers. However, in my test streams, decoder started outputting frames after
feeding 8-9 bitstream buffers.

+  }
+}
+
+void AndroidVideoDecodeAccelerator::DequeueOutput() {
+  if (picturebuffers_requested_ && output_picture_buffers_.empty()) {
+    return;
+  }
+  if (!output_picture_buffers_.empty() && free_picture_ids_.empty()) {
+    // Don't have any picture buffer to send. Need to wait more.
+    return;
+  }
+
+  int32 flag = 0;
+  int64 bitstream_buffer_id = 0;
+  int32 buf_index = 0;
+  do {
+    int32 offset = 0;
+    int32 size = 0;
+    buf_index = media_codec_->DequeueOutputBuffer(
+        0, &offset, &size, &bitstream_buffer_id, &flag);
+    switch (buf_index) {
+      case media::MediaCodecBridge::INFO_TRY_AGAIN_LATER: {
+        return;

[Ami GONE FROM CHROMIUM, 2013/01/28 19:49:45]

braces are unnecessary (here and elsewhere in case tatements that don't
introduce their own local variables; chromium/google3 style doesn't apply the
usual if/then/else rule about if one clause has braces they all must to case
statements).

[dwkang1, 2013/02/04 14:08:26]

Done.

+      }
+
+      case media::MediaCodecBridge::INFO_OUTPUT_FORMAT_CHANGED: {
+        int32 unused_color_format, width, height;
+        media_codec_->GetOutputFormat(&unused_color_format, &width, &height);
+
+        if (!picturebuffers_requested_) {
+          picturebuffers_requested_ = true;
+          size_ = gfx::Size(width, height);
+          texture_copier_.reset(new Gles2ExternalTextureCopier());
+          texture_copier_->Init(width, height);
+          client_->ProvidePictureBuffers(
+              kNumPictureBuffers,
+              size_,
+              GL_TEXTURE_2D);
+        } else {
+          // TODO(dwkang): support the dynamic resolution change.
+          RETURN_ON_FAILURE(size_.width() == width && size_.height() == height,

[Ami GONE FROM CHROMIUM, 2013/01/28 19:49:45]

size_ == gfx::Size(width, height)
might be more readable

[dwkang1, 2013/02/04 14:08:26]

Done.

+                            "Dynamic resolution change is not supported.",
+                            PLATFORM_FAILURE);
+        }
+        return;
+      }
+
+      case media::MediaCodecBridge::INFO_OUTPUT_BUFFERS_CHANGED: {
+        media_codec_->GetOutputBuffers();
+        break;
+      }
+    }
+  } while (buf_index < 0);
+
+  if (flag & media::MediaCodecBridge::BUFFER_FLAG_END_OF_STREAM) {
+    if (client_) {
+      client_->NotifyFlushDone();
+    }
+  }
+
+  media_codec_->ReleaseOutputBuffer(buf_index, true);
+
+  CHECK_EQ(bitstream_buffer_ids_in_decoder_.front(), bitstream_buffer_id);

[Ami GONE FROM CHROMIUM, 2013/01/28 19:49:45]

Presentation order does not have to match decode order, so I'm pretty sure this
is wrong.

[dwkang1, 2013/02/04 14:08:26]

Agreed. Haste made waste.

+  bitstream_buffer_ids_in_decoder_.pop();

[Ami GONE FROM CHROMIUM, 2013/01/28 19:49:45]

It's also wrong to assume that pictures & bitstreambuffers correspond 1:1 - a
single bitstreambuffer can contain more than one picture, and a single picture
can require multiple bitstreambuffers to decode.

[dwkang1, 2013/02/04 14:08:26]

You are correct. removed.

+
+  if (bitstream_buffer_id != -1) {
+    SendCurrentSurfaceToClient(static_cast<int32>(bitstream_buffer_id));
+  }
+}
+
+void AndroidVideoDecodeAccelerator::SendCurrentSurfaceToClient(
+    int32 bitstream_id) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK_NE(bitstream_id, -1);
+  DCHECK(!free_picture_ids_.empty());
+
+  int32 picture_buffer_id = free_picture_ids_.front();
+  free_picture_ids_.pop();
+
+  RETURN_ON_FAILURE(make_context_current_.Run(),
+                    "Failed to make this decoder's GL context current.",
+                    PLATFORM_FAILURE);
+
+  float transfrom_matrix[16];
+  surface_texture_->UpdateTexImage();
+  surface_texture_->GetTransformMatrix(transfrom_matrix);
+
+  OutputBufferMap::const_iterator i =
+      output_picture_buffers_.find(picture_buffer_id);
+  if (i == output_picture_buffers_.end()) {
+    LOG(ERROR) << "Can't find a PuctureBuffer for " << picture_buffer_id;

[Ami GONE FROM CHROMIUM, 2013/01/28 19:49:45]

typo: PuctureBuffer

[Ami GONE FROM CHROMIUM, 2013/01/28 19:49:45]

RETURN_ON_FAILURE

[dwkang1, 2013/02/04 14:08:26]

Done.

[dwkang1, 2013/02/04 14:08:26]

Done.

+    return;
+  }
+  uint32 picture_buffer_texture_id = i->second.texture_id();
+
+  // Here, we copy |surface_texture_id_| to the picture buffer instead of
+  // setting new texture to |surface_texture_| by calling attachToGLContext()
+  // because:
+  // 1. Once we call detachFrameGLContext(), it deletes the texture previous
+  //    attached.
+  // 2. SurfaceTexture requires us to apply a transform matrix when we show
+  //    the texture.
+  texture_copier_->Copy(
+      surface_texture_id_, picture_buffer_texture_id, transfrom_matrix);
+
+  client_->PictureReady(
+      media::Picture(picture_buffer_id, bitstream_id));
+}
+
+void AndroidVideoDecodeAccelerator::Decode(
+    const media::BitstreamBuffer& bitstream_buffer) {
+  LOG_LINE();
+  DCHECK(thread_checker_.CalledOnValidThread());
+  if (!client_) {
+    return;
+  }
+  pending_bitstream_buffers_.push(bitstream_buffer);
+
+  if (!io_task_is_running_) {
+    io_task_is_running_ = true;
+    MessageLoop::current()->PostDelayedTask(

[Ami GONE FROM CHROMIUM, 2013/01/28 19:49:45]

Why not call directly?

[dwkang1, 2013/02/04 14:08:26]

Done.

+        FROM_HERE,
+        base::Bind(
+            &AndroidVideoDecodeAccelerator::DoIOTask, base::AsWeakPtr(this)),
+            base::TimeDelta());
+  }
+}
+
+void AndroidVideoDecodeAccelerator::AssignPictureBuffers(
+    const std::vector<media::PictureBuffer>& buffers) {
+  LOG_LINE();
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(output_picture_buffers_.empty());
+
+  for (size_t i = 0; i < buffers.size(); ++i) {
+    output_picture_buffers_.insert(std::make_pair(buffers[i].id(), buffers[i]));

[Ami GONE FROM CHROMIUM, 2013/01/28 19:49:45]

output_picture_buffers_[buffers[i].id()] = buffers[i];
is slightly more readable.

[dwkang1, 2013/02/04 14:08:26]

It lead a compile error because PictureBuffer does not have default c-tor.
http://stackoverflow.com/questions/8279934/error-no-matching-function-for-cal...

+    free_picture_ids_.push(buffers[i].id());
+  }
+
+  RETURN_ON_FAILURE(output_picture_buffers_.size() == kNumPictureBuffers,
+                    "Invalid picture buffers were passed.",
+                    INVALID_ARGUMENT);
+}
+
+void AndroidVideoDecodeAccelerator::ReusePictureBuffer(
+    int32 picture_buffer_id) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  free_picture_ids_.push(picture_buffer_id);

[Ami GONE FROM CHROMIUM, 2013/01/28 19:49:45]

Doesn't this potentially require a DoIOTask?

[dwkang1, 2013/02/04 14:08:26]

Done.

+}
+
+void AndroidVideoDecodeAccelerator::Flush() {
+  LOG_LINE();
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  Decode(media::BitstreamBuffer(-1, base::SharedMemoryHandle(), 0));
+}
+
+void AndroidVideoDecodeAccelerator::ConfigureMediaCodec() {
+  DCHECK(surface_texture_.get());
+  DCHECK_NE(media::MediaCodecBridge::UNKNOWN, codec_);
+
+  media_codec_.reset(new media::MediaCodecBridge(codec_));

[Ami GONE FROM CHROMIUM, 2013/01/28 19:49:45]

You don't need codec_ to re-configure a MediaCodec, since you can get the
MediaFormat from GetOutputFormat.
(but, see above, I hope you can get away from this re-creating business)

[dwkang1, 2013/02/04 14:08:26]

Unfortunately, I was not able to drop the recreation because of
EOS->flush()->dequeueInputBuffer() issue. And, MediaCodec's getOutputFormat()
returns the format of output, mime type : "video/raw".

+
+  JNIEnv* env = base::android::AttachCurrentThread();
+  CHECK(env);
+  ScopedJavaLocalRef<jclass> cls(
+      base::android::GetClass(env, "android/view/Surface"));
+  jmethodID constructor = MethodID::Get<MethodID::TYPE_INSTANCE>(
+      env, cls.obj(), "<init>", "(Landroid/graphics/SurfaceTexture;)V");
+  ScopedJavaLocalRef<jobject> j_surface(
+      env, env->NewObject(
+          cls.obj(), constructor,
+          surface_texture_->j_surface_texture().obj()));
+
+  // VDA does not pass the container indicated resolution in the initialization
+  // phase. Here, we set 1080p by default.
+  media_codec_->ConfigureVideo(
+      codec_, gfx::Size(1920, 1080), j_surface.obj());

[Ami GONE FROM CHROMIUM, 2013/01/28 19:49:45]

Can you give more detail on what Exception MediaCodec threw when you used a 0x0
initial size?

[dwkang1, 2013/02/04 14:08:26]

Here is the logcat output when I pass 0x0 on Nexus10:
W/GraphicBufferAllocator(  124): alloc(1, 1, 263, 00002100, ...) failed -22
(Invalid argument)
E/SurfaceFlinger(  124): GraphicBufferAlloc::createGraphicBuffer(w=1, h=1)
failed (Invalid argument), handle=0x0
E/BufferQueue(28661): [unnamed-28661-0] dequeueBuffer:
SurfaceComposer::createGraphicBuffer failed
E/ACodec  (28661): dequeueBuffer failed: Invalid argument (22)
E/ACodec  (28661): Failed to allocate buffers after transitioning to IDLE state
(error 0xffffffea)
E/MediaCodec(28661): Codec reported an error. (omx error 0x80001001,
internalError -22)
W/System.err(28661): java.lang.IllegalStateException
W/System.err(28661):    at android.media.MediaCodec.start(Native Method)
W/System.err(28661):    at dalvik.system.NativeStart.run(Native Method)


FWIW, passing 1x1 also failed with similar output. A arbitrary small
resolution(320x240) worked, even though I tested with full-hd stream, but I
don't think this is the way to go.

+  content::ReleaseSurface(j_surface.obj());
+
+  media_codec_->GetInputBuffers();
+  media_codec_->GetOutputBuffers();
+}
+
+void AndroidVideoDecodeAccelerator::Reset() {
+  LOG_LINE();
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  while(!pending_bitstream_buffers_.empty()) {
+    media::BitstreamBuffer& bitstream_buffer =
+        pending_bitstream_buffers_.front();
+    pending_bitstream_buffers_.pop();
+
+    if (bitstream_buffer.id() != -1) {
+      client_->NotifyEndOfBitstreamBuffer(bitstream_buffer.id());

[Ami GONE FROM CHROMIUM, 2013/01/28 19:49:45]

Please audit all uses of client_ to make sure they don't de-reference on a
call-stack that could be rooted at the client itself (and Post instead of
calling directly).
Note that because of Destroy()'s contract, it's not enough to PostTask a Notify
method on the client directly; you must pass through a trampoline method on
*this* class that is guarded by a WeakPtr to avoid the case where a notification
to the client is Posted, then Destroy() arrives and deletes *this*, and then the
client gets an unexpected notification.

[dwkang1, 2013/02/04 14:08:26]

Done.

+    }
+  }
+  while(!bitstream_buffer_ids_in_decoder_.empty()) {
+    bitstream_buffer_ids_in_decoder_.pop();
+  }
+
+  media_codec_->Stop();
+  ConfigureMediaCodec();

[Ami GONE FROM CHROMIUM, 2013/01/28 19:49:45]

I still don't see why this is necessary; why not simply media_codec_->Start() at
this point?

[dwkang1, 2013/01/29 03:58:48]

When I tested "playback complete" and "replay" case, MediaCodec became unusable
once it meets EOS. Seems that this is another behavior which is not covered in
the api doc. Let me verify this, with Android team, and add some more comments
regarding this.

[dwkang1, 2013/02/04 14:08:26]

As you may know, we had a discussion with Android team, and it turned out a bug
on vendor's implementation. Considering that it happens many devices, I think we
need to have some workaround code. I changed to the code to do the recreation
only when we met EOS. PTAL.

+  state_ = NO_ERROR;
+
+  MessageLoop::current()->PostTask(FROM_HERE, base::Bind(
+      &AndroidVideoDecodeAccelerator::NotifyResetDone, base::AsWeakPtr(this)));
+}
+
+void AndroidVideoDecodeAccelerator::Destroy() {
+  LOG_LINE();
+  DCHECK(thread_checker_.CalledOnValidThread());
+  delete this;
+}
+
+void AndroidVideoDecodeAccelerator::NotifyInitializeDone() {
+  if (client_) {
+    client_->NotifyInitializeDone();
+  }
+}
+
+void AndroidVideoDecodeAccelerator::NotifyResetDone() {
+  if (client_) {

[Ami GONE FROM CHROMIUM, 2013/01/28 19:49:45]

here and elsewhere, client_ cannot be non-NULL; drop the if's.

[dwkang1, 2013/02/04 14:08:26]

Done.

+    client_->NotifyResetDone();
+  }
+}
+
+}  // namespace content