New issue
Advanced search Search tips

Issue 792790 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Feature



Sign in to add a comment

Factorize common code in V4L2SliceVideoDecodeAccelerator and V4L2VideoDecodeAccelerator

Project Member Reported by acourbot@chromium.org, Dec 7 2017

Issue description

Although V4L2SliceVideoDecodeAccelerator and V4L2VideoDecodeAccelerator are two difference accelerator classes, they share a lot of common V4L2 logic. This bug is to track the merging of this common code into a shared parent class.

While we are at it, it would be a good idea to move these classes into media/gpu/v4l2 similarly to what has been done with vaapi classes.
 
Labels: -videoshortlist
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 11 2017

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

commit fb70f64b2df7ca4ff01054af3e5acaec99d269cd
Author: Alexandre Courbot <acourbot@chromium.org>
Date: Mon Dec 11 09:19:32 2017

media/gpu: move V4L2-related files to their own sub-directory

This organizes the code in a more logical way, and makes it easier to
assign owners to V4L2.

BUG=chromium:792790
TEST=made sure Chromium built successfully for Chrome OS, on both x86
and ARM platforms

Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Ib6189acb9c054b20c7b83474347ffc2e160436fc
Reviewed-on: https://chromium-review.googlesource.com/813438
Reviewed-by: Kuang-che Wu <kcwu@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523062}
[modify] https://crrev.com/fb70f64b2df7ca4ff01054af3e5acaec99d269cd/media/gpu/BUILD.gn
[modify] https://crrev.com/fb70f64b2df7ca4ff01054af3e5acaec99d269cd/media/gpu/gpu_jpeg_decode_accelerator_factory.cc
[modify] https://crrev.com/fb70f64b2df7ca4ff01054af3e5acaec99d269cd/media/gpu/gpu_video_decode_accelerator_factory.cc
[modify] https://crrev.com/fb70f64b2df7ca4ff01054af3e5acaec99d269cd/media/gpu/gpu_video_encode_accelerator_factory.cc
[rename] https://crrev.com/fb70f64b2df7ca4ff01054af3e5acaec99d269cd/media/gpu/v4l2/generic_v4l2_device.cc
[rename] https://crrev.com/fb70f64b2df7ca4ff01054af3e5acaec99d269cd/media/gpu/v4l2/generic_v4l2_device.h
[rename] https://crrev.com/fb70f64b2df7ca4ff01054af3e5acaec99d269cd/media/gpu/v4l2/tegra_v4l2_device.cc
[rename] https://crrev.com/fb70f64b2df7ca4ff01054af3e5acaec99d269cd/media/gpu/v4l2/tegra_v4l2_device.h
[rename] https://crrev.com/fb70f64b2df7ca4ff01054af3e5acaec99d269cd/media/gpu/v4l2/v4l2_device.cc
[rename] https://crrev.com/fb70f64b2df7ca4ff01054af3e5acaec99d269cd/media/gpu/v4l2/v4l2_device.h
[rename] https://crrev.com/fb70f64b2df7ca4ff01054af3e5acaec99d269cd/media/gpu/v4l2/v4l2_image_processor.cc
[rename] https://crrev.com/fb70f64b2df7ca4ff01054af3e5acaec99d269cd/media/gpu/v4l2/v4l2_image_processor.h
[rename] https://crrev.com/fb70f64b2df7ca4ff01054af3e5acaec99d269cd/media/gpu/v4l2/v4l2_jpeg_decode_accelerator.cc
[rename] https://crrev.com/fb70f64b2df7ca4ff01054af3e5acaec99d269cd/media/gpu/v4l2/v4l2_jpeg_decode_accelerator.h
[rename] https://crrev.com/fb70f64b2df7ca4ff01054af3e5acaec99d269cd/media/gpu/v4l2/v4l2_slice_video_decode_accelerator.cc
[rename] https://crrev.com/fb70f64b2df7ca4ff01054af3e5acaec99d269cd/media/gpu/v4l2/v4l2_slice_video_decode_accelerator.h
[rename] https://crrev.com/fb70f64b2df7ca4ff01054af3e5acaec99d269cd/media/gpu/v4l2/v4l2_video_decode_accelerator.cc
[rename] https://crrev.com/fb70f64b2df7ca4ff01054af3e5acaec99d269cd/media/gpu/v4l2/v4l2_video_decode_accelerator.h
[rename] https://crrev.com/fb70f64b2df7ca4ff01054af3e5acaec99d269cd/media/gpu/v4l2/v4l2_video_encode_accelerator.cc
[rename] https://crrev.com/fb70f64b2df7ca4ff01054af3e5acaec99d269cd/media/gpu/v4l2/v4l2_video_encode_accelerator.h

