[libcamera-devel,RFC,1/5] libcamera: pipeline: uvcvideo: Add UVC metadata node
diff mbox series

Message ID 20230814112849.176943-2-gabbymg94@gmail.com
State Superseded
Headers show
Series
  • RFC:Add UVC Metadata buffer timestamp support
Related show

Commit Message

Gabrielle George Aug. 14, 2023, 11:28 a.m. UTC
Identify and open the UVC metadata's video node. This will give us
access to metadata associated with the video stream of the uvc camera.
The user will not have access to this video node and will not need to
manage its data in any way.

Signed-off-by: Gabby George <gabbymg94@gmail.com>
---
 src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 36 +++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

Comments

Kieran Bingham Aug. 16, 2023, 8:59 p.m. UTC | #1
Hi Gabby,

Quoting Gabby George (2023-08-14 12:28:45)
> Identify and open the UVC metadata's video node. This will give us
> access to metadata associated with the video stream of the uvc camera.
> The user will not have access to this video node and will not need to
> manage its data in any way.
> 
> Signed-off-by: Gabby George <gabbymg94@gmail.com>
> ---
>  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 36 +++++++++++++++++++-
>  1 file changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> index 38f48a5d..4470d8a2 100644
> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> @@ -49,10 +49,13 @@ public:
>         const std::string &id() const { return id_; }
>  
>         std::unique_ptr<V4L2VideoDevice> video_;
> +       std::unique_ptr<V4L2VideoDevice> metadata_;
>         Stream stream_;
>         std::map<PixelFormat, std::vector<SizeRange>> formats_;
>  
>  private:
> +       int initMetadata(MediaDevice *media);
> +
>         bool generateId();
>  
>         std::string id_;
> @@ -411,6 +414,37 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)
>         return true;
>  }
>  
> +int UVCCameraData::initMetadata(MediaDevice *media)
> +{
> +       int ret;
> +
> +       const std::vector<MediaEntity *> &entities = media->entities();
> +       std::string dev_node_name = video_->deviceNode();
> +       auto metadata = std::find_if(entities.begin(), entities.end(),
> +                                    [&dev_node_name](MediaEntity *e) {
> +                                            return e->type() == MediaEntity::Type::V4L2VideoDevice && !(e->flags() & MEDIA_ENT_FL_DEFAULT);
> +                                    });
> +
> +       if (metadata == entities.end()) {
> +               LOG(UVC, Error) << "Could not find a metadata video device.";
> +               return -ENODEV;
> +       }
> +
> +       /* configure the metadata node */
> +       metadata_ = std::make_unique<V4L2VideoDevice>(*metadata);
> +       ret = metadata_->open();
> +       if (ret)
> +               return ret;
> +
> +       if (!(metadata_->caps().isMeta())) {
> +               /* if the caps do not have the metadata attribute
> +                * (shouldn't happen) */

Keep an eye on the coding styles, multiline comments should be formatted
differently. Try not to be too succint with comments, we like to keep to
full sentences with valid grammar in the code base. Any comments should
explain reasoning and the situation.

Perhaps:

	/*
	 * UVC Devices are usually only anticipated to expose two
	 * devices, so we assume the non-default device is the metadata
	 * device node.
	 */

I do wonder if there are more complex devices out there that might break
this assumption, though I haven't personally come across any.

Laurent - any feedback here? Can we continue with this assumption? or do
we have to do a full search here, opening every entity to check each
caps?

> +               metadata_ = NULL;
> +               return -EINVAL;
> +       }
> +       return 0;
> +}
> +
>  int UVCCameraData::init(MediaDevice *media)
>  {
>         int ret;
> @@ -512,7 +546,7 @@ int UVCCameraData::init(MediaDevice *media)
>  
>         controlInfo_ = ControlInfoMap(std::move(ctrls), controls::controls);
>  
> -       return 0;
> +       return initMetadata(media);

Hrm .... if this fails - we can no longer open the camera at all.

Maybe we should just report an error and continue without the metadata
node just as we already do?

--
Kieran


>  }
>  
>  bool UVCCameraData::generateId()
> -- 
> 2.34.1
>

Patch
diff mbox series

diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
index 38f48a5d..4470d8a2 100644
--- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
+++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
@@ -49,10 +49,13 @@  public:
 	const std::string &id() const { return id_; }
 
 	std::unique_ptr<V4L2VideoDevice> video_;
+	std::unique_ptr<V4L2VideoDevice> metadata_;
 	Stream stream_;
 	std::map<PixelFormat, std::vector<SizeRange>> formats_;
 
 private:
+	int initMetadata(MediaDevice *media);
+
 	bool generateId();
 
 	std::string id_;
@@ -411,6 +414,37 @@  bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)
 	return true;
 }
 
+int UVCCameraData::initMetadata(MediaDevice *media)
+{
+	int ret;
+
+	const std::vector<MediaEntity *> &entities = media->entities();
+	std::string dev_node_name = video_->deviceNode();
+	auto metadata = std::find_if(entities.begin(), entities.end(),
+				     [&dev_node_name](MediaEntity *e) {
+					     return e->type() == MediaEntity::Type::V4L2VideoDevice && !(e->flags() & MEDIA_ENT_FL_DEFAULT);
+				     });
+
+	if (metadata == entities.end()) {
+		LOG(UVC, Error) << "Could not find a metadata video device.";
+		return -ENODEV;
+	}
+
+	/* configure the metadata node */
+	metadata_ = std::make_unique<V4L2VideoDevice>(*metadata);
+	ret = metadata_->open();
+	if (ret)
+		return ret;
+
+	if (!(metadata_->caps().isMeta())) {
+		/* if the caps do not have the metadata attribute
+		 * (shouldn't happen) */
+		metadata_ = NULL;
+		return -EINVAL;
+	}
+	return 0;
+}
+
 int UVCCameraData::init(MediaDevice *media)
 {
 	int ret;
@@ -512,7 +546,7 @@  int UVCCameraData::init(MediaDevice *media)
 
 	controlInfo_ = ControlInfoMap(std::move(ctrls), controls::controls);
 
-	return 0;
+	return initMetadata(media);
 }
 
 bool UVCCameraData::generateId()