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

[Jacopo Mondi]

Hi Laurent,

On Thu, Jan 07, 2021 at 05:09:26AM +0200, Laurent Pinchart wrote:
> 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().

Ouch, I mean 'setFormat()' with a V4L2DeviceFormat with sizes =
MAX_INT. But as you suggested, that would work for the current
pixelformat only.

Looking into the formats() map might be a better option.

Thanks
   j

>
> > > 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