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

[Jacopo Mondi]

Hi Hans

On Tue, Sep 30, 2025 at 09:30:55AM +0200, Hans Verkuil wrote:
> 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

I might have missed where. TRY formats on subdev sink pads influence
TRY formats on the source pads, are there other usages I might be
overlooking ?

> for normal devices you would automatically expect the same thing when
> you see a try state.
>
> You should reconsider this approach.

The 'try' state will be stored in the v4l2_fh and won't be accessible
to userspace.

Drivers instead, might accumulate the result of multiple TRY_FMT calls
into the state stored in the v4l2_fh incrementally. Is this a concern
for you ?

I still think having a single implementation for s_fmt and try_fmt is
a win for drivers, even if the try state are now effectively stored
somewhere.

Thanks
  j

>
> Regards,
>
> 	Hans