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

[Jacopo Mondi]

Hi Laurent,

On Tue, Jan 05, 2021 at 02:51:25PM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> 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 ?

> 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