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

[Milan Zamazal]

Hi Barnabás,

thank you for review.

Barnabás Pőcze <barnabas.pocze at ideasonboard.com> writes:

> 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.

IIRC I tried to replace only the given pattern and not simpler
constructs like when the info is already retrieved, to not complicate
the patches more than necessary.  But the rest can certainly be also
replaced, I can do it in v5 if there are no objections.

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

I think so, with the broader replacement.

> 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;
>>   	}
>>