[libcamera-devel] [PATCH 4/6] libcamera: PixelFormatInfo: Add methods bytesPerLine and imageSize

[Laurent Pinchart]

Hi Paul,

Thank you for the patch.

On Tue, Jun 30, 2020 at 12:14:09AM +0900, Paul Elder wrote:
> Add instance methods to PixelFormatInfo for calculating bytes-per-line

They're called member functions in C++ :-) "instance method" is Java.

> and image size. This will simplify existing code that calculates these
> things.

Let's bikeshed the names in 2/6.

> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
>  include/libcamera/internal/formats.h |  3 +++
>  src/libcamera/formats.cpp            | 21 +++++++++++++++++++++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/include/libcamera/internal/formats.h b/include/libcamera/internal/formats.h
> index 3c7440a..1372a43 100644
> --- a/include/libcamera/internal/formats.h
> +++ b/include/libcamera/internal/formats.h
> @@ -46,6 +46,9 @@ public:
>  	static const PixelFormatInfo &info(const PixelFormat &format);
>  	static const PixelFormatInfo &info(const V4L2PixelFormat &format);
>  
> +	unsigned int bytesPerLine(unsigned int width);
> +	unsigned int imageSize(unsigned int width, unsigned int height);
> +
>  	const char *name;
>  	PixelFormat format;
>  	V4L2PixelFormat v4l2Format;
> diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
> index 7029a94..dc1d348 100644
> --- a/src/libcamera/formats.cpp
> +++ b/src/libcamera/formats.cpp
> @@ -7,12 +7,16 @@
>  
>  #include "libcamera/internal/formats.h"
>  
> +#include <algorithm>

Is this needed ? Does it belong to 3/6 ?

>  #include <errno.h>
>  
>  #include <libcamera/formats.h>
>  
>  #include "libcamera/internal/log.h"
>  
> +
> +#define DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
> +

If this is used in a single place, I'd rather inline it. Otherwise it
should go to utils.h (as an inline function instead of a macro for type
safety and to avoid evaluating d twice).

>  /**
>   * \file internal/formats.h
>   * \brief Types and helper methods to handle libcamera image formats
> @@ -670,4 +674,21 @@ const PixelFormatInfo &PixelFormatInfo::info(const V4L2PixelFormat &format)
>  	return info->second;
>  }
>  
> +unsigned int PixelFormatInfo::bytesPerLine(unsigned int width)

Missing documentation for these two functions.

> +{
> +	if (!isValid())
> +		return 0;
> +
> +	return DIV_ROUND_UP(width, pixelsPerGroup) * bytesPerGroup;
> +
> +}
> +
> +unsigned int PixelFormatInfo::imageSize(unsigned int width, unsigned int height)
> +{
> +	if (!isValid())
> +		return 0;

You could skip this as bytesPerLine() has the same check.

> +
> +	return height * bytesPerLine(width);
> +}
> +
>  } /* namespace libcamera */

-- 
Regards,

Laurent Pinchart