New issue
Advanced search Search tips

Issue 878124 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 1
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

V4L2_CTRL_FLAG_CAN_STORE vs. V4L2_CTRL_FLAG_MODIFY_LAYOUT

Project Member Reported by briannorris@chromium.org, Aug 27

Issue description

I was looking at upgrading the linux-headers package, and I noticed that include/uapi/linux/videodev2.h has a conflict on the value of one of the V4L flags fields. Both V4L2_CTRL_FLAG_CAN_STORE (downstream V4L modification) and V4L2_CTRL_FLAG_MODIFY_LAYOUT (upstream as of v4.12) utilize the value 0x400.

It wasn't clear to me whether this flag is actually used anywhere outside the kernel. Is this a dead flag? Can it be handled in a less upstream-conflicting way? Or do we just need to upstream it?

It looks like we already renumbered this here:

https://chromium-review.googlesource.com/c/chromiumos/overlays/chromiumos-overlay/+/323623

Does that mean this *isn't* really a user-space flag?
 
Labels: media-kernel-shortlist
We do use this flag indeed. In particular, it's used by the Rockchip VPU driver in 3.14 and 4.4. We're working with upstream on a replacement (V4L2 Request API), but it's going to take some time until we can switch this driver to it.

Looks like we may need to shift it again. I guess we can just move it somewhere far, like 0x8000.

Also, despite being exposed through the VIDIOC_QUERY_CTRL ioctl, we don't use it in our user space, so we don't need to go through the complex set of steps to keep Chrome working across builds.
Status: Started (was: Assigned)
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 30

Labels: merge-merged-chromeos-3.14
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/4ade9eac7b0fd604f43b4776616cd4f64a5e3e9e

commit 4ade9eac7b0fd604f43b4776616cd4f64a5e3e9e
Author: Tomasz Figa <tfiga@chromium.org>
Date: Thu Aug 30 12:09:30 2018

CHROMIUM: media: Move V4L2_CTRL_FLAG_CAN_STORE far from previous flag

The V4L2_CTRL_FLAG_CAN_STORE flag was added by a downstream CL:232581,
which assigned it a bit next to the previous flag. It was then moved one
bit further due to a collision with upstream, which is now the case
again. Move if far a way from previous flag, so the chance of this
happening again is much lower.

Note that even though this flag is exposed to user space, it is not used
by our user space code, so there is no compatibility concern.

BUG= chromium:878124 
TEST=emerge-veyron_minnie chromeos-kernel-3_14

Change-Id: I0cfbd5edff41a210e3ddf8043e1245e11fd7ebe9
Signed-off-by: Tomasz Figa <tfiga@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1194553
Reviewed-by: Pawel Osciak <posciak@chromium.org>

[modify] https://crrev.com/4ade9eac7b0fd604f43b4776616cd4f64a5e3e9e/include/uapi/linux/videodev2.h

Project Member

Comment 5 by bugdroid1@chromium.org, Aug 30

Labels: merge-merged-chromeos-4.4
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/683e15ce0fd480376498db69f60e3ad125dacc06

commit 683e15ce0fd480376498db69f60e3ad125dacc06
Author: Tomasz Figa <tfiga@chromium.org>
Date: Thu Aug 30 12:09:32 2018

CHROMIUM: media: Move V4L2_CTRL_FLAG_CAN_STORE far from previous flag

The V4L2_CTRL_FLAG_CAN_STORE flag was added by a downstream CL:232581,
which assigned it a bit next to the previous flag. It was then moved one
bit further due to a collision with upstream, which is now the case
again. Move if far a way from previous flag, so the chance of this
happening again is much lower.

Note that even though this flag is exposed to user space, it is not used
by our user space code, so there is no compatibility concern.

BUG= chromium:878124 
TEST=emerge-kevin chromeos-kernel-4_4

Change-Id: I0cfbd5edff41a210e3ddf8043e1245e11fd7ebe9
Signed-off-by: Tomasz Figa <tfiga@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1195217
Reviewed-by: Pawel Osciak <posciak@chromium.org>

[modify] https://crrev.com/683e15ce0fd480376498db69f60e3ad125dacc06/include/uapi/linux/videodev2.h

Project Member

Comment 6 by bugdroid1@chromium.org, Aug 31

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/bd957dda82273bcbc73944d25316f77efffb4688

commit bd957dda82273bcbc73944d25316f77efffb4688
Author: Tomasz Figa <tfiga@chromium.org>
Date: Fri Aug 31 12:23:17 2018

sys-kernel/linux-headers: Move V4L2_CTRL_FLAG_CAN_STORE far from previous flag

