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

[Kieran Bingham]

Quoting Milan Zamazal (2025-05-26 11:15:23)
> 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.

This was tried before but not completed/successful.

https://patchwork.libcamera.org/project/libcamera/list/?series=4640&state=*

The problem is the definition of 'raw' is too vague.

Maybe we could have something like 'isBayerFormat' if we want to map all
bayer formats - but a RAW stream being defined on the colour encoding is
ill-defined...

I still think there's scope for trying to factor out parts here - but we
seem to need to do better at determining what 'RAW' means ...

--
Kieran


> 
> 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 ecda426a..ad36ebea 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 675f0a74..50c83fb7 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -538,8 +538,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;
> @@ -553,9 +552,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;
>         }
>  
> -- 
> 2.49.0
>