[PATCH v4 03/11] libcamera: formats: Add a helper to check for a raw pixel format

[Barnabás Pőcze]

Hi

2025. 04. 07. 10:56 keltezéssel, Milan Zamazal írta:
> There are several places with the same pattern to check whether a given
> pixel format is a raw format:
> 
>    return libcamera::PixelFormatInfo::info(pixFmt).colourEncoding ==
>           libcamera::PixelFormatInfo::ColourEncodingRAW;
> 
> Let's move the corresponding isFormatRaw helper from mali-c55.cpp to
> formats.cpp and use it where applicable.  This also avoids a need to
> introduce the same helper (or the same pattern) in the followup patches.

As far as I can see, there are a lot of other places where this check is "open coded",
I believe either all of them or none of them should be replaced. For example,
`IPARkISP1::configure()`, `ISICameraConfiguration::validate()`,
`IPU3CameraConfiguration::validate()`, etc.

And there is also `PipelineHandlerBase::isRaw()` in the rpi pipeline.

I am wondering if adding `PixelFormatInfo::isRaw()` could be useful as well.


Regards,
Barnabás Pőcze


> 
> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
> ---
>   include/libcamera/internal/formats.h         |  2 ++
>   src/libcamera/formats.cpp                    | 11 +++++++++++
>   src/libcamera/pipeline/imx8-isi/imx8-isi.cpp |  4 ++--
>   src/libcamera/pipeline/mali-c55/mali-c55.cpp | 10 ----------
>   src/libcamera/pipeline/rkisp1/rkisp1.cpp     |  7 ++-----
>   5 files changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/include/libcamera/internal/formats.h b/include/libcamera/internal/formats.h
> index 6a3e9c16..bc4417d0 100644
> --- a/include/libcamera/internal/formats.h
> +++ b/include/libcamera/internal/formats.h
> @@ -62,4 +62,6 @@ public:
>   	std::array<Plane, 3> planes;
>   };
>   
> +bool isFormatRaw(const libcamera::PixelFormat &pixFmt);
> +
>   } /* namespace libcamera */
> diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
> index bfcdfc08..c6e0a262 100644
> --- a/src/libcamera/formats.cpp
> +++ b/src/libcamera/formats.cpp
> @@ -1215,4 +1215,15 @@ unsigned int PixelFormatInfo::numPlanes() const
>   	return count;
>   }
>   
> +/**
> + * \brief Return whether the given pixel format is a raw format
> + * \param[in] pixFmt The pixel format to examine
> + * \return True iff the given format is a raw format
> + */
> +bool isFormatRaw(const libcamera::PixelFormat &pixFmt)
> +{
> +	return libcamera::PixelFormatInfo::info(pixFmt).colourEncoding ==
> +	       libcamera::PixelFormatInfo::ColourEncodingRAW;
> +}
> +
>   } /* namespace libcamera */
> diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
> index 4e66b336..9ff11a41 100644
> --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
> +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
> @@ -24,6 +24,7 @@
>   #include "libcamera/internal/camera.h"
>   #include "libcamera/internal/camera_sensor.h"
>   #include "libcamera/internal/device_enumerator.h"
> +#include "libcamera/internal/formats.h"
>   #include "libcamera/internal/media_device.h"
>   #include "libcamera/internal/pipeline_handler.h"
>   #include "libcamera/internal/v4l2_subdevice.h"
> @@ -312,8 +313,7 @@ unsigned int ISICameraData::getYuvMediaBusFormat(const PixelFormat &pixelFormat)
>   
>   unsigned int ISICameraData::getMediaBusFormat(PixelFormat *pixelFormat) const
>   {
> -	if (PixelFormatInfo::info(*pixelFormat).colourEncoding ==
> -	    PixelFormatInfo::ColourEncodingRAW)
> +	if (isFormatRaw(*pixelFormat))
>   		return getRawMediaBusFormat(pixelFormat);
>   
>   	return getYuvMediaBusFormat(*pixelFormat);
> diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> index a05e11fc..3721fb30 100644
> --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> @@ -42,16 +42,6 @@
>   #include "libcamera/internal/v4l2_subdevice.h"
>   #include "libcamera/internal/v4l2_videodevice.h"
>   
> -namespace {
> -
> -bool isFormatRaw(const libcamera::PixelFormat &pixFmt)
> -{
> -	return libcamera::PixelFormatInfo::info(pixFmt).colourEncoding ==
> -	       libcamera::PixelFormatInfo::ColourEncodingRAW;
> -}
> -
> -} /* namespace */
> -
>   namespace libcamera {
>   
>   LOG_DEFINE_CATEGORY(MaliC55)
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 52633fe3..a5b613bb 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -536,8 +536,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
>   	 */
>   	if (config_.size() > 1) {
>   		for (const auto &cfg : config_) {
> -			if (PixelFormatInfo::info(cfg.pixelFormat).colourEncoding ==
> -			    PixelFormatInfo::ColourEncodingRAW) {
> +			if (isFormatRaw(cfg.pixelFormat)) {
>   				config_.resize(1);
>   				status = Adjusted;
>   				break;
> @@ -551,9 +550,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
>   		 * Platforms with dewarper support, such as i.MX8MP, support
>   		 * only a single stream. We can inspect config_[0] only here.
>   		 */
> -		bool isRaw = PixelFormatInfo::info(config_[0].pixelFormat).colourEncoding ==
> -			     PixelFormatInfo::ColourEncodingRAW;
> -		if (!isRaw)
> +		if (!isFormatRaw(config_[0].pixelFormat))
>   			useDewarper = true;
>   	}
>