MJPEG acceleration for video capture using VAAPI

content/common/gpu/media/vaapi_jpeg_decode_accelerator.cc

+// 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 "content/common/gpu/media/vaapi_jpeg_decode_accelerator.h"
+
+#include "base/bind.h"
+#include "base/logging.h"
+#include "base/metrics/histogram.h"
+#include "base/stl_util.h"
+#include "base/strings/string_util.h"
+#include "base/synchronization/waitable_event.h"
+#include "content/common/gpu/gpu_channel.h"
+#include "content/common/gpu/media/vaapi_picture.h"
+#include "media/base/bind_to_current_loop.h"
+#include "media/base/video_frame.h"
+#include "media/filters/jpeg_parser.h"
+#include "media/video/picture.h"
+#include "ui/gl/gl_bindings.h"
+
+static void ReportVaapiError() {
+}
+
+namespace content {
+
+#define RETURN_AND_NOTIFY_ON_FAILURE(result, log, buffer_id, error_code, ret) \

[wuchengli, 2015/04/15 07:11:59]

This is only used once. Just use the following code directly.

[kcwu, 2015/04/16 14:38:28]

Done.

+  do {                                                                        \
+    if (!(result)) {                                                          \
+      LOG(ERROR) << log;                                                      \
+      NotifyError(buffer_id, error_code);                                     \
+      return ret;                                                             \
+    }                                                                         \
+  } while (0)
+
+VaapiJpegDecodeAccelerator::InputBuffer::InputBuffer() : id(0), size(0) {
+}
+
+VaapiJpegDecodeAccelerator::InputBuffer::~InputBuffer() {
+}
+
+void VaapiJpegDecodeAccelerator::NotifyError(int32_t bitstream_buffer_id,
+                                             Error error) {
+  if (message_loop_ != base::MessageLoop::current()) {
+    DCHECK(decoder_thread_proxy_->BelongsToCurrentThread());
+    message_loop_->PostTask(FROM_HERE,
+                            base::Bind(&VaapiJpegDecodeAccelerator::NotifyError,
+                                       weak_this_, bitstream_buffer_id, error));
+    return;
+  }
+
+  // Post Cleanup() as a task so we don't recursively acquire lock_.

[wuchengli, 2015/04/15 07:11:58]

I don't understand why recursively acquire lock_ is possible.

[kcwu, 2015/04/20 17:47:59]

I don't, neither. I guess VAVDA's original Cleanup might trigger NotifyError.
Remove this PostTask since we don't the issue here.

+  message_loop_->PostTask(
+      FROM_HERE, base::Bind(&VaapiJpegDecodeAccelerator::Cleanup, weak_this_));
+
+  LOG(ERROR) << "Notifying of error " << error;
+  if (client_) {
+    client_->NotifyError(bitstream_buffer_id, error);
+    client_ptr_factory_.reset();
+  }
+}
+
+VaapiJpegDecodeAccelerator::VaapiJpegDecodeAccelerator()
+    : state_(kUninitialized),
+      input_ready_(&lock_),
+      message_loop_(base::MessageLoop::current()),
+      decoder_thread_("VaapiDecoderThread"),

[wuchengli, 2015/04/15 07:11:58]

s/VaapiDecoderThread/VaapiJpegDecoderThread/ to distinguish from vaapi vda.

[kcwu, 2015/04/20 17:47:59]

Done.

+      weak_this_factory_(this) {
+  weak_this_ = weak_this_factory_.GetWeakPtr();
+}
+
+VaapiJpegDecodeAccelerator::~VaapiJpegDecodeAccelerator() {
+  DCHECK_EQ(message_loop_, base::MessageLoop::current());
+}
+
+bool VaapiJpegDecodeAccelerator::Initialize(Client* client) {
+  DCHECK_EQ(message_loop_, base::MessageLoop::current());
+
+  client_ptr_factory_.reset(new base::WeakPtrFactory<Client>(client));
+  client_ = client_ptr_factory_->GetWeakPtr();
+
+  base::AutoLock auto_lock(lock_);
+  DCHECK_EQ(state_, kUninitialized);
+
+  vaapi_wrapper_ =
+      VaapiWrapper::Create(VaapiWrapper::kDecode, VAProfileJPEGBaseline,
+                           base::Bind(&ReportVaapiError));
+
+  if (!vaapi_wrapper_.get()) {
+    DLOG(ERROR) << "Failed initializing VAAPI";
+    return false;
+  }
+
+  CHECK(decoder_thread_.Start());
+  decoder_thread_proxy_ = decoder_thread_.message_loop_proxy();
+
+  state_ = kIdle;
+  return true;
+}
+
+bool VaapiJpegDecodeAccelerator::OutputPicture(
+    VASurfaceID va_surface_id,
+    int32_t input_id,
+    const scoped_refptr<media::VideoFrame>& video_frame) {
+  DCHECK(decoder_thread_proxy_->BelongsToCurrentThread());
+
+  DVLOG(3) << "Outputting VASurface " << va_surface_id
+           << " into video_frame associate to input buffer id " << input_id;
+
+  VAImage image;
+  VAImageFormat format;
+  const uint32_t kI420Fourcc = VA_FOURCC('I', '4', '2', '0');
+  memset(&image, 0, sizeof(image));
+  memset(&format, 0, sizeof(format));
+  format.fourcc = kI420Fourcc;
+  format.byte_order = VA_LSB_FIRST;
+  format.bits_per_pixel = 12;  // 12 for I420
+
+  void* mem;
+  gfx::Size coded_size = video_frame->coded_size();
+  if (!vaapi_wrapper_->GetVaImage(va_surface_id, &format, coded_size, &image,
+                                  &mem)) {
+    DLOG(ERROR) << "Cannot get VAImage";
+    return false;
+  }
+
+  uint8* frame_mem = video_frame->data(media::VideoFrame::kYPlane);
+  size_t frame_buffer_size =
+      media::VideoFrame::AllocationSize(media::VideoFrame::I420, coded_size);
+  memcpy(frame_mem, mem, frame_buffer_size);

[wuchengli, 2015/04/15 07:11:59]

Can we import frame_mem to vaapi so we can get rid of this memcpy?

[kcwu, 2015/04/20 17:47:59]

I don't know how. I will ask.

+
+  vaapi_wrapper_->ReturnVaImage(&image);
+
+  if (client_)
+    client_->VideoFrameReady(input_id);

[wuchengli, 2015/04/15 07:11:59]

This should be posted to io thread to reduce latency. See how
v4l2_video_decode_accelerator.cc posts to io thread. V4l2VideoDecodeAccelerator
has a |io_client| parameter and GpuVideoDecodeAccelerator has
|weak_factory_for_io_|.

[kcwu, 2015/04/20 17:47:59]

I don't think this will reduce latency to post this to IO thread. The flow is
eventually passed
from decode thread to IO thread. It doesn't matter that PostTask here or
ChannelProxy Send() PostTask internally.

+
+  return true;
+}
+
+void VaapiJpegDecodeAccelerator::MapAndQueueNewInputBuffer(
+    const media::BitstreamBuffer& bitstream_buffer,
+    const scoped_refptr<media::VideoFrame>& video_frame) {
+  DVLOG(4) << "Mapping new input buffer id: " << bitstream_buffer.id()
+           << " size: " << (int)bitstream_buffer.size();
+
+  scoped_ptr<base::SharedMemory> shm(
+      new base::SharedMemory(bitstream_buffer.handle(), true));
+  RETURN_AND_NOTIFY_ON_FAILURE(shm->Map(bitstream_buffer.size()),
+                               "Failed to map input buffer",
+                               bitstream_buffer.id(), UNREADABLE_INPUT, );
+
+  base::AutoLock auto_lock(lock_);

[wuchengli, 2015/04/15 07:11:59]

This can be moved to line 159.

[kcwu, 2015/04/16 14:38:28]

Done.

+
+  // Set up a new input buffer and queue it for later.
+  linked_ptr<InputBuffer> input_buffer(new InputBuffer());
+  input_buffer->shm.reset(shm.release());
+  input_buffer->id = bitstream_buffer.id();
+  input_buffer->size = bitstream_buffer.size();
+  input_buffer->video_frame = video_frame;
+
+  input_buffers_.push(input_buffer);
+  input_ready_.Signal();
+}
+
+void VaapiJpegDecodeAccelerator::DecodeTask() {
+  DVLOG(3) << __func__;
+  DCHECK(decoder_thread_proxy_->BelongsToCurrentThread());
+  base::AutoLock auto_lock(lock_);
+
+  if (state_ != kIdle) {
+    DLOG(ERROR) << "not idle state";
+    return;
+  }
+
+  state_ = kDecoding;

[wuchengli, 2015/04/15 07:11:59]

Why do we need kDecoding state?

[kcwu, 2015/04/20 17:47:59]

Done.

+
+  DCHECK(!input_buffers_.empty());
+  curr_input_buffer_ = input_buffers_.front();

[wuchengli, 2015/04/15 07:11:59]

curr_input_buffer_ doesn't have to be a class variable.

[kcwu, 2015/04/16 14:38:28]

Done.

+  input_buffers_.pop();
+  DVLOG(4) << "New current bitstream buffer, id: " << curr_input_buffer_->id
+           << " size: " << curr_input_buffer_->size;
+
+  do {
+    base::AutoUnlock auto_unlock(lock_);

[wuchengli, 2015/04/15 07:11:58]

I think adding a code block for line 167-178 and line 222 is clearer.
|auto_unlock| block here can be removed.

[kcwu, 2015/04/20 17:47:59]

Done.

+
+    media::JpegParseResult parse_result;
+
+    if (!media::ParseJpegPicture(
+            reinterpret_cast<const uint8_t*>(curr_input_buffer_->shm->memory()),
+            curr_input_buffer_->size, &parse_result)) {

[wuchengli, 2015/04/15 07:11:59]

add a DLOG(ERROR) 

[kcwu, 2015/04/20 17:47:59]

Done.

+      NotifyError(curr_input_buffer_->id,
+                  media::JpegDecodeAccelerator::PARSE_JPEG_FAILED);
+      break;
+    }
+
+    gfx::Size coded_size(parse_result.frame_header.coded_width,
+                         parse_result.frame_header.coded_height);
+
+    vaapi_wrapper_->DestroySurfaces();

[wuchengli, 2015/04/15 07:11:59]

Do we need this?

[kcwu, 2015/04/16 14:38:28]

Done.

+    std::vector<VASurfaceID> va_surfaces;
+    if (!vaapi_wrapper_->CreateSurfaces(coded_size, 1, &va_surfaces))
+      break;
+
+    if (!VaapiJpegDecoder::Decode(vaapi_wrapper_.get(), parse_result,
+                                  va_surfaces[0])) {
+      DLOG(ERROR) << "Decode failed";

[wuchengli, 2015/04/15 07:11:59]

Can this happen? s/DLOG/LOG/

[kcwu, 2015/04/16 14:38:28]

Done.

+      NotifyError(curr_input_buffer_->id,
+                  media::JpegDecodeAccelerator::PLATFORM_FAILURE);
+      break;
+    }
+
+    if (!OutputPicture(va_surfaces[0], curr_input_buffer_->id,
+                       curr_input_buffer_->video_frame)) {
+      DLOG(ERROR) << "Output failed";

[wuchengli, 2015/04/15 07:11:59]

Can this happen? s/DLOG/LOG/

[kcwu, 2015/04/16 14:38:28]

Done.

+      NotifyError(curr_input_buffer_->id,
+                  media::JpegDecodeAccelerator::PLATFORM_FAILURE);
+      break;
+    }
+    vaapi_wrapper_->DestroySurfaces();
+  } while (0);
+  curr_input_buffer_.reset();
+
+  state_ = kIdle;
+}
+
+void VaapiJpegDecodeAccelerator::Decode(
+    const media::BitstreamBuffer& bitstream_buffer,
+    const scoped_refptr<media::VideoFrame>& video_frame) {
+  DVLOG(3) << __func__;
+  // We got a new input buffer from the client, map it and queue for later use.

[wuchengli, 2015/04/15 07:11:59]

s/client, map/client. Map/

[kcwu, 2015/04/16 14:38:28]

Done.

+  MapAndQueueNewInputBuffer(bitstream_buffer, video_frame);
+
+  base::AutoLock auto_lock(lock_);

[wuchengli, 2015/04/15 07:11:59]

The lock is not required here?

[kcwu, 2015/04/16 14:38:28]

Done.

+  decoder_thread_proxy_->PostTask(
+      FROM_HERE, base::Bind(&VaapiJpegDecodeAccelerator::DecodeTask,
+                            base::Unretained(this)));
+}
+
+void VaapiJpegDecodeAccelerator::Cleanup() {
+  DCHECK_EQ(message_loop_, base::MessageLoop::current());
+
+  base::AutoLock auto_lock(lock_);
+  if (state_ == kUninitialized || state_ == kDestroying)

[wuchengli, 2015/04/15 07:11:59]

Why do we need kDestroying?

[kcwu, 2015/04/20 17:47:59]

Done.

+    return;
+
+  DVLOG(1) << "Destroying VAJDA";

[wuchengli, 2015/04/15 07:11:58]

VAJDA is a uncommon abbreviation. s/VAJDA/VaapiJpegDecodeAccelerator/

[kcwu, 2015/04/16 14:38:28]

Done.

+  state_ = kDestroying;
+
+  client_ptr_factory_.reset();
+  weak_this_factory_.InvalidateWeakPtrs();
+
+  // Signal all potential waiters on the decoder_thread_, let them early-exit,
+  // as we've just moved to the kDestroying state, and wait for all tasks
+  // to finish.
+  input_ready_.Signal();

[wuchengli, 2015/04/15 07:11:58]

Not used. Also remove the comments.

[kcwu, 2015/04/16 14:38:28]

Done.

+  {
+    base::AutoUnlock auto_unlock(lock_);
+    decoder_thread_.Stop();
+  }
+
+  state_ = kUninitialized;
+}
+
+void VaapiJpegDecodeAccelerator::Destroy() {
+  DCHECK_EQ(message_loop_, base::MessageLoop::current());
+  Cleanup();
+  delete this;
+}
+
+}  // namespace content