MJPEG acceleration for video capture, browser part

content/browser/renderer_host/gpu_jpeg_decoder.cc

Index: content/browser/renderer_host/gpu_jpeg_decoder.cc
diff --git a/content/browser/renderer_host/gpu_jpeg_decoder.cc b/content/browser/renderer_host/gpu_jpeg_decoder.cc
new file mode 100644
index 0000000000000000000000000000000000000000..e56bf6726f6f813866efb410d5b005c2c482a844
--- /dev/null
+++ b/content/browser/renderer_host/gpu_jpeg_decoder.cc
@@ -0,0 +1,212 @@
+// 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/browser/renderer_host/gpu_jpeg_decoder.h"
+
+#include "base/bind.h"
+#include "base/logging.h"
+#include "base/single_thread_task_runner.h"
+#include "base/strings/stringprintf.h"
+#include "base/thread_task_runner_handle.h"
+#include "base/trace_event/trace_event.h"
+#include "content/browser/gpu/browser_gpu_channel_host_factory.h"
+#include "content/common/gpu/client/gpu_jpeg_decode_accelerator_host.h"
+#include "content/public/browser/browser_thread.h"
+#include "media/base/video_frame.h"
+
+namespace content {
+
+// static
+bool GpuJpegDecoder::Supported() {
+  // A lightweight check for caller to avoid IPC latency for known unsupported
+  // platform. Initialize() can do the real platform supporting check but it
+  // requires an IPC even for platforms that do not support HW decoder.
+  // TODO(kcwu): move this information to GpuInfo.

[Pawel Osciak, 2015/06/22 08:51:23]

Please add a crbug for this.

[kcwu, 2015/06/23 13:58:24]

Done.

+  return false;

[Pawel Osciak, 2015/06/22 08:51:22]

Are you planning to use a flag here for now?

[kcwu, 2015/06/22 09:18:48]

Yes, there is another CL https://codereview.chromium.org/1153163003/

+}
+
+GpuJpegDecoder::GpuJpegDecoder(const DecodeDoneCB& decode_done_cb,
+                               const ErrorCB& error_cb)
+    : decode_done_cb_(decode_done_cb),
+      error_cb_(error_cb),
+      next_bitstream_buffer_id_(0),
+      in_buffer_(media::JpegDecodeAccelerator::kInvalidBitstreamBufferId,
+                 base::SharedMemory::NULLHandle(),
+                 0) {
+}
+
+GpuJpegDecoder::~GpuJpegDecoder() {
+  DCHECK(CalledOnValidThread());
+
+  // |decoder_| guarantees no more JpegDecodeAccelerator::Client callbacks
+  // on IO thread after deletion.
+  decoder_.reset();
+
+  // |gpu_channel_host_| should outlive |decoder_|, so |gpu_channel_host_|
+  // must be released after |decoder_| is been destroyed.

[Pawel Osciak, 2015/06/22 08:51:22]

s/is/has/

[kcwu, 2015/06/23 13:58:24]

Done.

+  gpu_channel_host_ = nullptr;
+}
+
+bool GpuJpegDecoder::IsDecoding_Locked() {
+  lock_.AssertAcquired();
+  return !decode_done_closure_.is_null();
+}
+
+void GpuJpegDecoder::Initialize() {
+  DVLOG(3) << __func__;
+  DCHECK(CalledOnValidThread());
+
+  const scoped_refptr<base::SingleThreadTaskRunner> current_task_runner(
+      base::ThreadTaskRunnerHandle::Get());
+  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                          base::Bind(&EstablishGpuChannelOnUIThread,
+                                     current_task_runner, AsWeakPtr()));
+}
+
+// static
+void GpuJpegDecoder::EstablishGpuChannelOnUIThread(
+    const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
+    base::WeakPtr<GpuJpegDecoder> weak_this) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(BrowserGpuChannelHostFactory::instance());
+
+  BrowserGpuChannelHostFactory::instance()->EstablishGpuChannel(
+      CAUSE_FOR_GPU_LAUNCH_JPEGDECODEACCELERATOR_INITIALIZE,
+      base::Bind(&GpuJpegDecoder::GpuChannelEstablishedOnUIThread, task_runner,
+                 weak_this));
+}
+
+// static
+void GpuJpegDecoder::GpuChannelEstablishedOnUIThread(
+    const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
+    base::WeakPtr<GpuJpegDecoder> weak_this) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  scoped_refptr<GpuChannelHost> gpu_channel_host(
+      BrowserGpuChannelHostFactory::instance()->GetGpuChannel());
+  task_runner->PostTask(FROM_HERE,
+                        base::Bind(&GpuJpegDecoder::InitializeDone, weak_this,
+                                   base::Passed(&gpu_channel_host)));
+}
+
+void GpuJpegDecoder::InitializeDone(
+    scoped_refptr<GpuChannelHost> gpu_channel_host) {
+  DCHECK(CalledOnValidThread());
+  if (!gpu_channel_host) {
+    LOG(ERROR) << "Failed to establish GPU channel for JPEG decoder";
+    return;
+  }
+  gpu_channel_host_ = gpu_channel_host.Pass();
+
+  decoder_ = gpu_channel_host_->CreateJpegDecoder(this);
+}
+
+void GpuJpegDecoder::VideoFrameReady(int32_t bitstream_buffer_id) {
+  DVLOG(3) << __func__;
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  TRACE_EVENT0("jpeg", "GpuJpegDecoder::VideoFrameReady");
+  base::AutoLock lock(lock_);
+
+  if (!IsDecoding_Locked()) {
+    LOG(ERROR) << "Got decode response while not decoding";
+    return;
+  }
+
+  if (bitstream_buffer_id != in_buffer_.id()) {
+    LOG(ERROR) << "Unexpected bitstream_buffer_id " << bitstream_buffer_id

[Pawel Osciak, 2015/06/22 08:51:23]

Should we just store the id, instead of the whole in_buffer_, if we are not
using it? Or even just use next_bitstream_buffer_id_-1 if we don't allow more
than one decode?

[kcwu, 2015/06/23 13:58:24]

Replace in_buffer_ by in_buffer_id_. I don't use next_bitstream_buffer_id_-1 to
avoid handling the overflow case.

+               << ", expected " << in_buffer_.id();
+    return;
+  }

[Pawel Osciak, 2015/06/22 08:51:22]

Should we clear in_buffer_ here?

[kcwu, 2015/06/23 13:58:24]

Done.

+
+  decode_done_closure_.Run();
+  decode_done_closure_.Reset();
+
+  TRACE_EVENT_ASYNC_END0("jpeg", "GpuJpegDecoder decoding",
+                         bitstream_buffer_id);
+}
+
+void GpuJpegDecoder::NotifyError(int32_t bitstream_buffer_id,
+                                 media::JpegDecodeAccelerator::Error error) {
+  DVLOG(3) << __func__;
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  LOG(ERROR) << "Decode error, bitstream_buffer_id=" << bitstream_buffer_id;

[Pawel Osciak, 2015/06/22 08:51:23]

Should we print error here?

[kcwu, 2015/06/23 13:58:24]

Done.

+
+  base::AutoLock lock(lock_);
+  decode_done_closure_.Reset();
+
+  error_cb_.Run(
+      base::StringPrintf("Decode error, bitstream_buffer_id=%d, error=%d",

[Pawel Osciak, 2015/06/22 08:51:23]

s/Decode/JPEG Decode/

[kcwu, 2015/06/23 13:58:24]

Done.

+                         bitstream_buffer_id, error));
+}
+
+void GpuJpegDecoder::DecodeCapturedData(
+    const uint8* data,
+    size_t in_buffer_size,
+    const media::VideoCaptureFormat& frame_format,
+    const base::TimeTicks& timestamp,
+    scoped_ptr<media::VideoCaptureDevice::Client::Buffer> out_buffer) {
+  DCHECK(CalledOnValidThread());
+  DCHECK(decoder_);
+
+  TRACE_EVENT_ASYNC_BEGIN0("jpeg", "GpuJpegDecoder decoding",
+                           next_bitstream_buffer_id_);
+  TRACE_EVENT0("jpeg", "GpuJpegDecoder::DecodeCapturedData");
+
+  // TODO(kcwu): enqueue decode requests in case decoding is not fast enough
+  // (say, if decoding time is longer than 16ms for 60fps 4k image)

[Pawel Osciak, 2015/06/22 08:51:23]

I'd remove this, since 16ms would probably be too slow anyway, as decode is just
a part of the pipeline?

[kcwu, 2015/06/22 09:18:48]

Do you mean remove only line 157 or the whole TODO? Maybe the throughput is
enough, just high latency, so it is probably okay.

+  {
+    base::AutoLock lock(lock_);
+    if (IsDecoding_Locked()) {
+      DVLOG(1) << "Drop captured frame. Previous jpeg frame is still decoding";
+      return;
+    }
+  }
+
+  // Enlarge input buffer if necessary.

[Pawel Osciak, 2015/06/22 08:51:22]

Why do we need a larger input buffer than the input data?

[kcwu, 2015/06/22 09:18:48]

We are reusing the shared memory and want to avoid reallocations (as comment in
line 169). Does this answer your question?

+  if (!in_shared_memory_.get() ||
+      in_buffer_size > in_shared_memory_->mapped_size()) {
+    // Reserve 2x space to avoid frequent reallocations for initial frames.
+    const size_t reserved_size = 2 * in_buffer_size;
+    in_shared_memory_.reset(new base::SharedMemory);
+    if (!in_shared_memory_->CreateAndMapAnonymous(reserved_size)) {
+      base::AutoLock lock(lock_);
+      error_cb_.Run(base::StringPrintf("CreateAndMapAnonymous failed, size=%zd",
+                                       reserved_size));
+      return;
+    }
+  }
+  memcpy(in_shared_memory_->memory(), data, in_buffer_size);
+
+  // No need to lock for |in_buffer_| since IsDecoding_Locked() is false.
+  in_buffer_ = media::BitstreamBuffer(
+      next_bitstream_buffer_id_, in_shared_memory_->handle(), in_buffer_size);
+  // Mask against 30 bits, to avoid (undefined) wraparound on signed integer.
+  next_bitstream_buffer_id_ = (next_bitstream_buffer_id_ + 1) & 0x3FFFFFFF;
+
+  const gfx::Size dimensions = frame_format.frame_size;
+  base::SharedMemoryHandle out_handle(out_buffer->AsPlatformFile(), true);

[kcwu, 2015/06/22 12:59:35]

I found AsPlatformFile() become only available on OS_POSIX recently
crrev.com/335139.
@mcasas, what do you suggest to get SharedMemoryHandle on non-posix platform?

[wuchengli, 2015/06/25 07:42:27]

mcasas had no time to reply this week. Since other platforms do not support
HW JPEG decode anyway, let's add a #if defined(OS_POSIX) and merge the CL.

[kcwu, 2015/06/25 14:25:47]

Done.

+  scoped_refptr<media::VideoFrame> out_frame =
+      media::VideoFrame::WrapExternalSharedMemory(
+          media::VideoFrame::I420,                  // format

[Pawel Osciak, 2015/06/22 08:51:23]

VideoCaptureFormat has a pixel_format member. Could we use that instead (and
error out on something not supported)?

[kcwu, 2015/06/22 09:18:48]

Do you mean the argument |frame_format| ? If so, no, |frame_format.pixel_format|
is media::PIXEL_FORMAT_MJPEG

+          dimensions,                               // coded_size
+          gfx::Rect(dimensions),                    // visible_rect
+          dimensions,                               // natural_size
+          static_cast<uint8*>(out_buffer->data()),  // data

[Pawel Osciak, 2015/06/22 08:51:23]

s/uint8/uin8_t/

[kcwu, 2015/06/23 13:58:24]

Done.

+          out_buffer->size(),                       // data_size
+          out_handle,                               // handle
+          0,                                        // shared_memory_offset
+          base::TimeDelta());                       // timestamp

[Pawel Osciak, 2015/06/22 08:51:23]

Should we use timestamp from this method's arguments? Is the timestamp
propagated properly right now to the client?

[kcwu, 2015/06/22 09:18:48]

The timestamp is propagated separatedly. VideoFrame.timestamp is not used. And
their type mismatch (one is TimeDelta, the other is TimeTicks)

+  DCHECK(out_frame.get());

[Pawel Osciak, 2015/06/22 08:51:23]

I think we should handle this, instead of just DCHECKing.

[kcwu, 2015/06/23 13:58:24]

Done.

+  out_frame->metadata()->SetDouble(media::VideoFrameMetadata::FRAME_RATE,
+                                   frame_format.frame_rate);
+
+  {
+    base::AutoLock lock(lock_);
+    decode_done_closure_ = base::Bind(
+        decode_done_cb_, base::Passed(&out_buffer), out_frame, timestamp);
+  }
+  decoder_->Decode(in_buffer_, out_frame);
+}
+
+}  // namespace content