Hi Sakari and Laurent, Thanks for reviewing this On Tue, Jun 30, 2015 at 6:19 AM, Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> wrote: > Hi Laurent, > > Laurent Pinchart wrote: >> Hi Sakari, >> >> On Monday 29 June 2015 10:23:34 Sakari Ailus wrote: >>> Helen Fornazier wrote: >>>> According to the V4L2 API, the VIDIOC_STREAMON ioctl should return EPIPE >>>> when the pipeline configuration is invalid. >>>> >>>> As the .vidioc_streamon in the v4l2_ioctl_ops usually forwards the error >>>> caused by the v4l2_subdev_link_validate_default (if it is in use), it >>>> should return -EPIPE if it detects a format mismatch in the pipeline >>>> configuration >>> >>> Only link configuration errors have yielded -EPIPE so far, sub-device >>> format configuration error has returned -INVAL instead as you noticed. >> >> It should also be noted that while v4l2_subdev_link_validate() will return - >> EINVAL in case of error, the only driver that performs custom link validation >> (omap3isp/ispccdc.c) will return -EPIPE. > > Good point. That has escaped me until now. > >>> There are not many sources of -EINVAL while enabling streaming and all >>> others are directly caused by the application; I lean towards thinking >>> the code is good as it was. The documentation could be improved though. >>> It may not be clear which error codes could be caused by different >>> conditions. >>> >>> The debug level messages from media module >>> (drivers/media/media-entity.c) do provide more information if needed, >>> albeit this certainly is not an application interface. >>> >>> I wonder what others think. >> >> There's a discrepancy between the implementation and the documentation, so at >> least one of them need to be fixed. -EPIPE would be coherent with the >> documentation and seems appropriately named, but another error code would >> allow userspace to tell link configuration and format configuration problems >> apart. > > That was the original intent, I think. > >> Do you think -EINVAL is the most appropriate error code for format >> configuration ? It's already used to indicate that the stream type is invalid >> or that not enough buffers have been allocated, and is also used by drivers >> directly for various purposes. > > That's true, it's been used also for that purpose. At the time this > certainly was not the primary concern. If you can think of a better > error code for the purpose (than EINVAL) I'm certainly fine with using one. > > I still think that -EPIPE is worse for telling about incorrect format > configuration than -EINVAL since it's relatively easy to avoid -EINVAL > for the documented reasons. > > -- > Kind regards, > > Sakari Ailus > sakari.ailus@xxxxxxxxxxxxxxx I'd like just to point out where in the docs EPIPE for format mismatch is specified, as it is not described in the streamon page as I thought it would, but it is in the subdev page in case anyone is looking for it (as I took some time to find it too): http://linuxtv.org/downloads/v4l-dvb-apis/subdev.html "Applications are responsible for configuring coherent parameters on the whole pipeline and making sure that connected pads have compatible formats. The pipeline is checked for formats mismatch at VIDIOC_STREAMON time, and an EPIPE error code is then returned if the configuration is invalid" So maybe the doc should be improved as you already stated. -- Helen Fornazier -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
