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

[David Plowman]

Hi Naush

Thanks for the update.

On Fri, 22 Oct 2021 at 15:40, Naushir Patuck <naush at raspberrypi.com> wrote:
>
> 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.

Yes, I think this is the right way to do it!

>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index a31b0f81eba7..45725527d66e 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -61,11 +61,14 @@ SensorFormats populateSensorFormats(std::unique_ptr<CameraSensor> &sensor)
>         return formats;
>  }
>
> -inline V4L2DeviceFormat toV4L2DeviceFormat(V4L2SubdeviceFormat &mode)
> +inline V4L2DeviceFormat toV4L2DeviceFormat(V4L2SubdeviceFormat &mode, bool unpacked = false)

I'm not totally sure I like the default value here - I kind of feel
people should be forced to think about it. But that's a teeny thing so
I wouldn't worry.

And also ignoring the very minor brain contortion involved in passing
*un*-packed which gets flipped to set the *packed* field!

Reviewed-by: David Plowman <david.plowman at raspberrypi.com>

Thanks!
David

>  {
>         V4L2DeviceFormat deviceFormat;
>         BayerFormat bayer = BayerFormat::fromMbusCode(mode.mbus_code);
>
> +       bayer.packing = unpacked ? BayerFormat::Packing::None
> +                                : BayerFormat::Packing::CSI2Packed;
> +
>         deviceFormat.fourcc = bayer.toV4L2PixelFormat();
>         deviceFormat.size = mode.size;
>         return deviceFormat;
> @@ -598,6 +601,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
>         Size maxSize, sensorSize;
>         unsigned int maxIndex = 0;
>         bool rawStream = false;
> +       bool unpacked = false;
>
>         /*
>          * Look for the RAW stream (if given) size as well as the largest
> @@ -613,6 +617,10 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
>                          */
>                         sensorSize = cfg.size;
>                         rawStream = true;
> +                       /* Check if the user has explicitly set an unpacked format. */
> +                       V4L2PixelFormat fourcc = V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat);
> +                       BayerFormat bayer = BayerFormat::fromV4L2PixelFormat(fourcc);
> +                       unpacked = bayer.packing == BayerFormat::Packing::None;
>                 } else {
>                         if (cfg.size > maxSize) {
>                                 maxSize = config->at(i).size;
> @@ -623,7 +631,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
>
>         /* First calculate the best sensor mode we can use based on the user request. */
>         V4L2SubdeviceFormat sensorFormat = findBestMode(data->sensorFormats_, rawStream ? sensorSize : maxSize);
> -       V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat);
> +       V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat, unpacked);
>
>         ret = data->sensor_->setFormat(&sensorFormat);
>         if (ret)
> --
> 2.25.1
>