[libcamera-devel] [PATCH 1/5] pipeline: raspberrypi: Refactor stream configuration routine

[David Plowman]

Hi Naush

Thanks for this patch. No issues with this from me!

On Wed, 20 Jan 2021 at 08:34, Naushir Patuck <naush at raspberrypi.com> wrote:
>
> Refactor the high/low resolution stream format and output selection
> routine. This change is in preparation of adding 1/4 resolution output
> for fast colour denoise.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>

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

Best regards
David

> ---
>  .../pipeline/raspberrypi/raspberrypi.cpp      | 57 ++++++-------------
>  1 file changed, 16 insertions(+), 41 deletions(-)
>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index f121328ee9a9..e03bcb036f9f 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -624,52 +624,27 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
>                         continue;
>                 }
>
> -               if (i == maxIndex) {
> -                       /* ISP main output format. */
> -                       V4L2VideoDevice *dev = data->isp_[Isp::Output0].dev();
> -                       V4L2PixelFormat fourcc = dev->toV4L2PixelFormat(cfg.pixelFormat);
> -                       format.size = cfg.size;
> -                       format.fourcc = fourcc;
> -
> -                       ret = dev->setFormat(&format);
> -                       if (ret)
> -                               return -EINVAL;
> -
> -                       if (format.size != cfg.size || format.fourcc != fourcc) {
> -                               LOG(RPI, Error)
> -                                       << "Failed to set format on ISP capture0 device: "
> -                                       << format.toString();
> -                               return -EINVAL;
> -                       }
> -
> -                       cfg.setStream(&data->isp_[Isp::Output0]);
> -                       data->isp_[Isp::Output0].setExternal(true);
> -               }
> +               /* The largest resolution gets routed to the ISP Output 0 node. */
> +               RPi::Stream *stream = (i == maxIndex) ? &data->isp_[Isp::Output0] :
> +                                                       &data->isp_[Isp::Output1];
>
> -               /*
> -                * ISP second output format. This fallthrough means that if a
> -                * second output stream has not been configured, we simply use
> -                * the Output0 configuration.
> -                */
> -               V4L2VideoDevice *dev = data->isp_[Isp::Output1].dev();
> -               format.fourcc = dev->toV4L2PixelFormat(cfg.pixelFormat);
> +               V4L2PixelFormat fourcc = stream->dev()->toV4L2PixelFormat(cfg.pixelFormat);
>                 format.size = cfg.size;
> +               format.fourcc = fourcc;
>
> -               ret = dev->setFormat(&format);
> -               if (ret) {
> +               ret = stream->dev()->setFormat(&format);
> +               if (ret)
> +                       return -EINVAL;
> +
> +               if (format.size != cfg.size || format.fourcc != fourcc) {
>                         LOG(RPI, Error)
> -                               << "Failed to set format on ISP capture1 device: "
> -                               << format.toString();
> -                       return ret;
> -               }
> -               /*
> -                * If we have not yet provided a stream for this config, it
> -                * means this is to be routed from Output1.
> -                */
> -               if (!cfg.stream()) {
> -                       cfg.setStream(&data->isp_[Isp::Output1]);
> -                       data->isp_[Isp::Output1].setExternal(true);
> +                               << "Failed to set format on " << stream->name()
> +                               << " to " << format.toString();
> +                       return -EINVAL;
>                 }
> +
> +               cfg.setStream(stream);
> +               stream->setExternal(true);
>         }
>
>         /* ISP statistics output format. */
> --
> 2.25.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel