[libcamera-devel] [PATCH] libcamera: formats: Search V4L2 format info on map

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Mon, Aug 08, 2022 at 02:48:48PM +0200, Jacopo Mondi via libcamera-devel wrote:
> Commit f25ad4a2b16b ("libcamera: formats: Reimplement V4L2
> PixelFormatInfo::info()") changed the PixelFormatInfo::info(const
> V4L2PixelFormat &format) function overload to:
> 
> 	return info(format.toPixelFormat());
> 
> After the series that contains such commit, the PixelFormatInfo for the
> pixel format applied to a video device is retrieved at
> V4L2VideoDevice::open() time. Some video devices register formats not
> available to applications, for example metadata formats or, in the case
> of ISP devices, formats to describe the ISP statistics and parameters.
> 
> This causes the
> 
> 	format.toPixelFormat()
> 
> call to output a WARN message, which spams the log and unnecessary alert

s/unnecessary alert/unnecessarily alerts/

> the users.
> 
> Restore the behaviour preceding commit f25ad4a2b16b, which in the case
> a V4L2 pixel format is not registered in the map of formats known to
> libcamera, silently returns an invalid PixelFormatInfo without
> outputting any unnecessary warning message.
> 
> Fixes: f25ad4a2b16b ("libcamera: formats: Reimplement V4L2 PixelFormatInfo::info()")
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/formats.cpp | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
> index 0bd0e09ae44c..51eccdce064e 100644
> --- a/src/libcamera/formats.cpp
> +++ b/src/libcamera/formats.cpp
> @@ -852,7 +852,16 @@ const PixelFormatInfo &PixelFormatInfo::info(const PixelFormat &format)
>   */
>  const PixelFormatInfo &PixelFormatInfo::info(const V4L2PixelFormat &format)
>  {
> -	return info(format.toPixelFormat());
> +	const auto &info = std::find_if(pixelFormatInfo.begin(), pixelFormatInfo.end(),
> +					[&format](auto &pair) {
> +						const auto &formats = pair.second.v4l2Formats;
> +						return std::find(formats.begin(), formats.end(), format)
> +							!= formats.end();
> +					});
> +	if (info == pixelFormatInfo.end())
> +		return pixelFormatInfoInvalid;
> +
> +	return info->second;

As discussed separately, this isn't very efficient as it loops over the
whole map. You have posted a v2 that addresses this, and there are
ongoing discussions about which approach is best. In case this one gets
selected, with the commit message fixed,

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

>  }
> 
>  /**

-- 
Regards,

Laurent Pinchart