<div dir="ltr"><div dir="ltr">Hi David,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, 3 Dec 2021 at 17:46, David Plowman <<a href="mailto:david.plowman@raspberrypi.com">david.plowman@raspberrypi.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">Actually I found myself looking at this bit of code again as part of<br>
my occasional quest to figure out why the gstreamer element and v4l2<br>
compatibility library work so poorly.<br>
<br>
I was wondering if we should limit the output sizes to the max sensor<br>
resolution here. The trouble with advertising 16384x16384 is that<br>
applications (cheese, for example) decide that we really want 8K video<br>
and unsurprisingly it's all a complete disaster.<br>
<br>
What do you think - is this the right place to tackle the problem? (Or<br>
if not, where else?)<br>
<br>
Though I've not checked, maybe this would help the v4l2 compatibility<br>
layer too? (anyone know?)<br></blockquote><div><br></div><div>Yes, that does make sense.  Once this goes in, I'll look to make that change.</div><div><br></div><div>Regards,</div><div>Naush</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>
David<br>
<br>
On Fri, 3 Dec 2021 at 12:40, David Plowman<br>
<<a href="mailto:david.plowman@raspberrypi.com" target="_blank">david.plowman@raspberrypi.com</a>> wrote:<br>
><br>
> Hi Naush<br>
><br>
> Thanks for this patch!<br>
><br>
> On Fri, 3 Dec 2021 at 11:32, Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>> wrote:<br>
> ><br>
> > Return the available sensor PixelFormats and sizes from generateConfiguration()<br>
> > if the StreamRole is set to StreamRole::Raw. The existing code returns the<br>
> > PixelFormats and sizes for all other StreamRole types.<br>
> ><br>
> > Signed-off-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>><br>
> > ---<br>
> >  .../pipeline/raspberrypi/raspberrypi.cpp      | 21 ++++++++++++++-----<br>
> >  1 file changed, 16 insertions(+), 5 deletions(-)<br>
> ><br>
> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> > index 5b76916e9e98..cbfb58562626 100644<br>
> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> > @@ -595,12 +595,23 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,<br>
> >                         return nullptr;<br>
> >                 }<br>
> ><br>
> > -               /* Translate the V4L2PixelFormat to PixelFormat. */<br>
> >                 std::map<PixelFormat, std::vector<SizeRange>> deviceFormats;<br>
> > -               for (const auto &format : fmts) {<br>
> > -                       PixelFormat pf = format.first.toPixelFormat();<br>
> > -                       if (pf.isValid())<br>
> > -                               deviceFormats[pf] = format.second;<br>
> > +               if (role == StreamRole::Raw) {<br>
> > +                       /* Translate the MBUS codes to a PixelFormat. */<br>
> > +                       for (const auto &format : data->sensorFormats_) {<br>
> > +                               PixelFormat pf = mbusCodeToPixelFormat(format.first,<br>
> > +                                                                      BayerFormat::Packing::CSI2);<br>
> > +                               if (pf.isValid())<br>
> > +                                       deviceFormats.emplace(std::piecewise_construct, std::forward_as_tuple(pf),<br>
> > +                                               std::forward_as_tuple(format.second.begin(), format.second.end()));<br>
><br>
> I won't even begin to pretend that I understand this! I guess I wonder<br>
> just a little bit whether one could write it in a more "obvious" way<br>
> that the compiler could still optimise, but really I have no clue. So<br>
> anyway:<br>
><br>
> Reviewed-by: David Plowman <<a href="mailto:david.plowman@raspberrypi.com" target="_blank">david.plowman@raspberrypi.com</a>><br>
><br>
> Thanks!<br>
> David<br>
><br>
> > +                       }<br>
> > +               } else {<br>
> > +                       /* Translate the V4L2PixelFormat to PixelFormat. */<br>
> > +                       for (const auto &format : fmts) {<br>
> > +                               PixelFormat pf = format.first.toPixelFormat();<br>
> > +                               if (pf.isValid())<br>
> > +                                       deviceFormats[pf] = format.second;<br>
> > +                       }<br>
> >                 }<br>
> ><br>
> >                 /* Add the stream format based on the device node used for the use case. */<br>
> > --<br>
> > 2.25.1<br>
> ><br>
</blockquote></div></div>