[libcamera-devel] [PATCH 1/5] libcamera: media_device: Provide the default entity

[Jacopo Mondi]

Hi Kieran,
    ignorant question, is FL_DEFAULT an UVC specific thing?

A quick grep on kernel sources returns:

$ git grep FL_DEFAULT drivers/media/
drivers/media/platform/omap3isp/ispresizer.c:   res->video_out.video.entity.flags |= MEDIA_ENT_FL_DEFAULT;
drivers/media/usb/uvc/uvc_entity.c:             entity->vdev->entity.flags |= MEDIA_ENT_FL_DEFAULT;

So I guess not, as omap3 seems also omap3 uses that as well.

If that's not a UVC specificity, I welcome this change.

On Thu, Feb 07, 2019 at 09:21:15PM +0000, Kieran Bingham wrote:
> Add a helper to identify the default entity from a media device.
>
> Utilise it in the two places which iterate the entities manually, to allocated
> a V4L2Device from the default entity.
>
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>  src/libcamera/include/media_device.h  |  1 +
>  src/libcamera/media_device.cpp        | 15 +++++++++++++++
>  src/libcamera/pipeline/uvcvideo.cpp   | 10 ++++------
>  test/v4l2_device/v4l2_device_test.cpp | 12 +++++-------
>  4 files changed, 25 insertions(+), 13 deletions(-)
>
> diff --git a/src/libcamera/include/media_device.h b/src/libcamera/include/media_device.h
> index 9f038093b2b2..361e8f4a4b86 100644
> --- a/src/libcamera/include/media_device.h
> +++ b/src/libcamera/include/media_device.h
> @@ -42,6 +42,7 @@ public:
>
>  	const std::vector<MediaEntity *> &entities() const { return entities_; }
>  	MediaEntity *getEntityByName(const std::string &name) const;
> +	MediaEntity *defaultEntity() const;
>
>  	MediaLink *link(const std::string &sourceName, unsigned int sourceIdx,
>  			const std::string &sinkName, unsigned int sinkIdx);
> diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp
> index 800ed330fe6d..4af90e1590a1 100644
> --- a/src/libcamera/media_device.cpp
> +++ b/src/libcamera/media_device.cpp
> @@ -319,6 +319,21 @@ MediaEntity *MediaDevice::getEntityByName(const std::string &name) const
>  	return nullptr;
>  }
>
> +/**
> + * \brief Return the default MediaEntity within a MediaDevice
> + * \return The default entity if specified, or a nullptr otherwise
> + */
> +MediaEntity *MediaDevice::defaultEntity() const
> +{
> +	for (MediaEntity *entity : entities_) {
> +		if (entity->flags() & MEDIA_ENT_FL_DEFAULT) {
> +			return entity;
> +		}

You could drop the inner braces { } here
> +	}
> +
> +	return nullptr;
> +}
> +
>  /**
>   * \brief Retrieve the MediaLink connecting two pads, identified by entity
>   * names and pad indexes
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> index fc31c52c0ecd..ed8228bb2fc6 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -146,13 +146,11 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)
>
>  	media_->acquire();
>
> -	for (MediaEntity *entity : media_->entities()) {
> -		if (entity->flags() & MEDIA_ENT_FL_DEFAULT) {
> -			video_ = new V4L2Device(entity);
> -			break;
> -		}
> -	}
> +	MediaEntity *entity = media_->defaultEntity();
> +	if (!entity)

Looking at existing code if you don't find any DEFAULT entity, you
return false as you do here, but also release the media device.

Should the same be done here?

> +		return false;
>
> +	video_ = new V4L2Device(entity);
>  	if (!video_ || video_->open()) {
>  		if (!video_)
>  			LOG(UVC, Error) << "Could not find a default video device";
> diff --git a/test/v4l2_device/v4l2_device_test.cpp b/test/v4l2_device/v4l2_device_test.cpp
> index 18d014caf4c8..dd28cccada6b 100644
> --- a/test/v4l2_device/v4l2_device_test.cpp
> +++ b/test/v4l2_device/v4l2_device_test.cpp
> @@ -46,15 +46,13 @@ int V4L2DeviceTest::init()
>
>  	media_->acquire();
>
> -	for (MediaEntity *entity : media_->entities()) {
> -		if (entity->flags() & MEDIA_ENT_FL_DEFAULT) {
> -			dev_ = new V4L2Device(entity);
> -			break;
> -		}
> -	}
> +	MediaEntity *entity = media_->defaultEntity();
> +	if (!entity)
> +		return TestFail;
>
> +	dev_ = new V4L2Device(entity);
>  	if (!dev_)
> -		return TestSkip;
> +		return TestFail;
>
>  	return dev_->open();
>  }
> --
> 2.19.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20190207/f19fbcb5/attachment.sig>