Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2958213002:
V4L2 VDA/VEA: Use VIDIOC_G/S_SELECTION for accelerators (Closed)
DescriptionV4L2 VDA/VEA: Use VIDIOC_G/S_SELECTION for codec drivers
Applications have to use the selection API to work with codec drivers.
The old crop ioctls are not suitable for such hardware.
For backward compatibility, we may fallback to use old crop APIs if the
codec driver doesn't support selection interfaces.
BUG=615857
TEST==Run VDA/VEA tests on elm.
on Rowan for selection path
on peach_pi for fallback crop path.
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
Review-Url: https://codereview.chromium.org/2958213002
Cr-Commit-Position: refs/heads/master@{#489265}
Committed: https://chromium.googlesource.com/chromium/src/+/92c29f34ff9ad157f94b6f1a8f67a795c9deb439
Patch Set 1 #
Total comments: 6
Patch Set 2 : V4L2 VDA/VEA: Use VIDIOC_G/S_SELECTION for accelerators #Patch Set 3 : V4L2 VDA/VEA: Use VIDIOC_G/S_SELECTION for codec drivers #
Total comments: 2
Patch Set 4 : V4L2 VDA/VEA: Use VIDIOC_G/S_SELECTION for codec drivers #
Total comments: 1
Patch Set 5 : V4L2 VDA/VEA: Use VIDIOC_G/S_SELECTION for codec drivers #Messages
Total messages: 23 (9 generated)
Description was changed from ========== V4L2 VDA/VEA: Use VIDIOC_G/S_SELECTION for accelerators Applications have to use the selection API to work with codec drivers. The old crop ioctls are not suitable for such hardware. For backward compatibility we may fallback to use old crop APIs if the codec driver doesn't support selection interfaces. BUG=615857 TEST=Run VDA/VEA tests on elm. ========== to ========== V4L2 VDA/VEA: Use VIDIOC_G/S_SELECTION for accelerators Applications have to use the selection API to work with codec drivers. The old crop ioctls are not suitable for such hardware. For backward compatibility we may fallback to use old crop APIs if the codec driver doesn't support selection interfaces. BUG=615857 TEST=Run VDA/VEA tests on elm. 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 ==========
Description was changed from ========== V4L2 VDA/VEA: Use VIDIOC_G/S_SELECTION for accelerators Applications have to use the selection API to work with codec drivers. The old crop ioctls are not suitable for such hardware. For backward compatibility we may fallback to use old crop APIs if the codec driver doesn't support selection interfaces. BUG=615857 TEST=Run VDA/VEA tests on elm. 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 ========== to ========== V4L2 VDA/VEA: Use VIDIOC_G/S_SELECTION for accelerators Applications have to use the selection API to work with codec drivers. The old crop ioctls are not suitable for such hardware. For backward compatibility we may fallback to use old crop APIs if the codec driver doesn't support selection interfaces. BUG=615857 TEST=Run VDA/VEA tests on elm. 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 ==========
mojahsu@chromium.org changed reviewers: + henryhsu@chromium.org, wuchengli@chromium.org
wuchengli@chromium.org changed reviewers: + posciak@chromium.org
Pawel. PTAL.
https://codereview.chromium.org/2958213002/diff/1/media/gpu/v4l2_video_decode... File media/gpu/v4l2_video_decode_accelerator.cc (right): https://codereview.chromium.org/2958213002/diff/1/media/gpu/v4l2_video_decode... media/gpu/v4l2_video_decode_accelerator.cc:2113: struct v4l2_rect* ioctl_rect = NULL; s/ioctl_rect/visible_rect/. It's more readable. https://codereview.chromium.org/2958213002/diff/1/media/gpu/v4l2_video_decode... media/gpu/v4l2_video_decode_accelerator.cc:2121: } if (device_->Ioctl(VIDIOC_G_SELECTION, &selection_arg) == 0) { } else { CROP } https://codereview.chromium.org/2958213002/diff/1/media/gpu/v4l2_video_encode... File media/gpu/v4l2_video_encode_accelerator.cc (right): https://codereview.chromium.org/2958213002/diff/1/media/gpu/v4l2_video_encode... media/gpu/v4l2_video_encode_accelerator.cc:1084: struct v4l2_rect ioctl_rect; s/ioctl_rect/visible_rect/. It's more readable. https://codereview.chromium.org/2958213002/diff/1/media/gpu/v4l2_video_encode... media/gpu/v4l2_video_encode_accelerator.cc:1100: if (device_->Ioctl(VIDIOC_S_SELECTION, &selection_arg) != 0) { if (device_->Ioctl(VIDIOC_S_SELECTION, &selection_arg) == 0) { G_SELECTION } else { S_CROP G_CROP } We don't need to support the case of one selection and one crop. A driver should either support selection and crop. They don't mix and match. https://codereview.chromium.org/2958213002/diff/1/media/gpu/v4l2_video_encode... media/gpu/v4l2_video_encode_accelerator.cc:1101: VLOG(2) << "Fallback to VIDIOC_S_CROP"; DVLOG https://codereview.chromium.org/2958213002/diff/1/media/gpu/v4l2_video_encode... media/gpu/v4l2_video_encode_accelerator.cc:1112: VLOG(2) << "Fallback to VIDIOC_G_CROP"; DVLOG
V4L2IP also needs to change to SELECTION. https://cs.chromium.org/chromium/src/media/gpu/v4l2_image_processor.cc?q=v4l2...
On 2017/06/29 08:04:11, wuchengli wrote: > V4L2IP also needs to change to SELECTION. > https://cs.chromium.org/chromium/src/media/gpu/v4l2_image_processor.cc?q=v4l2... I want to change it on the other commit. Do we need to change all of them together?
On 2017/07/03 07:50:26, mojahsu1 wrote: > On 2017/06/29 08:04:11, wuchengli wrote: > > V4L2IP also needs to change to SELECTION. > > > https://cs.chromium.org/chromium/src/media/gpu/v4l2_image_processor.cc?q=v4l2... > > I want to change it on the other commit. OK. You can change it in another CL because you will need to test lots of devices at the same time if they are in the came CL. > Do we need to change all of them together? By the way, how do you migrate elm encode driver if it's implementation is wrong? I thought this CL wouldn't work with the current elm encoder driver?
On 2017/07/03 13:04:14, wuchengli wrote: > On 2017/07/03 07:50:26, mojahsu1 wrote: > > On 2017/06/29 08:04:11, wuchengli wrote: > > > V4L2IP also needs to change to SELECTION. > > > > > > https://cs.chromium.org/chromium/src/media/gpu/v4l2_image_processor.cc?q=v4l2... > > > > I want to change it on the other commit. > OK. You can change it in another CL because you will need to test lots of > devices at the same time if they are in the came CL. > > Do we need to change all of them together? > > By the way, how do you migrate elm encode driver if it's implementation is > wrong? I thought this CL wouldn't work with the current elm encoder driver? I will test the CL with Rowan device which use 4.4 kernel and same driver as elm's for selection path and other device with crop path.
https://codereview.chromium.org/2958213002/diff/40001/media/gpu/v4l2_video_de... File media/gpu/v4l2_video_decode_accelerator.cc (right): https://codereview.chromium.org/2958213002/diff/40001/media/gpu/v4l2_video_de... media/gpu/v4l2_video_decode_accelerator.cc:2122: } run git cl format https://codereview.chromium.org/2958213002/diff/40001/media/gpu/v4l2_video_en... File media/gpu/v4l2_video_encode_accelerator.cc (right): https://codereview.chromium.org/2958213002/diff/40001/media/gpu/v4l2_video_en... media/gpu/v4l2_video_encode_accelerator.cc:1100: IOCTL_OR_ERROR_RETURN_FALSE(VIDIOC_G_SELECTION, &selection_arg); VIDIOC_S_SELECTION will adjust the rect. No need to call VIDIOC_G_SELECTION. Remove this line.
Description was changed from
==========
V4L2 VDA/VEA: Use VIDIOC_G/S_SELECTION for accelerators
Applications have to use the selection API to work with codec drivers.
The old crop ioctls are not suitable for such hardware.
For backward compatibility we may fallback to use old crop APIs if the
codec driver doesn't support selection interfaces.
BUG=615857
TEST=Run VDA/VEA tests on elm.
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
==========
to
==========
V4L2 VDA/VEA: Use VIDIOC_G/S_SELECTION for codec drivers
Applications have to use the selection API to work with codec drivers.
The old crop ioctls are not suitable for such hardware.
For backward compatibility we may fallback to use old crop APIs if the
codec driver doesn't support selection interfaces.
BUG=615857
TEST==Run VDA/VEA tests on elm.
on Rowan for selection path
on peach_pi for fallback crop path.
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
==========
https://codereview.chromium.org/2958213002/diff/60001/media/gpu/v4l2_video_en... File media/gpu/v4l2_video_encode_accelerator.cc (right): https://codereview.chromium.org/2958213002/diff/60001/media/gpu/v4l2_video_en... media/gpu/v4l2_video_encode_accelerator.cc:1095: Actually the original comment is valid for both SELECTION and CROP api. Can you move it here? // The width and height might be adjusted by driver. // Need to read it back and set to visible_size_. Also remove line 1106 and 1107.
Description was changed from
==========
V4L2 VDA/VEA: Use VIDIOC_G/S_SELECTION for codec drivers
Applications have to use the selection API to work with codec drivers.
The old crop ioctls are not suitable for such hardware.
For backward compatibility we may fallback to use old crop APIs if the
codec driver doesn't support selection interfaces.
BUG=615857
TEST==Run VDA/VEA tests on elm.
on Rowan for selection path
on peach_pi for fallback crop path.
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
==========
to
==========
V4L2 VDA/VEA: Use VIDIOC_G/S_SELECTION for codec drivers
Applications have to use the selection API to work with codec drivers.
The old crop ioctls are not suitable for such hardware.
For backward compatibility, we may fallback to use old crop APIs if the
codec driver doesn't support selection interfaces.
BUG=615857
TEST==Run VDA/VEA tests on elm.
on Rowan for selection path
on peach_pi for fallback crop path.
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
==========
lgtm
The CQ bit was checked by mojahsu@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 80001, "attempt_start_ts": 1500971318610010,
"parent_rev": "9bd6882a7344b149f5fa8f9c4a4b0d80dadc64e0", "commit_rev":
"92c29f34ff9ad157f94b6f1a8f67a795c9deb439"}
Message was sent while issue was closed.
Description was changed from
==========
V4L2 VDA/VEA: Use VIDIOC_G/S_SELECTION for codec drivers
Applications have to use the selection API to work with codec drivers.
The old crop ioctls are not suitable for such hardware.
For backward compatibility, we may fallback to use old crop APIs if the
codec driver doesn't support selection interfaces.
BUG=615857
TEST==Run VDA/VEA tests on elm.
on Rowan for selection path
on peach_pi for fallback crop path.
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
==========
to
==========
V4L2 VDA/VEA: Use VIDIOC_G/S_SELECTION for codec drivers
Applications have to use the selection API to work with codec drivers.
The old crop ioctls are not suitable for such hardware.
For backward compatibility, we may fallback to use old crop APIs if the
codec driver doesn't support selection interfaces.
BUG=615857
TEST==Run VDA/VEA tests on elm.
on Rowan for selection path
on peach_pi for fallback crop path.
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
Review-Url: https://codereview.chromium.org/2958213002
Cr-Commit-Position: refs/heads/master@{#489265}
Committed:
https://chromium.googlesource.com/chromium/src/+/92c29f34ff9ad157f94b6f1a8f67...
==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as https://chromium.googlesource.com/chromium/src/+/92c29f34ff9ad157f94b6f1a8f67... |
