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]
Date:   Thu, 22 Jun 2023 11:14:41 -0400
From:   Nicolas Dufresne <nicolas@...fresne.ca>
To:     Benjamin Bara <bbara93@...il.com>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Manivannan Sadhasivam <mani@...nel.org>
Cc:     laurent.pinchart@...asonboard.com, jacopo.mondi@...asonboard.com,
        Dave Stevenson <dave.stevenson@...pberrypi.com>,
        linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
        Benjamin Bara <benjamin.bara@...data.com>
Subject: Re: [PATCH WIP RFC v2 0/2] media: uapi: Add V4L2_CID_VTOTAL control

Hi,

Le jeudi 22 juin 2023 à 09:12 +0200, Benjamin Bara a écrit :
> Hi!
> 
> This series adds new controls for the vertical and horizontal total
> size. Camera sensors usually have registers regarding the total size and
> not the blanking size. Also user space prefers to calculate with total
> frame sizes. Therefore, this would simplify implementations on both
> sides and could be seen as a replacement or upgrade for V4L2_CID_VBLANK.
> Additionally, its value is independent from format changes and therefore
> simplifies calculations in user space.

The naming is getting more and more generic. We need to find a way to give these
controls a name that associates them to their usage field (sensors). If we keep
growing the control list this way, in few years it will be terribly hard to
understand what is used for what.

What about, V4L2_CID_SENSOR_VTOTAL/HTOTAL ? I'm expecting the last bit of the
name to come from very well known specification, otherwise, its not a great name
either in my opinion.

> 
> For v2, I added a little bit more documentation and my personal
> expectations to 1/2. As I am fairly new to the camera world, they might
> be a little bit naive so please correct me if this is utopical. I added
> the RFC tag for that reason. However, my intention is to define a
> documented behaviour regarding the values and limits which could
> basically depend on HTOTAL. Currently, HBLANK is always set to the
> minimum in libcamera (I guess to simplify calculations). I think with
> HTOTAL, we could always use WIDTH_MAX(all modes) + HBLANK_MIN(WIDTH_MAX)
> as default to enable a constant frame size. If the current HTOTAL value
> differs from the default, we could infer that the user wants to either
> have a higher frame rate, or a higher exposure and could adapt to that.
> E.g. use the minimums to get the highest frame rate possible if the
> current values are outside of the possible range, or not limiting the
> exposure control by the current vertical blanking as it is done now. I
> guess this would help the driver to "understand" the needs of the user
> space, and therefore allow it to react in a defined and expected manner.
> 
> 2/2 is a WIP (same as in v1) and currently implements VTOTAL for the
> imx290, basically extending the current V4L2_CID_VBLANK implementation.
> 
> Thanks Dave for the feedback, insights and the examples (ov5647,
> ov9282). I might need some time to skim through the data sheets to learn
> why they do stuff like it is done now.
> 
> ---
> Changes in v2:
> - 1/2: add HTOTAL
> - 1/2: add documentation about expectations
> - Link to v1: https://lore.kernel.org/r/20230609-v4l2-vtotal-v1-0-4b7dee7e073e@skidata.com
> 

Do you have an link to an example user of this new API ?

Nicolas

> ---
> Benjamin Bara (2):
>       media: uapi: Add V4L2_CID_{V,H}TOTAL controls
>       media: i2c: imx290: Add support for V4L2_CID_VTOTAL
> 
>  Documentation/driver-api/media/camera-sensor.rst   | 11 ++++-
>  .../media/v4l/ext-ctrls-image-source.rst           | 16 ++++++++
>  drivers/media/i2c/imx290.c                         | 47 ++++++++++++++++------
>  drivers/media/v4l2-core/v4l2-ctrls-defs.c          |  2 +
>  include/uapi/linux/v4l2-controls.h                 |  2 +
>  5 files changed, 64 insertions(+), 14 deletions(-)
> ---
> base-commit: 9561de3a55bed6bdd44a12820ba81ec416e705a7
> change-id: 20230609-v4l2-vtotal-eb15d37cafea
> 
> Best regards,

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