[libcamera-devel] [PATCH v2 11/12] libcamera: pipeline: vimc: add pipeline handler for vimc

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Saturday, 29 December 2018 05:28:54 EET Niklas Söderlund wrote:
> Provide a pipeline handler for the virtual vimc driver.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  src/libcamera/meson.build          |  2 +
>  src/libcamera/pipeline/meson.build |  3 +
>  src/libcamera/pipeline/vimc.cpp    | 91 ++++++++++++++++++++++++++++++
>  3 files changed, 96 insertions(+)
>  create mode 100644 src/libcamera/pipeline/meson.build
>  create mode 100644 src/libcamera/pipeline/vimc.cpp
> 
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index a8cb3fdc22784334..ac5bba052d7f687b 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -21,6 +21,8 @@ includes = [
>      libcamera_internal_includes,
>  ]
> 
> +subdir('pipeline')
> +
>  libudev = dependency('libudev')
> 
>  libcamera = shared_library('camera',
> diff --git a/src/libcamera/pipeline/meson.build
> b/src/libcamera/pipeline/meson.build new file mode 100644
> index 0000000000000000..615ecd20f1a21141
> --- /dev/null
> +++ b/src/libcamera/pipeline/meson.build
> @@ -0,0 +1,3 @@
> +libcamera_sources += files([
> +    'vimc.cpp',
> +])
> diff --git a/src/libcamera/pipeline/vimc.cpp
> b/src/libcamera/pipeline/vimc.cpp new file mode 100644
> index 0000000000000000..e0f70a78f7e88412
> --- /dev/null
> +++ b/src/libcamera/pipeline/vimc.cpp
> @@ -0,0 +1,91 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2018, Google Inc.
> + *
> + * vimc.cpp - Pipeline handler for the vimc device
> + */
> +
> +#include <libcamera/camera.h>
> +
> +#include "device_enumerator.h"
> +#include "pipeline_handler.h"
> +
> +namespace libcamera {
> +
> +class PipeHandlerVimc : public PipelineHandler
> +{
> +public:
> +	PipeHandlerVimc();
> +	~PipeHandlerVimc();
> +
> +	bool match(DeviceEnumerator *enumerator);
> +
> +	unsigned int count();
> +	Camera *camera(unsigned int id);

Please declare overridden functions as override or final.

I'd add a blank line here.

> +private:
> +	DeviceInfo *info_;
> +	Camera *camera_;
> +};
> +
> +PipeHandlerVimc::PipeHandlerVimc()
> +	: info_(nullptr), camera_(nullptr)
> +{
> +}
> +
> +PipeHandlerVimc::~PipeHandlerVimc()
> +{
> +	if (camera_)
> +		camera_->put();
> +
> +	if (info_)
> +		info_->release();
> +}
> +
> +unsigned int PipeHandlerVimc::count()
> +{
> +	return 1;
> +}
> +
> +Camera *PipeHandlerVimc::camera(unsigned int id)
> +{
> +	if (id != 0)
> +		return nullptr;
> +
> +	return camera_;
> +}
> +
> +bool PipeHandlerVimc::match(DeviceEnumerator *enumerator)
> +{
> +	DeviceMatch dm("vimc");
> +
> +	dm.add("Raw Capture 0");
> +	dm.add("Raw Capture 1");
> +	dm.add("RGB/YUV Capture");
> +	dm.add("Sensor A");
> +	dm.add("Sensor B");
> +	dm.add("Debayer A");
> +	dm.add("Debayer B");
> +	dm.add("RGB/YUV Input");
> +	dm.add("Scaler");
> +
> +	info_ = enumerator->search(dm);
> +
> +	if (!info_)
> +		return false;
> +
> +	info_->acquire();
> +
> +	/* NOTE: A more complete Camera implementation could

s/ NOTE/\n\t* NOTE/

> +	 * be passed the DeviceInfo(s) it controls here or
> +	 * a reference to the PipelineHandler. Which method
> +	 * that is chosen will depend on how the Camera

s/that is chosen will depend/will be chosen depends/ ?

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> +	 * object is modeled.
> +	 */
> +	camera_ = new Camera("Dummy VIMC Camera");
> +
> +	return true;
> +}
> +
> +REGISTER_PIPELINE_HANDLER(PipeHandlerVimc);
> +
> +} /* namespace libcamera */

-- 
Regards,

Laurent Pinchart