<div dir="ltr"><div dir="ltr">Hi Laurent,<div><br></div><div>Thanks for your feedback.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, 18 Nov 2021 at 19:55, Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com" target="_blank">laurent.pinchart@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Naush,<br>
<br>
Thank you for the patch.<br>
<br>
On Thu, Nov 18, 2021 at 04:42:16PM +0000, Naushir Patuck wrote:<br>
> PixelFormatInfo::info() would log a warning message if the PixelFormat was<br>
> invalid when called from the isRaw() function. Add a validity test in isRaw()<br>
> to avoid this warning message.<br>
> <br>
> Signed-off-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>><br>
> ---<br>
>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 3 +++<br>
>  1 file changed, 3 insertions(+)<br>
> <br>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> index 4f6c699a4379..ad526a8be6a2 100644<br>
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> @@ -102,6 +102,9 @@ bool isRaw(const PixelFormat &pixFmt)<br>
>        * The isRaw test might be redundant right now the pipeline handler only<br>
>        * supports RAW sensors. Leave it in for now, just as a sanity check.<br>
>        */<br>
> +     if (!pixFmt.isValid())<br>
> +             return false;<br>
> +<br>
<br>
This will only catch some of the issues, as PixelFormat::isValid()<br>
returns false only when the fourcc is zero, while<br>
PixelFormatInfo::info() will warn for every unknown format.<br>
<br>
How did you trigger this issue ?<br></blockquote><div><br></div><div>With the change in patch 3/4, we check if a raw stream is requested</div><div>and use the buffer count value to make up at least 2 internal buffers</div><div>for unicam.  If no RAW stream is present, the pixelformat is invalid (fourcc == 0).</div><div>This is the bit of code that triggers it:</div><div><br></div><div>for (Stream *s : camera->streams()) {<br>    if (isRaw(s->configuration().pixelFormat)) {<br>        numRawBuffers = s->configuration().bufferCount;<br>        break;<br>    }<br>       }<br></div><div><br></div><div>This change may not pick up all invalid formats where fourcc !=0, but should</div><div>be sufficient for this particular usage.</div><div><br></div><div>Regards,</div><div>Naush</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
>       const PixelFormatInfo &info = PixelFormatInfo::info(pixFmt);<br>
>       if (!info.isValid())<br>
>               return false;<br>
<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
</blockquote></div></div>