[libcamera-devel] [PATCH v1] pipeline: raspberrypi: Return the sensor formats from generateConfiguration()

[Kieran Bingham]

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