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

[Kieran Bingham]

Quoting Milan Zamazal (2025-05-13 09:10:08)
> 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.

I'm sure I recall this used to be a global helper - and somehow/ for
some reason it got moved to being handled by pipeline handlers
specfically. I think the intention was that pipeline handlers would know
which formats are raw or not ... but I can't find anything specific in
the history yet..

Even if that was the case, maybe that needs to be revisited if in the
end every pipeline handler ends up implementing it in exactly the same
way...

--
Kieran


> 
> > Regards,
> > Barnabás Pőcze
> >
> >
> >>   +/**
> >> + * \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;
> >> +}
> >> +