[libcamera-devel] [PATCH 2/2] libcamera: pipeline: uvcvideo: add pipeline handler for uvcvideo

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Mon, Jan 21, 2019 at 04:30:05PM +0100, Niklas Söderlund wrote:
> Provide a pipeline handler for uvcvideo devices. The entity names for
> uvc devices are different for different cameras so matching on entity
> names is not possible in the generic case. This leaves options to create
> specialized uvc pipeline handlers if needed to fit a specific models

s/uvc/UVC/
s/models/model's/

> needs.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  src/libcamera/pipeline/meson.build  |  1 +
>  src/libcamera/pipeline/uvcvideo.cpp | 59 +++++++++++++++++++++++++++++
>  2 files changed, 60 insertions(+)
>  create mode 100644 src/libcamera/pipeline/uvcvideo.cpp
> 
> diff --git a/src/libcamera/pipeline/meson.build b/src/libcamera/pipeline/meson.build
> index 615ecd20f1a21141..4da5d8cf9c4384bb 100644
> --- a/src/libcamera/pipeline/meson.build
> +++ b/src/libcamera/pipeline/meson.build
> @@ -1,3 +1,4 @@
>  libcamera_sources += files([
> +    'uvcvideo.cpp',
>      'vimc.cpp',
>  ])
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> new file mode 100644
> index 0000000000000000..2579782e90caec5a
> --- /dev/null
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -0,0 +1,59 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * uvcvideo.cpp - Pipeline handler for uvcvideo devices
> + */
> +
> +#include <libcamera/camera.h>
> +#include <libcamera/camera_manager.h>
> +
> +#include "device_enumerator.h"
> +#include "media_device.h"
> +#include "pipeline_handler.h"
> +
> +namespace libcamera {
> +
> +class PipeHandlerUvcvideo : public PipelineHandler

How about PipelineHandlerUVC to retain the common prefix ?

> +{
> +public:
> +	PipeHandlerUvcvideo();
> +	~PipeHandlerUvcvideo();
> +
> +	bool match(CameraManager *manager, DeviceEnumerator *enumerator);
> +
> +private:
> +	MediaDevice *dev_;
> +};
> +
> +PipeHandlerUvcvideo::PipeHandlerUvcvideo()
> +	: dev_(nullptr)
> +{
> +}
> +
> +PipeHandlerUvcvideo::~PipeHandlerUvcvideo()
> +{
> +	if (dev_)
> +		dev_->release();
> +}
> +
> +bool PipeHandlerUvcvideo::match(CameraManager *manager, DeviceEnumerator *enumerator)
> +{
> +	DeviceMatch dm("uvcvideo");
> +
> +	dev_ = enumerator->search(dm);
> +
> +	if (!dev_)
> +		return false;
> +
> +	dev_->acquire();
> +
> +	std::shared_ptr<Camera> camera = Camera::create(dev_->model());

Does this guarantee unicity of the name ? I think we'll need more than
that, but it can probably be fixed later. I foresee the need for a
machine-parsable name and a human-readable description, along the lines
of "uvcvideo/1-5:1.0" and "Integrated_Webcam_HD", or
"uvcvideo/3-1.3:1.0" and "Logitech Portable Webcam C905 on USB port
3-1.3" (those are real examples).

> +	manager->addCamera(std::move(camera));
> +
> +	return true;

While this doesn't do much, it's a stumb that allows enumerating cameras
on most development platforms, so it's very useful as such. Let's get it
in, with the above issues fixed,

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

> +}
> +
> +REGISTER_PIPELINE_HANDLER(PipeHandlerUvcvideo);
> +
> +} /* namespace libcamera */

-- 
Regards,

Laurent Pinchart