Hi Hans, On Fri, Oct 27, 2023 at 03:13:55PM +0200, Hans Verkuil wrote: > On 25/10/2023 23:07, Laurent Pinchart wrote: > > On Mon, Oct 09, 2023 at 01:56:21PM +0300, Tomi Valkeinen wrote: > >> On 03/10/2023 15:08, Sakari Ailus wrote: > >>> On VIDIOC_SUBDEV_[GS]_ROUTING, only return as many routes back to the user > >>> as there's room. Do not consider it an error if more routes existed. > >>> Simply inform the user there are more routes. > >> > >> Inform how? And I agree with Hans here. How about return ENOSPC, but the > >> kernel fills in num_routes to tell the userspace how many there actually > >> are? > > > > For VIDIOC_SUBDEV_G_ROUTING I have no objection. For > > VIDIOC_SUBDEV_S_ROUTING, however, I would prefer the ioctl to succeed if > > the routes can be applied but there's not enough space to return them > > all to the application. The application may not have an interest in > > getting the applied routes back. > > For S_ROUTING, do we still want to update num_routes in that case? Even > though we return 0 since we could actually set the routes. num_routes should be updated in this case, otherwise the caller won't know there are more routes. The caller can then decide what to do about it, to get the routes using G_ROUTING for instance: the length of the routing table is now known. > > I think it depends a bit on the naming of these fields in v7. I recall there was a comment on the naming now, possibly from you, but I can't find the comments at the moment. :-I I'm open to renaming them but I don't think the current naming is bad either. > > How likely is it that an application would run into this anyway? I suspect a > typical app will get the routes first, then modify it. Laurent can probably comment on this from libcamera perspective, but I presume that most users of routes are somehow specific to the device. I would expect source sub-devices that produce the streams are typical places where you may have dependencies between streams, but they do exist elsewhere (but there are very few cases). > > It's worth giving this a try, but it depends a bit on how easy it is to > document it. If you need to jump through hoops to try to explain it to an > end user, then perhaps this is overly complicated. The documentation is changed in patch "media: v4l: subdev: Add len_routes field to struct v4l2_subdev_routing", two patches prior to this. The changes are split across several patches in order to avoid fewer difficult to review patches. I'll change the prefix of this patch to "v4l: subdev:" as well. -- Regards, Sakari Ailus
