[libcamera-devel] [PATCH 2/2] libcamera: pipeline: uvcvideo: create a V4L2Device for the default video entity

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Wed, Jan 23, 2019 at 04:03:51PM +0100, Niklas Söderlund wrote:
> Add a V4L2Device for the default video entity in the media graph. The
> UVC pipeline needs to search for the entity marked with the
> MEDIA_ENT_FL_DEFAULT flag as the entity names in the media graph varies
> depending on which device is used.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  src/libcamera/pipeline/uvcvideo.cpp | 26 +++++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> index b56ee9881239abdc..f853105f50a958a3 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -12,6 +12,7 @@
>  #include "log.h"
>  #include "media_device.h"
>  #include "pipeline_handler.h"
> +#include "v4l2_device.h"
>  
>  namespace libcamera {
>  
> @@ -31,15 +32,21 @@ public:
>  
>  private:
>  	MediaDevice *dev_;
> +	V4L2Device *v4l2dev_;

Nitpicking, how about renaming dev_ to media_ and naming v4l2dev_
either v4l2_ or video_ ?

>  };
>  
>  PipelineHandlerUVC::PipelineHandlerUVC()
> -	: dev_(nullptr)
> +	: dev_(nullptr), v4l2dev_(nullptr)
>  {
>  }
>  
>  PipelineHandlerUVC::~PipelineHandlerUVC()
>  {
> +	if (v4l2dev_) {
> +		v4l2dev_->close();
> +		delete v4l2dev_;

Doesn't the V4L2Device destructor close the device already ?

> +	}
> +
>  	if (dev_)
>  		dev_->release();
>  }
> @@ -83,6 +90,23 @@ bool PipelineHandlerUVC::match(CameraManager *manager, DeviceEnumerator *enumera
>  
>  	dev_->acquire();
>  
> +	for (MediaEntity *entity : dev_->entities()) {
> +		if (!(entity->flags() & MEDIA_ENT_FL_DEFAULT))
> +			continue;
> +
> +		v4l2dev_ = new V4L2Device(*entity);
> +
> +		if (v4l2dev_->open())
> +			delete v4l2dev_;

The PipelineHandlerUVC destructor will delete v4l2dev_, there's no need
to do it manually here.

Apart from this,

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

> +
> +		break;
> +	}
> +
> +	if (!v4l2dev_) {
> +		dev_->release();
> +		return false;
> +	}
> +
>  	std::shared_ptr<Camera> camera = Camera::create(this, dev_->model());
>  	registerCamera(manager, std::move(camera));
>  

-- 
Regards,

Laurent Pinchart