[libcamera-devel] [PATCH 09/14] libcamera: converter: Introduce dw100 converter

[Laurent Pinchart]

Hi Xavier,

Thank you for the patch.

On Thu, Sep 08, 2022 at 08:48:45PM +0200, Xavier Roumegue via libcamera-devel wrote:
> Add converter support for the Vivante DW100 Dewarp Processor IP core
> found on i.MX8MP SoC.
> 
> The processor core applies a programmable geometrical transformation on
> input images to correct distorsion introduced by lenses.
> The transformation function is exposed as a grid map with 16x16 pixel
> macroblocks indexed using X, Y vertex coordinates.
> 
> A set of input/output vertices mapping can be loaded at runtime through
> a configuration file. If no mapping is loaded, the vertices mapping
> fallbacks to an identity transformation. Scaling and pixel format
> conversion can be used independently of the vertices remapping.
> 
> Signed-off-by: Xavier Roumegue <xavier.roumegue at oss.nxp.com>
> ---
>  include/libcamera/internal/converter_dw100.h | 25 +++++++++++++++
>  include/libcamera/internal/meson.build       |  1 +
>  src/libcamera/converter_dw100.cpp            | 32 ++++++++++++++++++++
>  src/libcamera/meson.build                    |  1 +
>  4 files changed, 59 insertions(+)
>  create mode 100644 include/libcamera/internal/converter_dw100.h
>  create mode 100644 src/libcamera/converter_dw100.cpp
> 
> diff --git a/include/libcamera/internal/converter_dw100.h b/include/libcamera/internal/converter_dw100.h
> new file mode 100644
> index 00000000..1972d6a2
> --- /dev/null
> +++ b/include/libcamera/internal/converter_dw100.h
> @@ -0,0 +1,25 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright 2022 NXP
> + *
> + * converter_dw100.h - V4l2 M2M dw100 format converter interface
> + */
> +
> +#pragma once
> +
> +#include <linux/dw100.h>
> +
> +#include "libcamera/internal/converter_v4l2_m2m.h"
> +#include "libcamera/internal/media_device.h"

You can drop this header, it's guaranteed to be included by
libcamera/internal/converter_v4l2_m2m.h as the MediaDevice is passed to
the V4L2M2MConverter constructor.

> +
> +namespace libcamera {
> +
> +class DW100Converter : public V4L2M2MConverter
> +{
> +public:
> +	DW100Converter(MediaDevice *media)
> +		: V4L2M2MConverter(media){};

	DW100Converter(MediaDevice *media)
		: V4L2M2MConverter(media)
	{
	};

> +	virtual int applyMapping(Stream *stream, Mapping &mapping) override;

s/virtual //

> +};
> +
> +} /* namespace libcamera */
> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build
> index 132de5ef..c2dd094f 100644
> --- a/include/libcamera/internal/meson.build
> +++ b/include/libcamera/internal/meson.build
> @@ -20,6 +20,7 @@ libcamera_internal_headers = files([
>      'control_serializer.h',
>      'control_validator.h',
>      'converter.h',
> +    'converter_dw100.h',
>      'converter_v4l2_m2m.h',
>      'delayed_controls.h',
>      'device_enumerator.h',
> diff --git a/src/libcamera/converter_dw100.cpp b/src/libcamera/converter_dw100.cpp
> new file mode 100644
> index 00000000..b079fb37
> --- /dev/null
> +++ b/src/libcamera/converter_dw100.cpp
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright 2022 NXP
> + *
> + * converter_dw100.cpp - V4L2 M2M dw100 format converter
> + */
> +
> +#include "libcamera/internal/converter_dw100.h"
> +
> +#include <libcamera/base/log.h>
> +
> +#include <libcamera/controls.h>
> +
> +#include "libcamera/internal/v4l2_videodevice.h"
> +
> +namespace libcamera {
> +
> +LOG_DECLARE_CATEGORY(Converter)

Not used.

> +
> +int DW100Converter::applyMapping(Stream *stream, Mapping &mapping)
> +{
> +	ControlList ctrls;
> +	auto value = Span<const int32_t>(reinterpret_cast<const int32_t *>(mapping.getMapping()), mapping.getLength());
> +	ControlValue c(value);
> +	ctrls.set(V4L2_CID_DW100_DEWARPING_16x16_VERTEX_MAP, c);
> +	stream->m2m_->capture()->setControls(&ctrls);
> +	return 0;
> +}

As we have a single converter that can perform dewarping, I'd prefer
moving the Mapping support from V4L2M2MConverter to this class. It's
hard to predict what other dewarpers will need, and designing an API
based on a single example usually doesn't produce the best design. We
can always refactor the code later when we'll get a second device.

> +
> +REGISTER_CONVERTER("dw100", DW100Converter)
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index b12c8401..83da3e5f 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -14,6 +14,7 @@ libcamera_sources = files([
>      'control_serializer.cpp',
>      'control_validator.cpp',
>      'converter.cpp',
> +    'converter_dw100.cpp',
>      'converter_v4l2_m2m.cpp',
>      'delayed_controls.cpp',
>      'device_enumerator.cpp',

-- 
Regards,

Laurent Pinchart