[libcamera-devel] [PATCH] libcamera: v4l2_subdevice: Fix formats() returning only sizes for last mbus code

[Jacopo Mondi]

Hello Marian,

On Tue, Mar 09, 2021 at 03:44:57PM +0100, Marian Cichy wrote:
> V4L2Subdevice::formats() wants to enumerate all media bus codes and
> frame sizes on a pad, however, the current implementation always
> overwrites the SizeRange of the last mbus code, effectively returning
> only this SizeRange.
>
> This can be even fatal, as camera_sensor is using this function to
> enumerate all codes and sizes. If the last mbus code has no SizeRange
> for whatever reason, the camera_sensor will error out, claiming that it
> didn't found any image formats at all.

What is the use case for a driver returning an mbus code without any
image size associated ? Smells like a driver bug to me.

>
> Signed-off-by: Marian Cichy <m.cichy at pengutronix.de>
> ---
>  src/libcamera/v4l2_subdevice.cpp | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> index 9af1c0ab..37bafb9b 100644
> --- a/src/libcamera/v4l2_subdevice.cpp
> +++ b/src/libcamera/v4l2_subdevice.cpp
> @@ -354,10 +354,10 @@ V4L2Subdevice::Formats V4L2Subdevice::formats(unsigned int pad)
>  		return {};
>  	}
>
> +	std::vector<SizeRange> sizes = {};
>  	for (unsigned int code : enumPadCodes(pad)) {
> -		std::vector<SizeRange> sizes = enumPadSizes(pad, code);
> -		if (sizes.empty())
> -			return {};
> +		std::vector<SizeRange> codeSizeRange = enumPadSizes(pad, code);
> +		sizes.insert(sizes.end(), codeSizeRange.begin(), codeSizeRange.end());

Also, this builds 'sizes' incrementally, associating to the newly
enumerated 'code' the sizes previously enumerated for other codes
if I'm not mistaken.

Is this intentional ?

Thanks
   j

>
>  		const auto inserted = formats.insert({ code, sizes });
>  		if (!inserted.second) {
> --
> 2.29.2
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel