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

[Niklas Söderlund]

Hi Laurent,

Thanks for your feedback.

On 2018-12-31 01:26:04 +0200, Laurent Pinchart wrote:
> 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>

All comments addressed.

Thanks for your tag and time i reviewing this series.

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

-- 
Regards,
Niklas Söderlund