MJPEG acceleration for video capture using VAAPI, the GPU and IPC part

content/common/gpu/media/vaapi_jpeg_decode_accelerator.cc

Index: content/common/gpu/media/vaapi_jpeg_decode_accelerator.cc
diff --git a/content/common/gpu/media/vaapi_jpeg_decode_accelerator.cc b/content/common/gpu/media/vaapi_jpeg_decode_accelerator.cc
new file mode 100644
index 0000000000000000000000000000000000000000..3d83a32f5a0a6b9ee9923558579b5674f0865322
--- /dev/null
+++ b/content/common/gpu/media/vaapi_jpeg_decode_accelerator.cc
@@ -0,0 +1,255 @@
+// 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/thread_task_runner_handle.h"
+#include "base/trace_event/trace_event.h"
+#include "content/common/gpu/gpu_channel.h"
+#include "content/common/gpu/media/vaapi_picture.h"
+#include "media/base/video_frame.h"
+#include "media/filters/jpeg_parser.h"
+
+static void ReportVaapiError() {

[Pawel Osciak, 2015/05/28 09:13:18]

Please put this inside an anonymous namespace in namespace content {}

[kcwu, 2015/05/28 12:10:27]

Done.

+  // TODO(kcwu) report error to UMA
+}
+
+namespace content {
+
+VaapiJpegDecodeAccelerator::DecodeRequest::DecodeRequest(
+    const media::BitstreamBuffer& bitstream_buffer,
+    scoped_refptr<media::VideoFrame> video_frame)

[Pawel Osciak, 2015/05/28 09:13:18]

const scoped_refptr<>& should be enough

[kcwu, 2015/05/28 12:10:27]

Done.

+    : bitstream_buffer(bitstream_buffer), video_frame(video_frame) {
+}
+
+VaapiJpegDecodeAccelerator::DecodeRequest::~DecodeRequest() {
+}
+
+void VaapiJpegDecodeAccelerator::NotifyError(int32_t bitstream_buffer_id,
+                                             Error error) {
+  DCHECK_EQ(task_runner_, base::ThreadTaskRunnerHandle::Get());
+  DLOG(ERROR) << "Notifying of error " << error;
+  DCHECK(client_);
+  client_->NotifyError(bitstream_buffer_id, error);
+}
+
+void VaapiJpegDecodeAccelerator::NotifyErrorFromDecoderThread(

[Pawel Osciak, 2015/05/28 09:13:17]

Could we have one NotifyError() and do a

void VJDA::NotifyError() {
  if (!decoder_task_runner_->BelongsToCurrentThread()) {
    PostTask...
    return;
  }

  (...)
  client-_>NotifyError()
  (...)
}

?

[kcwu, 2015/05/28 12:10:28]

piman suggest not to do so.
https://chromiumcodereview.appspot.com/1124423008/diff/1/content/common/gpu/m...

+    int32_t bitstream_buffer_id,
+    Error error) {
+  DCHECK(decoder_task_runner_->BelongsToCurrentThread());
+  task_runner_->PostTask(FROM_HERE,
+                         base::Bind(&VaapiJpegDecodeAccelerator::NotifyError,
+                                    weak_this_, bitstream_buffer_id, error));
+}
+
+void VaapiJpegDecodeAccelerator::VideoFrameReady(int32_t bitstream_buffer_id) {
+  DCHECK_EQ(task_runner_, base::ThreadTaskRunnerHandle::Get());

[Pawel Osciak, 2015/05/28 09:13:18]

Would task_runner_->BelongsToCurrentThread() work here?

[kcwu, 2015/05/28 12:10:27]

Why not? This method is posted from OutputPicture.

+  client_->VideoFrameReady(bitstream_buffer_id);
+}
+
+VaapiJpegDecodeAccelerator::VaapiJpegDecodeAccelerator(
+    const scoped_refptr<base::SingleThreadTaskRunner>& io_task_runner)
+    : initialized_(false),
+      task_runner_(base::ThreadTaskRunnerHandle::Get()),
+      io_task_runner_(io_task_runner),
+      decoder_thread_("VaapiJpegDecoderThread"),
+      va_surface_(VA_INVALID_SURFACE),
+      weak_this_factory_(this) {
+  weak_this_ = weak_this_factory_.GetWeakPtr();
+}
+
+VaapiJpegDecodeAccelerator::~VaapiJpegDecodeAccelerator() {
+  DCHECK_EQ(task_runner_, base::ThreadTaskRunnerHandle::Get());
+  Cleanup();
+}
+
+bool VaapiJpegDecodeAccelerator::Initialize(Client* client) {
+  DCHECK_EQ(task_runner_, base::ThreadTaskRunnerHandle::Get());
+
+  client_ = client;
+
+  base::AutoLock auto_lock(lock_);
+  DCHECK(!initialized_);
+
+  vaapi_wrapper_ =
+      VaapiWrapper::Create(VaapiWrapper::kDecode, VAProfileJPEGBaseline,
+                           base::Bind(&ReportVaapiError));
+
+  if (!vaapi_wrapper_.get()) {
+    DLOG(ERROR) << "Failed initializing VAAPI";
+    return false;
+  }
+
+  if (!decoder_thread_.Start()) {
+    DLOG(ERROR) << "Failed to start decoding thread.";
+    return false;
+  }
+  decoder_task_runner_ = decoder_thread_.task_runner();
+
+  initialized_ = true;
+  return true;
+}
+
+bool VaapiJpegDecodeAccelerator::OutputPicture(
+    VASurfaceID va_surface_id,
+    int32_t input_buffer_id,
+    const scoped_refptr<media::VideoFrame>& video_frame) {
+  DCHECK(decoder_task_runner_->BelongsToCurrentThread());
+
+  TRACE_EVENT1("jpeg", "VaapiJpegDecodeAccelerator::OutputPicture",
+               "input_buffer_id", input_buffer_id);
+
+  DVLOG(3) << "Outputting VASurface " << va_surface_id
+           << " into video_frame associated with input buffer id "
+           << input_buffer_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 = nullptr;
+  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);

[Pawel Osciak, 2015/05/28 09:13:18]

We shouldn't assume the same stride and size. We should use libyuv::I420Copy and
use strides from the va_image and frame. We should also probably use
image->image_data + image->image.offsets[plane] to get pointer for each plane,
as well as corresponding methods from VideoFrame.

[kcwu, 2015/06/02 15:07:25]

Done.

+
+  vaapi_wrapper_->ReturnVaImage(&image);
+
+  task_runner_->PostTask(
+      FROM_HERE, base::Bind(&VaapiJpegDecodeAccelerator::VideoFrameReady,
+                            weak_this_, input_buffer_id));
+
+  return true;
+}
+
+void VaapiJpegDecodeAccelerator::DecodeTask() {
+  DVLOG(3) << __func__;
+  DCHECK(decoder_task_runner_->BelongsToCurrentThread());
+  TRACE_EVENT0("jpeg", "DecodeTask");
+  linked_ptr<DecodeRequest> request;
+  {
+    base::AutoLock auto_lock(lock_);
+    if (!initialized_)
+      return;
+
+    DCHECK(!decode_requests_.empty());
+    request = decode_requests_.front();
+    decode_requests_.pop();
+  }
+
+  DVLOG(4) << "Mapping new input buffer id: " << request->bitstream_buffer.id()
+           << " size: " << (int)request->bitstream_buffer.size();

[Pawel Osciak, 2015/05/28 09:13:18]

Please no C-style casts.

[kcwu, 2015/05/28 12:10:28]

Done.

+
+  scoped_ptr<base::SharedMemory> shm(
+      new base::SharedMemory(request->bitstream_buffer.handle(), true));
+  if (!shm->Map(request->bitstream_buffer.size())) {
+    LOG(ERROR) << "Failed to map input buffer";
+    NotifyErrorFromDecoderThread(request->bitstream_buffer.id(),
+                                 UNREADABLE_INPUT);
+    return;
+  }
+
+  media::JpegParseResult parse_result;
+
+  if (!media::ParseJpegPicture(reinterpret_cast<const uint8_t*>(shm->memory()),
+                               request->bitstream_buffer.size(),
+                               &parse_result)) {
+    DLOG(ERROR) << "ParseJpegPicture failed";
+    NotifyErrorFromDecoderThread(
+        request->bitstream_buffer.id(),
+        media::JpegDecodeAccelerator::PARSE_JPEG_FAILED);
+    return;
+  }
+
+  // Reuse VASurface if size doesn't change.
+  gfx::Size coded_size(parse_result.frame_header.coded_width,
+                       parse_result.frame_header.coded_height);
+  if (coded_size != coded_size_ || va_surface_ == VA_INVALID_SURFACE) {
+    vaapi_wrapper_->DestroySurfaces();

[Pawel Osciak, 2015/05/28 09:13:18]

We should va_surface_ = VA_INVALID_SURFACE here in case CreateSurfaces() fails
below.

[kcwu, 2015/05/28 12:10:27]

Done.

+    std::vector<VASurfaceID> va_surfaces;
+    if (!vaapi_wrapper_->CreateSurfaces(coded_size, 1, &va_surfaces)) {
+      LOG(ERROR) << "Create VA surface failed";

[Pawel Osciak, 2015/05/28 09:13:17]

Perhaps we could have a LOG(ERROR) in NotifyError printing out error, instead of
LOG(ERROR) after errors in the code? If we had a macro like this:
https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/m...,
it would also report the line of code from which it happened, so we would know
where exactly we reported error from.

[kcwu, 2015/05/28 12:10:28]

Passing error string as function argument doesn't save much. LOG(ERROR) is one
line, string argument is still one line (the string is not short enough).

I would prefer not to use macro since the benefit is not big enough.
LOG() already can report line number.

+      NotifyErrorFromDecoderThread(
+          request->bitstream_buffer.id(),
+          media::JpegDecodeAccelerator::PLATFORM_FAILURE);
+      return;
+    }
+    va_surface_ = va_surfaces[0];
+    coded_size_ = coded_size;
+  }
+
+  if (!VaapiJpegDecoder::Decode(vaapi_wrapper_.get(), parse_result,
+                                va_surface_)) {
+    LOG(ERROR) << "Decode JPEG failed";
+    NotifyErrorFromDecoderThread(
+        request->bitstream_buffer.id(),
+        media::JpegDecodeAccelerator::PLATFORM_FAILURE);
+    return;
+  }
+
+  if (!OutputPicture(va_surface_, request->bitstream_buffer.id(),
+                     request->video_frame)) {
+    LOG(ERROR) << "Output picture failed";
+    NotifyErrorFromDecoderThread(
+        request->bitstream_buffer.id(),
+        media::JpegDecodeAccelerator::PLATFORM_FAILURE);
+    return;
+  }
+}
+
+void VaapiJpegDecodeAccelerator::Decode(
+    const media::BitstreamBuffer& bitstream_buffer,
+    const scoped_refptr<media::VideoFrame>& video_frame) {
+  DVLOG(3) << __func__;
+  DCHECK(io_task_runner_->BelongsToCurrentThread());
+  TRACE_EVENT1("jpeg", "Decode", "input_id", bitstream_buffer.id());
+
+  base::AutoLock auto_lock(lock_);
+  if (!initialized_)
+    return;
+
+  // Set up a new decode request and queue it for later.
+  linked_ptr<DecodeRequest> input_buffer(

[Pawel Osciak, 2015/05/28 09:13:18]

We could just post this forward to DecodeTask and push on requests queue from
there, we wouldn't have to lock then?

[kcwu, 2015/05/28 12:10:28]

Done.

+      new DecodeRequest(bitstream_buffer, video_frame));
+  decode_requests_.push(input_buffer);
+
+  decoder_task_runner_->PostTask(
+      FROM_HERE, base::Bind(&VaapiJpegDecodeAccelerator::DecodeTask,
+                            base::Unretained(this)));
+}
+
+void VaapiJpegDecodeAccelerator::Cleanup() {
+  DCHECK_EQ(task_runner_, base::ThreadTaskRunnerHandle::Get());
+
+  base::AutoLock auto_lock(lock_);
+  if (!initialized_)
+    return;
+
+  DVLOG(1) << "Destroying VaapiJpegDecodeAccelerator";
+
+  weak_this_factory_.InvalidateWeakPtrs();
+
+  {
+    base::AutoUnlock auto_unlock(lock_);
+    decoder_thread_.Stop();
+  }
+
+  initialized_ = false;
+}
+
+}  // namespace content