Project Member

Comment 3 by bugdroid1@chromium.org, Mar 29 2018

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

commit fccfac8573b3e64dc91b7404020f7c2f3de7ffec
Author: Alexandre Courbot <acourbot@chromium.org>
Date: Thu Mar 29 02:55:04 2018

v4l2: convert linked_ptr to unique_ptr

The V4L2 VDAs were still using the deprecated linked_ptr where unique_ptr
ought to be used. This patch performs the conversion.

BUG=792790
TEST=Checked that video_VDAPerf was still passing on hana (v4l2_vda) and kevin (v4l2_slice_vda)

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Ic3290d9ae5fa5fa2630cbcc5a35eda93e1cd03ca
Reviewed-on: https://chromium-review.googlesource.com/983316
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Kuang-che Wu <kcwu@chromium.org>
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546714}
[modify] https://crrev.com/fccfac8573b3e64dc91b7404020f7c2f3de7ffec/media/gpu/v4l2/v4l2_slice_video_decode_accelerator.cc
[modify] https://crrev.com/fccfac8573b3e64dc91b7404020f7c2f3de7ffec/media/gpu/v4l2/v4l2_slice_video_decode_accelerator.h
[modify] https://crrev.com/fccfac8573b3e64dc91b7404020f7c2f3de7ffec/media/gpu/v4l2/v4l2_video_decode_accelerator.cc
[modify] https://crrev.com/fccfac8573b3e64dc91b7404020f7c2f3de7ffec/media/gpu/v4l2/v4l2_video_decode_accelerator.h

Comment 4 by hiroh@chromium.org, Apr 11 2018

Cc: hiroh@chromium.org
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 29

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

commit 71a96673aee3d4f2945504d915ac8481e1c68be8
Author: Alexandre Courbot <acourbot@chromium.org>
Date: Wed Aug 29 07:38:21 2018

media: make VideoFrame::DmabufFds() returned borrowed ScopedFDs

The current interface of VideoFrame::DmabufFds() returns ints, which may
confuse users as to who owns the FDs. Change it to return a const
reference to its own vector of ScopedFDs, to make it explicit that the
FDs are borrowed and require users to use media::DuplicateFDs() in order
to make their own copy.

BUG=792790
TEST=Made sure that VDA test and VEA tests were passing on Hana.

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I387ed8dbb67777035f44241803dbd09a4f37db2a
Reviewed-on: https://chromium-review.googlesource.com/1170705
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587032}
[modify] https://crrev.com/71a96673aee3d4f2945504d915ac8481e1c68be8/media/base/video_frame.cc
[modify] https://crrev.com/71a96673aee3d4f2945504d915ac8481e1c68be8/media/base/video_frame.h
[modify] https://crrev.com/71a96673aee3d4f2945504d915ac8481e1c68be8/media/gpu/v4l2/v4l2_image_processor.cc
[modify] https://crrev.com/71a96673aee3d4f2945504d915ac8481e1c68be8/media/gpu/v4l2/v4l2_video_encode_accelerator.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Sep 28

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

commit 660e3c779d0d0c464fb5b7a05bbfcd233946f039
Author: Alexandre Courbot <acourbot@chromium.org>
Date: Fri Sep 28 07:58:23 2018

media/gpu/v4l2: add queue management classes

Add the V4L2Queue class, that manages buffers allocations and queuing.
Users of this class don't have to manage the set of free buffers
themselves, which will help factorizing a considerable amount of code
and reduce the potential for bugs.

Users of V4L2Queue can obtain a unique reference to a free buffer that
they can prepare and queue. Dequeued buffers come in the form of a
refcounted buffer instance, which puts the V4L2 buffer back into the
list of free buffer as it is destroyed. This greatly simplifies V4L2
buffers lifecycle, which is now managed by short functions which
invariants can be precisely enforced.

BUG=792790
TEST=Make sure VDA test was compiling and running

Change-Id: I7b4724fbc444c4cd8ed077617a1aaadcbe4c8011
Reviewed-on: https://chromium-review.googlesource.com/1170706
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Pawel Osciak <posciak@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595014}
[modify] https://crrev.com/660e3c779d0d0c464fb5b7a05bbfcd233946f039/media/gpu/v4l2/v4l2_device.cc
[modify] https://crrev.com/660e3c779d0d0c464fb5b7a05bbfcd233946f039/media/gpu/v4l2/v4l2_device.h

Project Member

Comment 7 by bugdroid1@chromium.org, Oct 1

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

