Re: [RFC PATCH v1 2/4] media: v4l2-subdev: Add which field to struct v4l2_subdev_frame_interval

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

On Tue, Oct 24, 2023 at 10:11:39AM +0300, Tomi Valkeinen wrote:
> On 24/10/2023 03:51, Laurent Pinchart wrote:
> > Due to a historical mishap, the v4l2_subdev_frame_interval structure
> > is the only part of the V4L2 subdev userspace API that doesn't contain a
> > 'which' field. This prevents trying frame intervals using the subdev
> > 'TRY' state mechanism.
> > 
> > Add a 'which' field is simple as the structure has 8 reserved fields.
> 
> "Adding"
> 
> > This would however break userspace as the field is currently set to 0,
> > corresponding to V4L2_SUBDEV_FORMAT_TRY, while the corresponding ioctls
> > currently operate on the 'ACTIVE' state. We thus need to add a new
> 
> That's not the only problem. It wouldn't work even if 0 would be 
> 'ACTIVE'. The userspace is required to clear the reserved fields, but 
> the newly added 'which' field is no longer a reserved field, and thus 
> the userspace might not clear it, leading to it being uninitialized. But 
> this is solved with the V4L2_SUBDEV_CLIENT_CAP_WHICH_INTERVAL.

Yes, I didn't claim it would be the only problem :-)

> > subdev client cap, V4L2_SUBDEV_CLIENT_CAP_WHICH_INTERVAL, to indicate
> > that userspace is aware of this new field.
> > 
> > All drivers that implement the subdev .g_frame_interval() and
> > .s_frame_interval() operations are updated to return -EINVAL when
> 
> .get_frame_interval() and .set_frame_interval() now =).

Will be fixed in v2.

-- 
Regards,

Laurent Pinchart