[libcamera-devel] [PATCH v3 6/9] pipeline: raspberrypi: Set packing formats for the Unicam image node

[Kieran Bingham]

Quoting Naushir Patuck (2021-10-27 13:39:11)
> Hi Kieran,
> 
> Thank you for your feedback.
> 
> On Wed, 27 Oct 2021 at 13:17, Kieran Bingham <
> kieran.bingham at ideasonboard.com> wrote:
> 
> > Quoting Naushir Patuck (2021-10-27 10:28:00)
> > > Default to using CSI2 packed formats when setting up the Unicam image
> > format,
> > > but use an unpacked format if the user requests one through
> > StreamConfiguration.
> > >
> > > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > > Reviewed-by: David Plowman <david.plowman at raspberrypi.com>
> > > ---
> > >  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 12 ++++++++----
> > >  1 file changed, 8 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > index 48f561d31a31..1b78b5e74a63 100644
> > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > @@ -96,14 +96,14 @@ BayerFormat mbusCodeToBayerFormat(unsigned int
> > mbusCode)
> > >         return BayerFormat{};
> > >  }
> > >
> > > -V4L2DeviceFormat toV4L2DeviceFormat(V4L2SubdeviceFormat &format)
> > > +V4L2DeviceFormat toV4L2DeviceFormat(V4L2SubdeviceFormat &format,
> > BayerFormat::Packing packing)
> > >  {
> > >         V4L2DeviceFormat deviceFormat;
> > >         BayerFormat bayer = mbusCodeToBayerFormat(format.mbus_code);
> > >
> > >         ASSERT(bayer.isValid());
> > >
> > > -       bayer.packing = BayerFormat::Packing::CSI2Packed;
> > > +       bayer.packing = packing;
> >
> > Ok, that really solidifies the answer to my earlier question ;-)
> >
> >
> > >         deviceFormat.fourcc = bayer.toV4L2PixelFormat();
> > >         deviceFormat.size = format.size;
> > >         return deviceFormat;
> > > @@ -413,7 +413,8 @@ CameraConfiguration::Status
> > RPiCameraConfiguration::validate()
> > >                          * the user request.
> > >                          */
> > >                         V4L2SubdeviceFormat sensorFormat =
> > findBestFormat(data_->sensorFormats_, cfg.size);
> > > -                       V4L2DeviceFormat unicamFormat =
> > toV4L2DeviceFormat(sensorFormat);
> > > +                       V4L2DeviceFormat unicamFormat =
> > toV4L2DeviceFormat(sensorFormat,
> > > +
> >   BayerFormat::Packing::CSI2Packed);
> >
> > I believe ::Packing is optional if you want to reduce line length. But
> > can also stay, it won't make a big difference to the line formatting.
> >
> >
> > >                         int ret =
> > data_->unicam_[Unicam::Image].dev()->tryFormat(&unicamFormat);
> > >                         if (ret)
> > >                                 return Invalid;
> > > @@ -631,6 +632,7 @@ int PipelineHandlerRPi::configure(Camera *camera,
> > CameraConfiguration *config)
> > >         for (auto const stream : data->streams_)
> > >                 stream->reset();
> > >
> > > +       BayerFormat::Packing packing = BayerFormat::Packing::CSI2Packed;
> >
> > Same.
> >
> 
> Yes, I can remove the extra scoping to be more consistent.
> 

Erm... given Laurent's just submitted patch, I might ahve to take back
my suggestions I'm afraid.

Looks like this should be /with/ ::Packing::

--
Kieran



> Naush
> 
> 
> >
> >
> > Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> >
> > >         Size maxSize, sensorSize;
> > >         unsigned int maxIndex = 0;
> > >         bool rawStream = false;
> > > @@ -649,6 +651,8 @@ int PipelineHandlerRPi::configure(Camera *camera,
> > CameraConfiguration *config)
> > >                          */
> > >                         sensorSize = cfg.size;
> > >                         rawStream = true;
> > > +                       /* Check if the user has explicitly set an
> > unpacked format. */
> > > +                       packing =
> > BayerFormat::fromPixelFormat(cfg.pixelFormat).packing;
> > >                 } else {
> > >                         if (cfg.size > maxSize) {
> > >                                 maxSize = config->at(i).size;
> > > @@ -667,7 +671,7 @@ int PipelineHandlerRPi::configure(Camera *camera,
> > CameraConfiguration *config)
> > >          * Unicam image output format. The ISP input format gets set at
> > start,
> > >          * just in case we have swapped bayer orders due to flips.
> > >          */
> > > -       V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat);
> > > +       V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat,
> > packing);
> > >         ret =
> > data->unicam_[Unicam::Image].dev()->setFormat(&unicamFormat);
> > >         if (ret)
> > >                 return ret;
> > > --
> > > 2.25.1
> > >
> >