Re: [PATCH v2 02/10] media: v4l2-dev: Add support for try state

[Hans Verkuil]

On 29/09/2025 18:15, Jai Luthra wrote:
> Hi Hans,
> 
> Quoting Hans Verkuil (2025-09-22 13:28:26)
>> On 22/09/2025 09:52, Hans Verkuil wrote:
>>> On 19/09/2025 11:55, Jai Luthra wrote:
>>>> Format negotiation performed via the TRY_FMT ioctl should only affect
>>>> the temporary context of a specific filehandle, not the active state
>>>> stored in the video device structure. To support this distinction, we
>>>> need separate storage for try and active states.
>>>>
>>>> Introduce an enum to distinguish between these two state types and store
>>>> the try state in struct v4l2_fh instead of the video device structure.
>>>> The try state is allocated during file handle initialization in
>>>> v4l2_fh_init() and released in v4l2_fh_exit().
>>>
>>> There is a major difference between TRY in video devices and TRY in subdev
>>> devices: TRY for video devices is not persistent, i.e. it doesn't need to
>>> be stored anywhere since nobody will be using that information.
>>>
> 
> Yes, the v4l2 format sent with VIDIOC_TRY_FMT is currently not stored by
> the drivers, but simply modified and returned back to userspace. From the
> userspace point of view, that should still work the same way with this
> series.
> 
> The drivers will get access to the correct state (active) for internal use
> through framework helpers (that will be introduced in next revision), so
> the try state doesn't have any real "use" today.
> 
>>> If the plan is to change that in later patch series, then that should be
>>> very clearly stated. And I think I would need to know the details of those
>>> future plans before I can OK this.
>>>
>>> So how is this try state intended to be used in the future?
>>
>> Follow-up: if there are no plans to change/enhance the VIDIOC_TRY_FMT semantics,
>> then I don't really see the point of this since there is no need to store this
>> information.
> 
> There are no plans to change the userspace side of this. The main
> motivation to allocate and keep a try state in the framework is to simplify
> the driver implementation. A single function can serve as both s_fmt and
> try_fmt ioctl handler, and operate on the passed state argument without
> caring if it will be applied on the device or just for negotiation.
> 
> In future, drivers may subclass the state with their device specific data,
> so that nothing tied to the hardware state is stored in the driver
> structures directly, but I don't see why drivers will need to inspect TRY
> formats.

I think having an unused try state is a bad idea and really confusing.

Especially because for subdevices the try state is actually used, so
for normal devices you would automatically expect the same thing when
you see a try state.

You should reconsider this approach.

Regards,

	Hans