<div dir="ltr"><div dir="ltr">Hi Kieran,<div><br></div><div>Thank you for your feedback.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, 27 Oct 2021 at 13:17, Kieran Bingham <<a href="mailto:kieran.bingham@ideasonboard.com" target="_blank">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 10:28:00)<br>
> Default to using CSI2 packed formats when setting up the Unicam image format,<br>
> but use an unpacked format if the user requests one through 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 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 mbusCode)<br>
>         return BayerFormat{};<br>
>  }<br>
>  <br>
> -V4L2DeviceFormat toV4L2DeviceFormat(V4L2SubdeviceFormat &format)<br>
> +V4L2DeviceFormat toV4L2DeviceFormat(V4L2SubdeviceFormat &format, 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 RPiCameraConfiguration::validate()<br>
>                          * the user request.<br>
>                          */<br>
>                         V4L2SubdeviceFormat sensorFormat = findBestFormat(data_->sensorFormats_, cfg.size);<br>
> -                       V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat);<br>
> +                       V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat,<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 = data_->unicam_[Unicam::Image].dev()->tryFormat(&unicamFormat);<br>
>                         if (ret)<br>
>                                 return Invalid;<br>
> @@ -631,6 +632,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)<br>
>         for (auto const stream : data->streams_)<br>
>                 stream->reset();<br>
>  <br>
> +       BayerFormat::Packing packing = BayerFormat::Packing::CSI2Packed;<br>
<br>
Same.<br></blockquote><div><br></div><div>Yes, I can remove the extra scoping to be more consistent.</div><div><br></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>
<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, CameraConfiguration *config)<br>
>                          */<br>
>                         sensorSize = cfg.size;<br>
>                         rawStream = true;<br>
> +                       /* Check if the user has explicitly set an unpacked format. */<br>
> +                       packing = 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, CameraConfiguration *config)<br>
>          * Unicam image output format. The ISP input format gets set at start,<br>
>          * just in case we have swapped bayer orders due to flips.<br>
>          */<br>
> -       V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat);<br>
> +       V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat, packing);<br>
>         ret = data->unicam_[Unicam::Image].dev()->setFormat(&unicamFormat);<br>
>         if (ret)<br>
>                 return ret;<br>
> -- <br>
> 2.25.1<br>
><br>
</blockquote></div></div>