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

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Thu, Jan 07, 2021 at 10:47:32AM +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 use.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> index 7cb310e20511..e0c4131496be 100644
> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> @@ -509,6 +509,21 @@ 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.
> +	 */
> +	Size resolution{};

No need for {}, there's a suitable default constructor.

> +	for (const auto &[pixelformat, sizerange] : video_->formats()) {

s/pixelformat/pixelFormat/
s/sizerange/sizeRanges/

But I'm afraid this breaks compilation with gcc 7, which generates a
warning due to the unused pixelformat variable. You need to use the
pre-C++17 syntax, with the infamous .second to get the size range.

> +		for (const SizeRange &size : sizerange) {

s/size/sizeRange/

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> +			if (size.max > resolution)
> +				resolution = size.max;
> +		}
> +	}
> +
> +	properties_.set(properties::PixelArraySize, resolution);
> +	properties_.set(properties::PixelArrayActiveAreas, { Rectangle(resolution) });
> +
>  	/* Initialise the supported controls. */
>  	ControlInfoMap::Map ctrls;
>  

-- 
Regards,

Laurent Pinchart