[RFC PATCH v2 01/13] libcamera: software_isp: Move a non-loop condition out of the loop

Laurent Pinchart laurent.pinchart at ideasonboard.com
Sat Jan 25 23:13:01 CET 2025


Hi Milan,

Thank you for the patch.

On Fri, Jan 24, 2025 at 10:57:52PM +0100, Milan Zamazal wrote:
> The check for the number of outputs is done in a loop over the outputs.
> It should be moved out of the loop as it's not loop specific and is just
> repeated there.
> 
> This is a cosmetic change not changing any functionality.
> 
> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
> ---
>  src/libcamera/software_isp/software_isp.cpp | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
> index 2bea64d9..67b27727 100644
> --- a/src/libcamera/software_isp/software_isp.cpp
> +++ b/src/libcamera/software_isp/software_isp.cpp
> @@ -290,13 +290,12 @@ int SoftwareIsp::queueBuffers(uint32_t frame, FrameBuffer *input,
>  	 */
>  	if (outputs.empty())
>  		return -EINVAL;

We could drop this condition, but I don't mind keeping it.

> +	if (outputs.size() != 1) /* only single stream atm */
> +		return -EINVAL;

While at it,

	/* We only support a single stream for now. */
	if (outputs.size() != 1)
		return -EINVAL;

>  
> -	for (auto [stream, buffer] : outputs) {
> +	for (auto [stream, buffer] : outputs)
>  		if (!buffer)
>  			return -EINVAL;
> -		if (outputs.size() != 1) /* only single stream atm */
> -			return -EINVAL;
> -	}

You can keep the curly braces.

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

>  
>  	for (auto iter = outputs.begin(); iter != outputs.end(); iter++)
>  		process(frame, input, iter->second);

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list