[PATCH 1/2] libcamera: pixel_format: Add isRaw() helper

[Jacopo Mondi]

Hi Umang

On Mon, Sep 30, 2024 at 08:50:38PM GMT, Umang Jain wrote:
> Add a isRaw() helper to the PixelFormat class, to know whether the
> pixel format has RAW encoding.
>
> This will used by validation and configuration code paths in pipeline
> handlers, to know whether a pixel format is a raw format or not.
>
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>

Do you know why RPi does instead

bool PipelineHandlerBase::isRaw(const PixelFormat &pixFmt)
{
	/* This test works for both Bayer and raw mono formats. */
	return BayerFormat::fromPixelFormat(pixFmt).isValid();
}

cc RPi which has upstreamed BayerFormat to know if there's a specfic reason
why they do this instead of using PixelFormatInfo.

Seems like we have two maps of raw formants, one part of the canonical
PixelFormatInfo and one in BayerFormat. They store more or less the
same information, BayerFormat has a 'packing' field that
PixelFormatInfo doesn't have, but I wonder why we have duplicated
this instead of properly extending PixelFormatInfo

PixelFormatInfo:

	{ formats::RGGB_PISP_COMP1, {
		.name = "RGGB_PISP_COMP1",
		.format = formats::RGGB_PISP_COMP1,
		.v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB), },
		.bitsPerPixel = 8,
		.colourEncoding = PixelFormatInfo::ColourEncodingRAW,
		.packed = true,
		.pixelsPerGroup = 2,
		.planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},
	} },

BayerFormat:
	{ { BayerFormat::RGGB, 16, BayerFormat::Packing::PISP1 },
		{ formats::RGGB_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB) } },

True, BayerFormat can be constructed from an mbusCode (something that
has proven to be problematic in the past, as multiple codes map to the
same BayerFormat), I wonder if that's the main reason.

> ---
>  include/libcamera/pixel_format.h |  1 +
>  src/libcamera/pixel_format.cpp   | 11 +++++++++++
>  2 files changed, 12 insertions(+)
>
> diff --git a/include/libcamera/pixel_format.h b/include/libcamera/pixel_format.h
> index 1b4d8c7c..aed53ea6 100644
> --- a/include/libcamera/pixel_format.h
> +++ b/include/libcamera/pixel_format.h
> @@ -37,6 +37,7 @@ public:
>  	constexpr uint64_t modifier() const { return modifier_; }
>
>  	std::string toString() const;
> +	bool isRaw() const;
>
>  	static PixelFormat fromString(const std::string &name);
>
> diff --git a/src/libcamera/pixel_format.cpp b/src/libcamera/pixel_format.cpp
> index 314179a8..436ef5fb 100644
> --- a/src/libcamera/pixel_format.cpp
> +++ b/src/libcamera/pixel_format.cpp
> @@ -100,6 +100,17 @@ bool PixelFormat::operator<(const PixelFormat &other) const
>   * \return DRM modifier
>   */
>
> +/**
> + * \brief Checks if \a this is a RAW pixel format
> + * \return True if \a this is a RAW pixel format, false otherwise
> + */
> +bool PixelFormat::isRaw() const
> +{
> +	const PixelFormatInfo &info = PixelFormatInfo::info(*this);
> +
> +	return info.colourEncoding == PixelFormatInfo::ColourEncodingRAW;
> +}
> +
>  /**
>   * \brief Assemble and return a string describing the pixel format
>   * \return A string describing the pixel format
> --
> 2.45.2
>