[PATCH v6 4/5] libcamera: converter: Add DW100 converter class

[Laurent Pinchart]

Hi Umang,

Thank you for the patch.

On Fri, Jul 26, 2024 at 05:17:14PM +0530, Umang Jain wrote:
> DW100 Dewarp engine is present on i.MX8MP SoC. This patch
> provides a converter class (inherited from V4L2M2MConverter).
> 
> The DW100 dewarp engine has scaling capabilites and can be used

Sounds like you should expose a scaling feature flag then, it could come
handy later.

> to set crop rectangles in the rkisp1 pipeline handler. Plumbing
> in the RkISP1 pipeline-handler is done in the subsequent patch.
> 
> The ConverterDW100 class can be extended later (as additional
> development) to support the dewarping by setting a dewarp vertex
> map as well.
> 
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
>  .../internal/converter/converter_dw100.h      | 24 ++++++++++++
>  .../libcamera/internal/converter/meson.build  |  1 +
>  src/libcamera/converter/converter_dw100.cpp   | 37 +++++++++++++++++++
>  src/libcamera/converter/meson.build           |  1 +
>  4 files changed, 63 insertions(+)
>  create mode 100644 include/libcamera/internal/converter/converter_dw100.h
>  create mode 100644 src/libcamera/converter/converter_dw100.cpp
> 
> diff --git a/include/libcamera/internal/converter/converter_dw100.h b/include/libcamera/internal/converter/converter_dw100.h
> new file mode 100644
> index 00000000..dc41f365
> --- /dev/null
> +++ b/include/libcamera/internal/converter/converter_dw100.h
> @@ -0,0 +1,24 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2024, Ideas On Board Oy
> + *
> + * i.MX8MP Dewarp Engine integration
> + */
> +
> +#pragma once
> +
> +#include "libcamera/internal/converter/converter_v4l2_m2m.h"
> +
> +namespace libcamera {
> +
> +class MediaDevice;
> +class Rectangle;
> +class Stream;

The last two are not needed.

> +
> +class ConverterDW100 : public V4L2M2MConverter
> +{
> +public:
> +	ConverterDW100(std::shared_ptr<MediaDevice> media);

Why a shared pointer ?

> +};
> +
> +} /* namespace libcamera */
> diff --git a/include/libcamera/internal/converter/meson.build b/include/libcamera/internal/converter/meson.build
> index 891e79e7..85007a4b 100644
> --- a/include/libcamera/internal/converter/meson.build
> +++ b/include/libcamera/internal/converter/meson.build
> @@ -1,5 +1,6 @@
>  # SPDX-License-Identifier: CC0-1.0
>  
>  libcamera_internal_headers += files([
> +    'converter_dw100.h',
>      'converter_v4l2_m2m.h',
>  ])
> diff --git a/src/libcamera/converter/converter_dw100.cpp b/src/libcamera/converter/converter_dw100.cpp
> new file mode 100644
> index 00000000..eb9906f0
> --- /dev/null
> +++ b/src/libcamera/converter/converter_dw100.cpp
> @@ -0,0 +1,37 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2024, Ideas On Board Oy
> + *
> + * i.MX8MP Dewarp Engine integration
> + */
> +
> +#include "libcamera/internal/converter/converter_dw100.h"
> +
> +#include <libcamera/base/log.h>
> +
> +#include <libcamera/geometry.h>
> +
> +#include "libcamera/internal/media_device.h"
> +#include "libcamera/internal/v4l2_videodevice.h"
> +
> +namespace libcamera {
> +
> +LOG_DECLARE_CATEGORY(Converter)
> +
> +/**
> + * \class libcamera::ConverterDW100
> + * \brief The i.MX8MP dewarp converter implements the converter interface based
> + * on V4L2 M2M device.
> +*/
> +
> +/**
> + * \fn ConverterDW100::ConverterDW100
> + * \brief Construct a ConverterDW100 instance
> + * \param[in] media The media device implementing the converter
> + */
> +ConverterDW100::ConverterDW100(std::shared_ptr<MediaDevice> media)
> +	: V4L2M2MConverter(media.get(), Feature::InputCrop)

Can't you query support for the input crop feature in the
V4L2M2MConverter class and avoid subclassing ? I know we should add
support for dewarping later, I think the subclass should be introduced
at that point. Dynamic discovery of features will be useful for other
converters.

> +{
> +}
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/converter/meson.build b/src/libcamera/converter/meson.build
> index 2aa72fe4..175581b8 100644
> --- a/src/libcamera/converter/meson.build
> +++ b/src/libcamera/converter/meson.build
> @@ -1,5 +1,6 @@
>  # SPDX-License-Identifier: CC0-1.0
>  
>  libcamera_sources += files([
> +        'converter_dw100.cpp',
>          'converter_v4l2_m2m.cpp'
>  ])

-- 
Regards,

Laurent Pinchart