[libcamera-devel,v2,11/12] libcamera: pipeline: vimc: add pipeline handler for vimc
diff mbox series

Message ID 20181229032855.26249-12-niklas.soderlund@ragnatech.se
State Accepted
Headers show
Series
  • Add basic camera enumeration
Related show

Commit Message

Niklas Söderlund Dec. 29, 2018, 3:28 a.m. UTC 
Provide a pipeline handler for the virtual vimc driver.

Signed-off-by: Niklas Söderlund <niklas.soderlund@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

Comments

Jacopo Mondi Dec. 30, 2018, 10:48 a.m. UTC | #1 
Hi Niklas,
   thanks, this is very useful for testing locally

On Sat, Dec 29, 2018 at 04:28:54AM +0100, Niklas Söderlund wrote:
> Provide a pipeline handler for the virtual vimc driver.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund@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);

nit: missing empty line

Otherwise,
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
  j

> +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
> +	 * 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
> +	 * object is modeled.
> +	 */
> +	camera_ = new Camera("Dummy VIMC Camera");
> +
> +	return true;
> +}
> +
> +REGISTER_PIPELINE_HANDLER(PipeHandlerVimc);
> +
> +} /* namespace libcamera */
> --
> 2.20.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Laurent Pinchart Dec. 30, 2018, 11:26 p.m. UTC | #2 
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@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@ideasonboard.com>

> +	 * object is modeled.
> +	 */
> +	camera_ = new Camera("Dummy VIMC Camera");
> +
> +	return true;
> +}
> +
> +REGISTER_PIPELINE_HANDLER(PipeHandlerVimc);
> +
> +} /* namespace libcamera */
Niklas Söderlund Dec. 31, 2018, 12:09 a.m. UTC | #3 
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@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@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
> 
> 
>

Patch
diff mbox series 

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);
+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
+	 * 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
+	 * object is modeled.
+	 */
+	camera_ = new Camera("Dummy VIMC Camera");
+
+	return true;
+}
+
+REGISTER_PIPELINE_HANDLER(PipeHandlerVimc);
+
+} /* namespace libcamera */