New issue
Advanced search Search tips

Issue 850375 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Yesterday
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Feature



Sign in to add a comment

Support for non-V4L2 image processors

Project Member Reported by acourbot@chromium.org, Jun 7 2018

Issue description

Currently image processor (IP) support is limited to V4L2 IPs working with V4L2 encoders or decoders. There is however a need for other IPs using GPUs or even pure software conversion.

This bug tracks enabling support for such IPs. Several steps are required:

1) Extract the useful interface of V4L2ImageProcessor into a common, shared ImageProcessor interface class that IPs can implement. An IP should take a VideoFrame as input, and provide another VideoFrame as output.

2) Implement non-V4L2 IPs.

3) Create an ImageProcessorFactory class that will return the optimal ImageProcessor instance for a requested conversion.
 
Cc: posciak@chromium.org
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 13 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c4e3005fdd7b57585be9e68e149964c5a1ef936e

commit c4e3005fdd7b57585be9e68e149964c5a1ef936e
Author: Alexandre Courbot <acourbot@chromium.org>
Date: Wed Jun 13 07:46:35 2018

media: use ScopedFD with VideoFrame

DMA-BUF support in VideoFrame was suffering from several issues:

* The number of DMA-BUF FDs provided to back a frame was assumed to be
equal to the number of planes of the format of the frame. This is not
always correct, since several planes can be backed by the same buffer,

* FDs were always duplicated when creating a new VideoFrame - there was
no option to make it take ownership of the passed FDs.

* The class' public interface was taking FDs as ints instead of the more
secure ScopedFD.

This patch fixes these issues by doing the following:

* Make VideoFrame::WrapExternalDmabufs() take a vector of ScopedFDs
instead of a vector of integers, make it take ownership of the
passed FDs, and make it accept a smaller number of FDs than there are
planes in the frame's format,

* Turn VideoFrame::DmabufFd() into VideoFrame::DmabufFds(), which
returns all the FDs associated to a frame.

* Introduce the media::DuplicateFDs() function which can be used by
callers to explicitly duplicate a vector of FDs (a rather common thing
to do).

* Update users of VideoFrame::WrapExternalDmabufs() (concentrated in
media/gpu/v4l2) to make use of this updated interface.

BUG= 850375 
BUG=b:73752373
TEST=Tested VDA unittest and video playback on Hana (decoder + IP), and
VEA unittest and resulting video playback on pit (IP + encoder).

Change-Id: I9318fb3be82e3ff50c13c17dfd5faf8f4b5fd419
Reviewed-on: https://chromium-review.googlesource.com/1090416
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Pawel Osciak <posciak@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566752}
[modify] https://crrev.com/c4e3005fdd7b57585be9e68e149964c5a1ef936e/media/base/BUILD.gn
[add] https://crrev.com/c4e3005fdd7b57585be9e68e149964c5a1ef936e/media/base/scopedfd_helper.cc
[add] https://crrev.com/c4e3005fdd7b57585be9e68e149964c5a1ef936e/media/base/scopedfd_helper.h
[modify] https://crrev.com/c4e3005fdd7b57585be9e68e149964c5a1ef936e/media/base/video_frame.cc
[modify] https://crrev.com/c4e3005fdd7b57585be9e68e149964c5a1ef936e/media/base/video_frame.h
[modify] https://crrev.com/c4e3005fdd7b57585be9e68e149964c5a1ef936e/media/gpu/v4l2/v4l2_image_processor.cc
[modify] https://crrev.com/c4e3005fdd7b57585be9e68e149964c5a1ef936e/media/gpu/v4l2/v4l2_video_decode_accelerator.cc
[modify] https://crrev.com/c4e3005fdd7b57585be9e68e149964c5a1ef936e/media/gpu/v4l2/v4l2_video_encode_accelerator.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Jun 15 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d61a1156b0cf8a04f2a03aedd005a2e0cc6d2e56

commit d61a1156b0cf8a04f2a03aedd005a2e0cc6d2e56
Author: Alexandre Courbot <acourbot@chromium.org>
Date: Fri Jun 15 05:03:09 2018

media/gpu/v4l2: make image processor output VideoFrames

Although V4L2ImageProcessor takes VideoFrames as input, the output that
is passed to the completion callback was just an output buffer index. It
was the reponsibility of the client to obtain the backing memory of this
buffer by dedicated means. So far the only way to do this was to call
the GetDmabufsForOutputBuffer() method, which is DMA-BUF specific and
non-portable.

