[libcamera-devel] [RFC PATCH 1/2] libcamera: v4l2_videodevice: Get formats even if no framesizes

[Laurent Pinchart]

Hello,

On Tue, Jul 19, 2022 at 05:54:32PM +0200, Jacopo Mondi via libcamera-devel wrote:
> On Tue, Jul 19, 2022 at 09:10:02PM +0900, Paul Elder via libcamera-devel wrote:
> > V4L2VideoDevice::formats() returns an empty list of formats if enumSizes
> > fails, including if the driver doesn't implement ENUM_FRAMESIZES.
> >
> > Add an optional ignoreSizes parameter to formats() so that it can still
> > be used when the pipeline handler knows that its driver doesn't
> > implement ENUM_FRAMESIZES.
> 
> I think we shall require drivers to implement ENUM_FRAMESIZES, instead
> of allowing them to ignore it.
> 
> I'm surprised the rkisp1 capture device does not support
> .vidioc_enum_framesizes(). Should that be fixed in the driver ?

Good question. It should be fairly easy, as it should report a single
range with the min/max values corresponding to the absolute limits of
the hardware, and it would allow deferring the question of whether or
not we require all drivers to report sizes (as well as the bikeshedding
on the bool argument to formats() :-)).

> > Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> > ---
> >  include/libcamera/internal/v4l2_videodevice.h |  2 +-
> >  src/libcamera/v4l2_videodevice.cpp            | 12 ++++++++----
> >  2 files changed, 9 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h
> > index 8525acbc..6cc9be85 100644
> > --- a/include/libcamera/internal/v4l2_videodevice.h
> > +++ b/include/libcamera/internal/v4l2_videodevice.h
> > @@ -205,7 +205,7 @@ public:
> >  	int getFormat(V4L2DeviceFormat *format);
> >  	int tryFormat(V4L2DeviceFormat *format);
> >  	int setFormat(V4L2DeviceFormat *format);
> > -	Formats formats(uint32_t code = 0);
> > +	Formats formats(uint32_t code = 0, bool ignoreSizes = false);
> >
> >  	int setSelection(unsigned int target, Rectangle *rect);
> >
> > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> > index 63911339..b8bd4bc4 100644
> > --- a/src/libcamera/v4l2_videodevice.cpp
> > +++ b/src/libcamera/v4l2_videodevice.cpp
> > @@ -1045,14 +1045,18 @@ int V4L2VideoDevice::trySetFormatSingleplane(V4L2DeviceFormat *format, bool set)
> >   *
> >   * \return A list of the supported video device formats
> >   */
> > -V4L2VideoDevice::Formats V4L2VideoDevice::formats(uint32_t code)
> > +V4L2VideoDevice::Formats V4L2VideoDevice::formats(uint32_t code, bool ignoreSizes)
> >  {
> >  	Formats formats;
> >
> >  	for (V4L2PixelFormat pixelFormat : enumPixelformats(code)) {
> > -		std::vector<SizeRange> sizes = enumSizes(pixelFormat);
> > -		if (sizes.empty())
> > -			return {};
> > +		std::vector<SizeRange> sizes;
> > +
> > +		if (!ignoreSizes) {
> > +			sizes = enumSizes(pixelFormat);
> > +			if (sizes.empty())
> > +				return {};
> > +		}
> >
> >  		if (formats.find(pixelFormat) != formats.end()) {
> >  			LOG(V4L2, Error)

-- 
Regards,

Laurent Pinchart