The V4L2_CTRL_FLAG_CAN_STORE flag was added by a downstream CL:232581,
which assigned it a bit next to the previous flag. It was then moved one
bit further due to a collision with upstream, which is now the case
again. Move if far a way from previous flag, so the chance of this
happening again is much lower.

Note that even though this flag is exposed to user space, it is not used
currently by our user space code, so there is no compatibility concern.

This is a header change corresponding to following kernel CL:1194553 and
CL:1195217.

BUG= chromium:878124 
TEST=pre-CQ passes (unused definition)

Change-Id: Ic359877506308e79e21cbc0417fae42995633f6c
Reviewed-on: https://chromium-review.googlesource.com/1194530
Commit-Ready: Tomasz Figa <tfiga@chromium.org>
Tested-by: Tomasz Figa <tfiga@chromium.org>
Reviewed-by: Pawel Osciak <posciak@chromium.org>

[modify] https://crrev.com/bd957dda82273bcbc73944d25316f77efffb4688/sys-kernel/linux-headers/files/0001-CHROMIUM-media-headers-Import-V4L2-headers-from-Chro.patch
[rename] https://crrev.com/bd957dda82273bcbc73944d25316f77efffb4688/sys-kernel/linux-headers/linux-headers-4.4-r27.ebuild

Project Member

Comment 7 by bugdroid1@chromium.org, Aug 31

Labels: merge-merged-chromeos-3.18
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/01ea8c1cfdd71457d7a644c60bcbb6c7fad0986c

commit 01ea8c1cfdd71457d7a644c60bcbb6c7fad0986c
Author: Tomasz Figa <tfiga@chromium.org>
Date: Fri Aug 31 18:19:50 2018

Revert "CHROMIUM: Add config store support"

This reverts commit bfd7e2d1a04fc950e64e071670e040abce16e345.

This downstream code was cherry picked from chromeos-3.14 branch, but is
not used by any board running chromeos-3.18 branch.

BUG= chromium:878124 
TEST=cros tryjob --hwtest hana-release-tryjob gives no new/relevant
     test failures

Change-Id: I114cfbe476586951e924aa8c21acd527e6841a7d
Signed-off-by: Tomasz Figa <tfiga@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1195208
Reviewed-by: Pawel Osciak <posciak@chromium.org>

[modify] https://crrev.com/01ea8c1cfdd71457d7a644c60bcbb6c7fad0986c/drivers/media/v4l2-core/v4l2-ioctl.c
[modify] https://crrev.com/01ea8c1cfdd71457d7a644c60bcbb6c7fad0986c/drivers/media/v4l2-core/v4l2-ctrls.c
[modify] https://crrev.com/01ea8c1cfdd71457d7a644c60bcbb6c7fad0986c/include/media/v4l2-ctrls.h

Project Member

Comment 8 by bugdroid1@chromium.org, Aug 31

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/29e4d3aca6c914e57c383034ef5f3a4cb4e1907a

commit 29e4d3aca6c914e57c383034ef5f3a4cb4e1907a
Author: Tomasz Figa <tfiga@chromium.org>
Date: Fri Aug 31 18:19:51 2018

Revert "CHROMIUM: v4l: Add H264 low-level decoder API compound controls."

This reverts commit 5461eb7aad62094b3aeb723498fe1f5685fca8b8.

This downstream code was cherry picked from chromeos-3.14 branch, but is
not used by any board running chromeos-3.18 branch.

BUG= chromium:878124 
TEST=pre-CQ passes (unused code)

Change-Id: Id7adfbcdd2cdc07dd3768ef91afb1b1bb2e4e14f
Signed-off-by: Tomasz Figa <tfiga@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1195209
Reviewed-by: Pawel Osciak <posciak@chromium.org>

[modify] https://crrev.com/29e4d3aca6c914e57c383034ef5f3a4cb4e1907a/include/media/v4l2-ctrls.h
[modify] https://crrev.com/29e4d3aca6c914e57c383034ef5f3a4cb4e1907a/drivers/media/v4l2-core/v4l2-ctrls.c
[modify] https://crrev.com/29e4d3aca6c914e57c383034ef5f3a4cb4e1907a/include/uapi/linux/videodev2.h
[modify] https://crrev.com/29e4d3aca6c914e57c383034ef5f3a4cb4e1907a/include/uapi/linux/v4l2-controls.h

Project Member

Comment 9 by bugdroid1@chromium.org, Aug 31

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/15e1351f2b3d10a3ea56cd5305f75324867b7404

commit 15e1351f2b3d10a3ea56cd5305f75324867b7404
Author: Tomasz Figa <tfiga@chromium.org>
Date: Fri Aug 31 18:19:53 2018

