[libcamera-devel] [PATCH v5 07/10] libcamera: uvc: Initialize the pixel array properties

[Laurent Pinchart]

Hi Jacopo,

On Tue, Jan 05, 2021 at 02:45:03PM +0100, Jacopo Mondi wrote:
> On Tue, Jan 05, 2021 at 02:51:25PM +0200, Laurent Pinchart wrote:
> > On Tue, Jan 05, 2021 at 01:31:25PM +0100, Jacopo Mondi wrote:
> > > Initialize the pixel array properties in the UVC pipeline handler as
> > > they're now initialized in the CameraSensor class, which the UVC
> > > pipeline handler does not ue.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > > ---
> > >  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 13 +++++++++++++
> > >  1 file changed, 13 insertions(+)
> > >
> > > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > > index 7cb310e20511..1efced949762 100644
> > > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > > @@ -509,6 +509,19 @@ int UVCCameraData::init(MediaDevice *media)
> > >  	properties_.set(properties::Location, properties::CameraLocationExternal);
> > >  	properties_.set(properties::Model, utils::toAscii(media->model()));
> > >
> > > +	/*
> > > +	 * Get the current format in order to initialize the sensor array
> > > +	 * properties.
> > > +	 */
> >
> > Could we use the largest format instead ? Otherwise, if the current
> > format isn't the largest, we'll end up with a pixel array size that is
> > smaller than what the camera could produce.
> >
> 
> I (wrongly) assumed the default size is usually the largest available
> one.
> 
> Would something like:
> 
> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> @@ -8,6 +8,7 @@
>  #include <algorithm>
>  #include <fstream>
>  #include <iomanip>
> +#include <limits>
>  #include <math.h>
>  #include <memory>
>  #include <tuple>
> @@ -514,6 +515,8 @@ int UVCCameraData::init(MediaDevice *media)
>          * properties.
>          */
>         V4L2DeviceFormat format;
> +       format.size.width = std::numeric_limits<unsigned int>::max();
> +       format.size.height = format.size.width;
>         ret = video_->getFormat(&format);
>         if (ret)
>                 return ret;
> 
> work better ?

That won't change anything, getFormat() retrieves the current format,
the format argument is output only. tryFormat() would be an option, but
it will only give you the largest size for the default pixel format. The
best option is to retrieve the largest size from video_->formats().

> > Other than that this is exactly what I had in mind.
> >
> > > +	V4L2DeviceFormat format;
> > > +	ret = video_->getFormat(&format);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	properties_.set(properties::PixelArraySize, format.size);
> > > +	properties_.set(properties::PixelArrayActiveAreas,
> > > +			{ Rectangle(format.size) });
> > > +
> > >  	/* Initialise the supported controls. */
> > >  	ControlInfoMap::Map ctrls;
> > >

-- 
Regards,

Laurent Pinchart