<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, 27 Oct 2021 at 13:50, Kieran Bingham <<a href="mailto:kieran.bingham@ideasonboard.com">kieran.bingham@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">Quoting Naushir Patuck (2021-10-27 13:39:11)<br>
> Hi Kieran,<br>
> <br>
> Thank you for your feedback.<br>
> <br>
> On Wed, 27 Oct 2021 at 13:17, Kieran Bingham <<br>
> <a href="mailto:kieran.bingham@ideasonboard.com" target="_blank">kieran.bingham@ideasonboard.com</a>> wrote:<br>
> <br>
> > Quoting Naushir Patuck (2021-10-27 10:28:00)<br>
> > > Default to using CSI2 packed formats when setting up the Unicam image<br>
> > format,<br>
> > > but use an unpacked format if the user requests one through<br>
> > StreamConfiguration.<br>
> > ><br>
> > > Signed-off-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>><br>
> > > Reviewed-by: David Plowman <<a href="mailto:david.plowman@raspberrypi.com" target="_blank">david.plowman@raspberrypi.com</a>><br>
> > > ---<br>
> > >  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 12 ++++++++----<br>
> > >  1 file changed, 8 insertions(+), 4 deletions(-)<br>
> > ><br>
> > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> > b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> > > index 48f561d31a31..1b78b5e74a63 100644<br>
> > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> > > @@ -96,14 +96,14 @@ BayerFormat mbusCodeToBayerFormat(unsigned int<br>
> > mbusCode)<br>
> > >         return BayerFormat{};<br>
> > >  }<br>
> > ><br>
> > > -V4L2DeviceFormat toV4L2DeviceFormat(V4L2SubdeviceFormat &format)<br>
> > > +V4L2DeviceFormat toV4L2DeviceFormat(V4L2SubdeviceFormat &format,<br>
> > BayerFormat::Packing packing)<br>
> > >  {<br>
> > >         V4L2DeviceFormat deviceFormat;<br>
> > >         BayerFormat bayer = mbusCodeToBayerFormat(format.mbus_code);<br>
> > ><br>
> > >         ASSERT(bayer.isValid());<br>
> > ><br>
> > > -       bayer.packing = BayerFormat::Packing::CSI2Packed;<br>
> > > +       bayer.packing = packing;<br>
> ><br>
> > Ok, that really solidifies the answer to my earlier question ;-)<br>
> ><br>
> ><br>
> > >         deviceFormat.fourcc = bayer.toV4L2PixelFormat();<br>
> > >         deviceFormat.size = format.size;<br>
> > >         return deviceFormat;<br>
> > > @@ -413,7 +413,8 @@ CameraConfiguration::Status<br>
> > RPiCameraConfiguration::validate()<br>
> > >                          * the user request.<br>
> > >                          */<br>
> > >                         V4L2SubdeviceFormat sensorFormat =<br>
> > findBestFormat(data_->sensorFormats_, cfg.size);<br>
> > > -                       V4L2DeviceFormat unicamFormat =<br>
> > toV4L2DeviceFormat(sensorFormat);<br>
> > > +                       V4L2DeviceFormat unicamFormat =<br>
> > toV4L2DeviceFormat(sensorFormat,<br>
> > > +<br>
> >   BayerFormat::Packing::CSI2Packed);<br>
> ><br>
> > I believe ::Packing is optional if you want to reduce line length. But<br>
> > can also stay, it won't make a big difference to the line formatting.<br>
> ><br>
> ><br>
> > >                         int ret =<br>
> > data_->unicam_[Unicam::Image].dev()->tryFormat(&unicamFormat);<br>
> > >                         if (ret)<br>
> > >                                 return Invalid;<br>
> > > @@ -631,6 +632,7 @@ int PipelineHandlerRPi::configure(Camera *camera,<br>
> > CameraConfiguration *config)<br>
> > >         for (auto const stream : data->streams_)<br>
> > >                 stream->reset();<br>
> > ><br>
> > > +       BayerFormat::Packing packing = BayerFormat::Packing::CSI2Packed;<br>
> ><br>
> > Same.<br>
> ><br>
> <br>
> Yes, I can remove the extra scoping to be more consistent.<br>
> <br>
<br>
Erm... given Laurent's just submitted patch, I might ahve to take back<br>
my suggestions I'm afraid.<br>
<br>
Looks like this should be /with/ ::Packing::<br></blockquote><div><br></div><div>Yes, I saw that change in my inbox just after replying to your message...</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>
--<br>
Kieran<br>
<br>
<br>
<br>
> Naush<br>
> <br>
> <br>
> ><br>
> ><br>
> > Reviewed-by: Kieran Bingham <<a href="mailto:kieran.bingham@ideasonboard.com" target="_blank">kieran.bingham@ideasonboard.com</a>><br>
> ><br>
> > >         Size maxSize, sensorSize;<br>
> > >         unsigned int maxIndex = 0;<br>
> > >         bool rawStream = false;<br>
> > > @@ -649,6 +651,8 @@ int PipelineHandlerRPi::configure(Camera *camera,<br>
> > CameraConfiguration *config)<br>
> > >                          */<br>
> > >                         sensorSize = cfg.size;<br>
> > >                         rawStream = true;<br>
> > > +                       /* Check if the user has explicitly set an<br>
> > unpacked format. */<br>
> > > +                       packing =<br>
> > BayerFormat::fromPixelFormat(cfg.pixelFormat).packing;<br>
> > >                 } else {<br>
> > >                         if (cfg.size > maxSize) {<br>
> > >                                 maxSize = config->at(i).size;<br>
> > > @@ -667,7 +671,7 @@ int PipelineHandlerRPi::configure(Camera *camera,<br>
> > CameraConfiguration *config)<br>
> > >          * Unicam image output format. The ISP input format gets set at<br>
> > start,<br>
> > >          * just in case we have swapped bayer orders due to flips.<br>
> > >          */<br>
> > > -       V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat);<br>
> > > +       V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat,<br>
> > packing);<br>
> > >         ret =<br>
> > data->unicam_[Unicam::Image].dev()->setFormat(&unicamFormat);<br>
> > >         if (ret)<br>
> > >                 return ret;<br>
> > > --<br>
> > > 2.25.1<br>
> > ><br>
> ><br>
</blockquote></div></div>