Revert "CHROMIUM: videodev2.h: rename reserved2 to config_store in v4l2_buffer."

This reverts commit 45454ef8afd8f609de228dae7d10e54a2219e328.

This downstream code was cherry picked from chromeos-3.14 branch, but is
not used by any board running chromeos-3.18 branch.

BUG= chromium:878124 
TEST=pre-CQ passes (unused code)

Change-Id: Ieff6f1b6c830ee071f045ac5e17d1a9e63841f81
Signed-off-by: Tomasz Figa <tfiga@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1195210
Reviewed-by: Pawel Osciak <posciak@chromium.org>

[modify] https://crrev.com/15e1351f2b3d10a3ea56cd5305f75324867b7404/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
[modify] https://crrev.com/15e1351f2b3d10a3ea56cd5305f75324867b7404/drivers/media/usb/cpia2/cpia2_v4l.c
[modify] https://crrev.com/15e1351f2b3d10a3ea56cd5305f75324867b7404/drivers/media/v4l2-core/videobuf2-core.c
[modify] https://crrev.com/15e1351f2b3d10a3ea56cd5305f75324867b7404/include/uapi/linux/videodev2.h

Project Member

Comment 10 by bugdroid1@chromium.org, Aug 31

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/e46ceb869aba517f89dc90717944d8c268f2507d

commit e46ceb869aba517f89dc90717944d8c268f2507d
Author: Tomasz Figa <tfiga@chromium.org>
Date: Fri Aug 31 18:19:54 2018

Revert "CHROMIUM: videodev2.h: add config_store to v4l2_ext_controls"

This reverts commit f76346e6a7f85bbdd606f05a8af8b00f48c4e99d.

This downstream code was cherry picked from chromeos-3.14 branch, but is
not used by any board running chromeos-3.18 branch.

BUG= chromium:878124 
TEST=pre-CQ passes (unused code)

Change-Id: Ic96efc9a06d3aa498c4fc9e062b4445650cdc959
Signed-off-by: Tomasz Figa <tfiga@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1195211
Reviewed-by: Pawel Osciak <posciak@chromium.org>

[modify] https://crrev.com/e46ceb869aba517f89dc90717944d8c268f2507d/include/uapi/linux/videodev2.h

Project Member

Comment 11 by bugdroid1@chromium.org, Aug 31

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/f9fd43ba3bbf805db6e2dfa730e1965adacbf7ac

commit f9fd43ba3bbf805db6e2dfa730e1965adacbf7ac
Author: Tomasz Figa <tfiga@chromium.org>
Date: Fri Aug 31 18:19:56 2018

Revert "CHROMIUM: videodev2.h: add V4L2_CTRL_FLAG_CAN_STORE"

This reverts commit 0b0a3e941c832b19605d7c7a88fadacb5739c978.

This downstream code was cherry picked from chromeos-3.14 branch, but is
not used by any board running chromeos-3.18 branch.

BUG= chromium:878124 
TEST=pre-CQ passes (unused code)

Change-Id: I1b410006d8772a15f865d4a2b3170651ebf77b92
Signed-off-by: Tomasz Figa <tfiga@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1195212
Reviewed-by: Pawel Osciak <posciak@chromium.org>

[modify] https://crrev.com/f9fd43ba3bbf805db6e2dfa730e1965adacbf7ac/include/uapi/linux/videodev2.h

Project Member

Comment 12 by bugdroid1@chromium.org, Aug 31

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/878770f5d736eb9831cb5106e6e9de933301d9a8

commit 878770f5d736eb9831cb5106e6e9de933301d9a8
Author: Tomasz Figa <tfiga@chromium.org>
Date: Fri Aug 31 18:19:57 2018

Revert "CHROMIUM: [media] v4l: Add private compound control type."

This reverts commit 36c47a1710fdf7031b0537bd2f15efd67e137e96.

This downstream code was cherry picked from chromeos-3.14 branch, but is
not used by any board running chromeos-3.18 branch.

BUG= chromium:878124 
TEST=pre-CQ passes (unused code)

Change-Id: I92816cb96f22e01b81919ed64f2de6db7e147d02
Signed-off-by: Tomasz Figa <tfiga@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1195213
Reviewed-by: Pawel Osciak <posciak@chromium.org>

[modify] https://crrev.com/878770f5d736eb9831cb5106e6e9de933301d9a8/drivers/media/v4l2-core/v4l2-ctrls.c
[modify] https://crrev.com/878770f5d736eb9831cb5106e6e9de933301d9a8/include/uapi/linux/videodev2.h

Status: Fixed (was: Started)

Sign in to add a comment