commit 7041363ffacdd76aba828f60cb29dce6ecb9a9bf
Author: Alexandre Courbot <acourbot@chromium.org>
Date: Mon Oct 01 02:45:31 2018

media/gpu/v4l2vda: use the V4L2Queue class

Convert the V4L2VideoDecodeAccelerator class to use the V4L2Queue. This
considerably reduces its amount of code, while also making buffers
lifecycle safer.

BUG=792790
TEST=Made sure that VDA unittest was compiling and running on Hana

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

Project Member

Comment 8 by bugdroid1@chromium.org, Oct 11

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

commit 713c48f427528dac5cb4afa80b9d43da4b131fc4
Author: Alexandre Courbot <acourbot@chromium.org>
Date: Thu Oct 11 07:52:17 2018

media/gpu/v4l2: factorize code for sending picture to client

We can send pictures to clients from two situations: when dequeuing a
capture buffer if the image processor is not used, or when the image
processor is done with a frame. The code for these two cases is
identical, so factorize it.

BUG=792790
TEST=Checked that VDA was passing on hana.

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I18aee0e2ea98c80da98e3e507c018bb8960572b1
Reviewed-on: https://chromium-review.googlesource.com/c/1264340
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Kuang-che Wu <kcwu@chromium.org>
Reviewed-by: Pawel Osciak <posciak@chromium.org>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598695}
[modify] https://crrev.com/713c48f427528dac5cb4afa80b9d43da4b131fc4/media/gpu/v4l2/v4l2_video_decode_accelerator.cc
[modify] https://crrev.com/713c48f427528dac5cb4afa80b9d43da4b131fc4/media/gpu/v4l2/v4l2_video_decode_accelerator.h

Project Member

Comment 9 by bugdroid1@chromium.org, Oct 23

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

commit 42a63f2bc8066b20bd44aaaff34cc1b76e7f0685
Author: Alexandre Courbot <acourbot@chromium.org>
Date: Tue Oct 23 08:29:45 2018

media/gpu/v4l2vda: Assign output_fds earlier

When using import mode, we used to assign output_fds last, during
the call to AssignEGLImage(). However this can also be done ahead of
time in ImportBufferForPictureTask(). Doing so allows us to stop passing
the DMABUF FDs as an argument to AssignEGLImage(), simplifying its
interface as well as the code in ImportBufferForPictureTask().

BUG=792790
TEST=Checked that VDA unittest was passing on Hana, in import and
non-import mode, with rendering enabled or not.

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I95b66f7b3b4ee31147b2eb34a19499c39ad4efa8
Reviewed-on: https://chromium-review.googlesource.com/c/1288497
Reviewed-by: Pawel Osciak <posciak@chromium.org>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601882}
[modify] https://crrev.com/42a63f2bc8066b20bd44aaaff34cc1b76e7f0685/media/gpu/v4l2/v4l2_video_decode_accelerator.cc
[modify] https://crrev.com/42a63f2bc8066b20bd44aaaff34cc1b76e7f0685/media/gpu/v4l2/v4l2_video_decode_accelerator.h

Project Member

Comment 10 by bugdroid1@chromium.org, Nov 13

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

commit 64cc5178609066005164870dbbf531f276806e4d
Author: Alexandre Courbot <acourbot@chromium.org>
Date: Tue Nov 13 04:23:16 2018

media/gpu/v4l2: lazy-assign EGLImages to image processor buffers

Right now the VDA and IP devices are tightly coupled: notably, their
buffers follow a 1:1 mapping, and the VDA decides which IP processor
buffer to use. This seriously restricts the applicability of the IP
interface.

In order to remove this constraint, the VDA must not make any assumption
about which IP buffer it will receive, the number of buffers it will use,
or even their memory type. Doing so means that the VDA will see the IP
buffers for the first time as the FrameProcessed() callback is called,
and thus that it cannot create an EGLImage for them ahead of time as we
used to do.

This patch thus delays EGLImage assignment to the first time we meet a
given buffer in the FrameProcessed() callback. Thanks to the fact that
every picture needs to be cleared on the child thread first, we can
schedule the EGLImage creation on the same thread before this happens,
thus ensuring that no synchronization issue occur.

BUG=792790
TEST=Checked that VDA unittest was passing on Hana, in import and
non-import mode, with rendering enabled or not.

Change-Id: Ia89b7620fe890fcd4bbd6791fd59660d9bd89f46
Reviewed-on: https://chromium-review.googlesource.com/c/1288498
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607484}
[modify] https://crrev.com/64cc5178609066005164870dbbf531f276806e4d/media/gpu/v4l2/v4l2_video_decode_accelerator.cc

Sign in to add a comment