[libcamera-devel] [PATCH] libcamera: formats: add planeCount helper

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Wed, Jul 29, 2020 at 12:25:24PM +0100, Kieran Bingham wrote:
> Determine the number of planes used by a format by counting the number
> of PxielFormatPlaneInfo entries with a valid entry.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>  include/libcamera/internal/formats.h |  2 ++
>  src/libcamera/formats.cpp            | 22 ++++++++++++++++++++++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/include/libcamera/internal/formats.h b/include/libcamera/internal/formats.h
> index 0bb151044294..beeaab1ae2f5 100644
> --- a/include/libcamera/internal/formats.h
> +++ b/include/libcamera/internal/formats.h
> @@ -45,6 +45,8 @@ public:
>  	unsigned int frameSize(const Size &size,
>  			       const std::array<unsigned int, 3> &strides) const;
>  
> +	unsigned int planeCount() const;
> +
>  	/* \todo Add support for non-contiguous memory planes */
>  	const char *name;
>  	PixelFormat format;
> diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
> index cd63c15cb926..70d31fb36c37 100644
> --- a/src/libcamera/formats.cpp
> +++ b/src/libcamera/formats.cpp
> @@ -805,4 +805,26 @@ PixelFormatInfo::frameSize(const Size &size,
>  	return sum;
>  }
>  
> +/**
> + * \brief Identify the number of planes represented by the format
> + *
> + * This function counts the number of planes which have a valid configuration
> + * and uses that to determine the number of planes used by the format.

I think this is just an internal implementation detail.

 * \brief Retrieve the number of planes used by the format
 * \return The number of planes used by the format

should be enough.

And if we hide the fact that we count them, maybe
s/planeCount/numPlanes/ ? I think that's a bit more consistent with how
we name fields (haven't checked though).

> + *
> + * \return The number of planes used by the format
> + */
> +unsigned int PixelFormatInfo::planeCount() const
> +{
> +	unsigned int count = 0;
> +
> +	for (const PixelFormatPlaneInfo &p : planes) {
> +		if (p.bytesPerGroup == 0)
> +			break;
> +
> +		count++;
> +	}
> +
> +	return count;

	return std::count_if(planes.begin(), planes.end(),
			     [](const PixelFormatPlaneInfo &p) {
				     return p.bytesPerGroup != 0;
			     });

would also work, entirely up to you.

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

> +}
> +
>  } /* namespace libcamera */

-- 
Regards,

Laurent Pinchart