This patch makes the completion callback take a VideoFrame representing
the output buffer as as argument. That way, the image processor can
construct a VideoFrame with its backing memory of choice, and clients
are no more limited to DMA-BUF.

This allows us to get rid of the non-portable
GetDmabufsForOutputBuffer(), and to extract a common, non-V4L2 specific
ImageProcessor interface in a future patch.

BUG= 850375 
BUG=b:73752373
TEST=Tested VDA unittest and video playback on Hana (decoder + IP), and
VEA unittest and resulting video playback on pit (IP + encoder).

Change-Id: I97eed0c1e831a8b2bee7dae763e1de7e280b7c40
Reviewed-on: https://chromium-review.googlesource.com/1090417
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Pawel Osciak <posciak@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567552}
[modify] https://crrev.com/d61a1156b0cf8a04f2a03aedd005a2e0cc6d2e56/media/gpu/v4l2/v4l2_image_processor.cc
[modify] https://crrev.com/d61a1156b0cf8a04f2a03aedd005a2e0cc6d2e56/media/gpu/v4l2/v4l2_image_processor.h
[modify] https://crrev.com/d61a1156b0cf8a04f2a03aedd005a2e0cc6d2e56/media/gpu/v4l2/v4l2_video_decode_accelerator.cc
[modify] https://crrev.com/d61a1156b0cf8a04f2a03aedd005a2e0cc6d2e56/media/gpu/v4l2/v4l2_video_decode_accelerator.h
[modify] https://crrev.com/d61a1156b0cf8a04f2a03aedd005a2e0cc6d2e56/media/gpu/v4l2/v4l2_video_encode_accelerator.cc
[modify] https://crrev.com/d61a1156b0cf8a04f2a03aedd005a2e0cc6d2e56/media/gpu/v4l2/v4l2_video_encode_accelerator.h

Project Member

Comment 4 by bugdroid1@chromium.org, Jun 21 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/213fadfab61b6ff4d7f82071ec85995cc123acb2

commit 213fadfab61b6ff4d7f82071ec85995cc123acb2
Author: Alexandre Courbot <acourbot@chromium.org>
Date: Thu Jun 21 05:42:21 2018

media/gpu: extract image processor interface

Extract the useful interface from the V4L2ImageProcessor into a generic
interface that can be used for other kinds of processors and codecs.
Update the V4L2 encoder and decoder to use this more generic interface.

Also convert one instance of Callback into OnceCallback in the image
processor.

BUG= 850375 
BUG=b:73752373
TEST=Checked both VDA and VEA unittests on Hana.

Change-Id: Ia40a3989717f3cb03711b13a2ec574e018d2c614
Reviewed-on: https://chromium-review.googlesource.com/1084364
Reviewed-by: Kuang-che Wu <kcwu@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569165}
[modify] https://crrev.com/213fadfab61b6ff4d7f82071ec85995cc123acb2/media/gpu/BUILD.gn
[add] https://crrev.com/213fadfab61b6ff4d7f82071ec85995cc123acb2/media/gpu/image_processor.h
[modify] https://crrev.com/213fadfab61b6ff4d7f82071ec85995cc123acb2/media/gpu/v4l2/v4l2_image_processor.cc
[modify] https://crrev.com/213fadfab61b6ff4d7f82071ec85995cc123acb2/media/gpu/v4l2/v4l2_image_processor.h
[modify] https://crrev.com/213fadfab61b6ff4d7f82071ec85995cc123acb2/media/gpu/v4l2/v4l2_video_decode_accelerator.cc
[modify] https://crrev.com/213fadfab61b6ff4d7f82071ec85995cc123acb2/media/gpu/v4l2/v4l2_video_decode_accelerator.h
[modify] https://crrev.com/213fadfab61b6ff4d7f82071ec85995cc123acb2/media/gpu/v4l2/v4l2_video_encode_accelerator.cc
[modify] https://crrev.com/213fadfab61b6ff4d7f82071ec85995cc123acb2/media/gpu/v4l2/v4l2_video_encode_accelerator.h

Comment 5 by acourbot@chromium.org, Yesterday (34 hours ago)

Status: Verified (was: Started)
This has been completed with the merging of LibYUVImageProcessor and the image processor factory - closing.

Sign in to add a comment