linux-kernel - Re: [PATCH v4 0/6] media: v4l2: Add extended fmt and buffer ioctls

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200721131537.6ff83c71@collabora.com>
Date:   Tue, 21 Jul 2020 13:15:37 +0200
From:   Boris Brezillon <boris.brezillon@...labora.com>
To:     Helen Koike <helen.koike@...labora.com>, Brian.Starkey@....com
Cc:     mchehab@...nel.org, hans.verkuil@...co.com,
        laurent.pinchart@...asonboard.com, sakari.ailus@....fi,
        linux-media@...r.kernel.org, tfiga@...omium.org,
        hiroh@...omium.org, nicolas@...fresne.ca, kernel@...labora.com,
        narmstrong@...libre.com, linux-kernel@...r.kernel.org,
        frkoenig@...omium.org, mjourdan@...libre.com,
        stanimir.varbanov@...aro.org
Subject: Re: [PATCH v4 0/6] media: v4l2: Add extended fmt and buffer ioctls

Hello Helen,

Just a few drive-by comments.

On Fri, 17 Jul 2020 08:54:29 -0300
Helen Koike <helen.koike@...labora.com> wrote:

> Hi,
> 
> I'm sorry for taking too long to submit v4.
> 
> It is not perfect, not all v4l2-compliance tests passes, but I'd like a review,
> specially on the API and potential problems, so I can focus on improving implementation
> and maybe drop the RFC tag for next version.
> 
> Follow below what changed in v4 and some items I'd like to discuss:
> 
> 
> * Ioctl to replace v4l2_pix_format
> ---------------------------------------------------------------------------------
> During last media summit, we agreed to create ioctls that replace the v4l2_pix_format
> struct and leave the other structs in the v4l2_format union alone.
> Thus I refactored the code to receive struct v4l2_ext_pix_format, and I renamed the
> ioctls, so now we have:
> 
> int ioctl(int fd, VIDIOC_G_EXT_FMT, struct v4l2_ext_pix_format *argp);

Maybe use the EXT_PIX_FMT suffix here since the struct is really only
about pixel formats.

> int ioctl(int fd, VIDIOC_S_EXT_FMT, struct v4l2_ext_pix_format *argp);
> int ioctl(int fd, VIDIOC_TRY_EXT_FMT, struct v4l2_ext_pix_format *argp);
> 
> The only valid types are V4L2_BUF_TYPE_VIDEO_CAPTURE and V4L2_BUF_TYPE_VIDEO_OUTPUT,
> all the other types are invalid with this API.
> 

[...]

> 
> 
> Boris Brezillon (5):
>   media: v4l2: Extend pixel formats to unify single/multi-planar
>     handling (and more)
>   media: videobuf2: Expose helpers to implement the _ext_fmt and
>     _ext_buf hooks
>   media: mediabus: Add helpers to convert a ext_pix format to/from a
>     mbus_fmt
>   media: vivid: Convert the capture and output drivers to
>     EXT_FMT/EXT_BUF
>   media: vimc: Implement the ext_fmt and ext_buf hooks

I think you should take ownership of these patches. The end result is
likely to be completely different from what I initially posted, and
you're the one doing the hard work here.

> 
> Hans Verkuil (1):
>   media: v4l2: Add extended buffer operations
> 
>  .../media/common/videobuf2/videobuf2-core.c   |   2 +
>  .../media/common/videobuf2/videobuf2-v4l2.c   | 549 +++++-----
>  .../media/test-drivers/vimc/vimc-capture.c    |  61 +-
>  drivers/media/test-drivers/vimc/vimc-common.c |   6 +-
>  drivers/media/test-drivers/vimc/vimc-common.h |   2 +-
>  drivers/media/test-drivers/vivid/vivid-core.c |  70 +-
>  .../test-drivers/vivid/vivid-touch-cap.c      |  26 +-
>  .../test-drivers/vivid/vivid-touch-cap.h      |   3 +-
>  .../media/test-drivers/vivid/vivid-vid-cap.c  | 169 +---
>  .../media/test-drivers/vivid/vivid-vid-cap.h  |  15 +-
>  .../media/test-drivers/vivid/vivid-vid-out.c  | 193 ++--
>  .../media/test-drivers/vivid/vivid-vid-out.h  |  15 +-
>  drivers/media/v4l2-core/v4l2-dev.c            |  50 +-
>  drivers/media/v4l2-core/v4l2-ioctl.c          | 934 ++++++++++++++++--
>  include/media/v4l2-ioctl.h                    |  60 ++
>  include/media/v4l2-mediabus.h                 |  42 +
>  include/media/videobuf2-core.h                |   6 +-
>  include/media/videobuf2-v4l2.h                |  21 +-
>  include/uapi/linux/videodev2.h                | 144 +++
>  19 files changed, 1650 insertions(+), 718 deletions(-)
>