[libcamera-devel] [PATCH v3 4/4] pipeline: raspberrypi: Avoid invalid PixelFormat warning message

[Naushir Patuck]

Hi Laurent,

Thanks for your feedback.

On Thu, 18 Nov 2021 at 19:55, Laurent Pinchart <
laurent.pinchart at ideasonboard.com> wrote:

> Hi Naush,
>
> Thank you for the patch.
>
> On Thu, Nov 18, 2021 at 04:42:16PM +0000, Naushir Patuck wrote:
> > PixelFormatInfo::info() would log a warning message if the PixelFormat
> was
> > invalid when called from the isRaw() function. Add a validity test in
> isRaw()
> > to avoid this warning message.
> >
> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > ---
> >  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > index 4f6c699a4379..ad526a8be6a2 100644
> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > @@ -102,6 +102,9 @@ bool isRaw(const PixelFormat &pixFmt)
> >        * The isRaw test might be redundant right now the pipeline
> handler only
> >        * supports RAW sensors. Leave it in for now, just as a sanity
> check.
> >        */
> > +     if (!pixFmt.isValid())
> > +             return false;
> > +
>
> This will only catch some of the issues, as PixelFormat::isValid()
> returns false only when the fourcc is zero, while
> PixelFormatInfo::info() will warn for every unknown format.
>
> How did you trigger this issue ?
>

With the change in patch 3/4, we check if a raw stream is requested
and use the buffer count value to make up at least 2 internal buffers
for unicam.  If no RAW stream is present, the pixelformat is invalid
(fourcc == 0).
This is the bit of code that triggers it:

for (Stream *s : camera->streams()) {
    if (isRaw(s->configuration().pixelFormat)) {
        numRawBuffers = s->configuration().bufferCount;
        break;
    }
}

This change may not pick up all invalid formats where fourcc !=0, but should
be sufficient for this particular usage.

Regards,
Naush



>
> >       const PixelFormatInfo &info = PixelFormatInfo::info(pixFmt);
> >       if (!info.isValid())
> >               return false;
>
> --
> Regards,
>
> Laurent Pinchart
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20211119/84190545/attachment-0001.htm>