[PATCH 2/3] media: uvcvideo: uvc_queue_to_stream(): Support meta queues

Ricardo Ribalda posted 3 patches 1 month ago
[PATCH 2/3] media: uvcvideo: uvc_queue_to_stream(): Support meta queues
Posted by Ricardo Ribalda 1 month ago
The stream data structure has two queues: the metadata and the data
queues, but uvc_queue_to_stream() only supports the data queue. If we
pass the metadata queue the function will return an invalid pointer.

This patch add a parameter to the function to explicitly tell the
function which queue are we using.

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/usb/uvc/uvc_isight.c |  3 ++-
 drivers/media/usb/uvc/uvc_queue.c  | 13 ++++++-------
 drivers/media/usb/uvc/uvcvideo.h   |  4 +++-
 3 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_isight.c b/drivers/media/usb/uvc/uvc_isight.c
index 43cda5e760a3..ea9dc31dfbad 100644
--- a/drivers/media/usb/uvc/uvc_isight.c
+++ b/drivers/media/usb/uvc/uvc_isight.c
@@ -41,7 +41,8 @@ static int isight_decode(struct uvc_video_queue *queue, struct uvc_buffer *buf,
 		0xde, 0xad, 0xfa, 0xce
 	};
 
-	struct uvc_streaming *stream = uvc_queue_to_stream(queue);
+	struct uvc_streaming *stream = uvc_queue_to_stream(queue,
+						V4L2_BUF_TYPE_VIDEO_CAPTURE);
 	unsigned int maxlen, nbytes;
 	u8 *mem;
 	int is_header = 0;
diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c
index 0eddd4f872ca..68ed2883edb2 100644
--- a/drivers/media/usb/uvc/uvc_queue.c
+++ b/drivers/media/usb/uvc/uvc_queue.c
@@ -78,7 +78,7 @@ static int uvc_queue_setup(struct vb2_queue *vq,
 			   unsigned int sizes[], struct device *alloc_devs[])
 {
 	struct uvc_video_queue *queue = vb2_get_drv_priv(vq);
-	struct uvc_streaming *stream;
+	struct uvc_streaming *stream = uvc_queue_to_stream(queue, vq->type);
 	unsigned int size;
 
 	switch (vq->type) {
@@ -87,7 +87,6 @@ static int uvc_queue_setup(struct vb2_queue *vq,
 		break;
 
 	default:
-		stream = uvc_queue_to_stream(queue);
 		size = stream->ctrl.dwMaxVideoFrameSize;
 		break;
 	}
@@ -113,7 +112,7 @@ static int uvc_buffer_prepare(struct vb2_buffer *vb)
 
 	if (vb->type == V4L2_BUF_TYPE_VIDEO_OUTPUT &&
 	    vb2_get_plane_payload(vb, 0) > vb2_plane_size(vb, 0)) {
-		uvc_dbg(uvc_queue_to_stream(queue)->dev, CAPTURE,
+		uvc_dbg(uvc_queue_to_stream(queue, vb->type)->dev, CAPTURE,
 			"[E] Bytes used out of bounds\n");
 		return -EINVAL;
 	}
@@ -160,7 +159,7 @@ static void uvc_buffer_finish(struct vb2_buffer *vb)
 {
 	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
 	struct uvc_video_queue *queue = vb2_get_drv_priv(vb->vb2_queue);
-	struct uvc_streaming *stream = uvc_queue_to_stream(queue);
+	struct uvc_streaming *stream = uvc_queue_to_stream(queue, vb->type);
 	struct uvc_buffer *buf = uvc_vbuf_to_buffer(vbuf);
 
 	if (vb->state == VB2_BUF_STATE_DONE)
@@ -170,7 +169,7 @@ static void uvc_buffer_finish(struct vb2_buffer *vb)
 static int uvc_start_streaming_video(struct vb2_queue *vq, unsigned int count)
 {
 	struct uvc_video_queue *queue = vb2_get_drv_priv(vq);
-	struct uvc_streaming *stream = uvc_queue_to_stream(queue);
+	struct uvc_streaming *stream = uvc_queue_to_stream(queue, vq->type);
 	int ret;
 
 	lockdep_assert_irqs_enabled();
@@ -197,11 +196,11 @@ static int uvc_start_streaming_video(struct vb2_queue *vq, unsigned int count)
 static void uvc_stop_streaming_video(struct vb2_queue *vq)
 {
 	struct uvc_video_queue *queue = vb2_get_drv_priv(vq);
-	struct uvc_streaming *stream = uvc_queue_to_stream(queue);
+	struct uvc_streaming *stream = uvc_queue_to_stream(queue, vq->type);
 
 	lockdep_assert_irqs_enabled();
 
-	uvc_video_stop_streaming(uvc_queue_to_stream(queue));
+	uvc_video_stop_streaming(stream);
 
 	uvc_pm_put(stream->dev);
 
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index 8480d65ecb85..9b4849fda12f 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -703,8 +703,10 @@ static inline int uvc_queue_streaming(struct uvc_video_queue *queue)
 }
 
 static inline struct uvc_streaming *
-uvc_queue_to_stream(struct uvc_video_queue *queue)
+uvc_queue_to_stream(struct uvc_video_queue *queue, unsigned int type)
 {
+	if (type == V4L2_BUF_TYPE_META_CAPTURE)
+		return container_of(queue, struct uvc_streaming, meta.queue);
 	return container_of(queue, struct uvc_streaming, queue);
 }
 

-- 
2.53.0.473.g4a7958ca14-goog
Re: [PATCH 2/3] media: uvcvideo: uvc_queue_to_stream(): Support meta queues
Posted by Laurent Pinchart 3 weeks ago
Hi Ricardo,

Thank you for the patch.

On Mon, Mar 09, 2026 at 03:01:55PM +0000, Ricardo Ribalda wrote:
> The stream data structure has two queues: the metadata and the data
> queues, but uvc_queue_to_stream() only supports the data queue. If we
> pass the metadata queue the function will return an invalid pointer.
> 
> This patch add a parameter to the function to explicitly tell the
> function which queue are we using.
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  drivers/media/usb/uvc/uvc_isight.c |  3 ++-
>  drivers/media/usb/uvc/uvc_queue.c  | 13 ++++++-------
>  drivers/media/usb/uvc/uvcvideo.h   |  4 +++-
>  3 files changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_isight.c b/drivers/media/usb/uvc/uvc_isight.c
> index 43cda5e760a3..ea9dc31dfbad 100644
> --- a/drivers/media/usb/uvc/uvc_isight.c
> +++ b/drivers/media/usb/uvc/uvc_isight.c
> @@ -41,7 +41,8 @@ static int isight_decode(struct uvc_video_queue *queue, struct uvc_buffer *buf,
>  		0xde, 0xad, 0xfa, 0xce
>  	};
>  
> -	struct uvc_streaming *stream = uvc_queue_to_stream(queue);
> +	struct uvc_streaming *stream = uvc_queue_to_stream(queue,
> +						V4L2_BUF_TYPE_VIDEO_CAPTURE);
>  	unsigned int maxlen, nbytes;
>  	u8 *mem;
>  	int is_header = 0;
> diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c
> index 0eddd4f872ca..68ed2883edb2 100644
> --- a/drivers/media/usb/uvc/uvc_queue.c
> +++ b/drivers/media/usb/uvc/uvc_queue.c
> @@ -78,7 +78,7 @@ static int uvc_queue_setup(struct vb2_queue *vq,
>  			   unsigned int sizes[], struct device *alloc_devs[])
>  {
>  	struct uvc_video_queue *queue = vb2_get_drv_priv(vq);
> -	struct uvc_streaming *stream;
> +	struct uvc_streaming *stream = uvc_queue_to_stream(queue, vq->type);
>  	unsigned int size;
>  
>  	switch (vq->type) {
> @@ -87,7 +87,6 @@ static int uvc_queue_setup(struct vb2_queue *vq,
>  		break;
>  
>  	default:
> -		stream = uvc_queue_to_stream(queue);
>  		size = stream->ctrl.dwMaxVideoFrameSize;
>  		break;
>  	}
> @@ -113,7 +112,7 @@ static int uvc_buffer_prepare(struct vb2_buffer *vb)
>  
>  	if (vb->type == V4L2_BUF_TYPE_VIDEO_OUTPUT &&
>  	    vb2_get_plane_payload(vb, 0) > vb2_plane_size(vb, 0)) {
> -		uvc_dbg(uvc_queue_to_stream(queue)->dev, CAPTURE,
> +		uvc_dbg(uvc_queue_to_stream(queue, vb->type)->dev, CAPTURE,
>  			"[E] Bytes used out of bounds\n");
>  		return -EINVAL;
>  	}
> @@ -160,7 +159,7 @@ static void uvc_buffer_finish(struct vb2_buffer *vb)
>  {
>  	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
>  	struct uvc_video_queue *queue = vb2_get_drv_priv(vb->vb2_queue);
> -	struct uvc_streaming *stream = uvc_queue_to_stream(queue);
> +	struct uvc_streaming *stream = uvc_queue_to_stream(queue, vb->type);
>  	struct uvc_buffer *buf = uvc_vbuf_to_buffer(vbuf);
>  
>  	if (vb->state == VB2_BUF_STATE_DONE)
> @@ -170,7 +169,7 @@ static void uvc_buffer_finish(struct vb2_buffer *vb)
>  static int uvc_start_streaming_video(struct vb2_queue *vq, unsigned int count)
>  {
>  	struct uvc_video_queue *queue = vb2_get_drv_priv(vq);
> -	struct uvc_streaming *stream = uvc_queue_to_stream(queue);
> +	struct uvc_streaming *stream = uvc_queue_to_stream(queue, vq->type);
>  	int ret;
>  
>  	lockdep_assert_irqs_enabled();
> @@ -197,11 +196,11 @@ static int uvc_start_streaming_video(struct vb2_queue *vq, unsigned int count)
>  static void uvc_stop_streaming_video(struct vb2_queue *vq)
>  {
>  	struct uvc_video_queue *queue = vb2_get_drv_priv(vq);
> -	struct uvc_streaming *stream = uvc_queue_to_stream(queue);
> +	struct uvc_streaming *stream = uvc_queue_to_stream(queue, vq->type);
>  
>  	lockdep_assert_irqs_enabled();
>  
> -	uvc_video_stop_streaming(uvc_queue_to_stream(queue));
> +	uvc_video_stop_streaming(stream);
>  
>  	uvc_pm_put(stream->dev);
>  
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index 8480d65ecb85..9b4849fda12f 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -703,8 +703,10 @@ static inline int uvc_queue_streaming(struct uvc_video_queue *queue)
>  }
>  
>  static inline struct uvc_streaming *
> -uvc_queue_to_stream(struct uvc_video_queue *queue)
> +uvc_queue_to_stream(struct uvc_video_queue *queue, unsigned int type)
>  {
> +	if (type == V4L2_BUF_TYPE_META_CAPTURE)
> +		return container_of(queue, struct uvc_streaming, meta.queue);
>  	return container_of(queue, struct uvc_streaming, queue);

This was implemented with container_of() as there has never been a need
to get the uvc_streaming for the metadata queue. As that's changing in
patch 3/3, I'd rather use a backpointer from uvc_video_queue to
uvc_streaming. That will be simpler for the callers, and less
error-prone.

>  }
>  
> 

-- 
Regards,

Laurent Pinchart
Re: [PATCH 2/3] media: uvcvideo: uvc_queue_to_stream(): Support meta queues
Posted by Hans de Goede 3 weeks, 2 days ago
Hi,

On 9-Mar-26 4:01 PM, Ricardo Ribalda wrote:
> The stream data structure has two queues: the metadata and the data
> queues, but uvc_queue_to_stream() only supports the data queue. If we
> pass the metadata queue the function will return an invalid pointer.
> 
> This patch add a parameter to the function to explicitly tell the
> function which queue are we using.
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>

Regards,

Hans


> ---
>  drivers/media/usb/uvc/uvc_isight.c |  3 ++-
>  drivers/media/usb/uvc/uvc_queue.c  | 13 ++++++-------
>  drivers/media/usb/uvc/uvcvideo.h   |  4 +++-
>  3 files changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_isight.c b/drivers/media/usb/uvc/uvc_isight.c
> index 43cda5e760a3..ea9dc31dfbad 100644
> --- a/drivers/media/usb/uvc/uvc_isight.c
> +++ b/drivers/media/usb/uvc/uvc_isight.c
> @@ -41,7 +41,8 @@ static int isight_decode(struct uvc_video_queue *queue, struct uvc_buffer *buf,
>  		0xde, 0xad, 0xfa, 0xce
>  	};
>  
> -	struct uvc_streaming *stream = uvc_queue_to_stream(queue);
> +	struct uvc_streaming *stream = uvc_queue_to_stream(queue,
> +						V4L2_BUF_TYPE_VIDEO_CAPTURE);
>  	unsigned int maxlen, nbytes;
>  	u8 *mem;
>  	int is_header = 0;
> diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c
> index 0eddd4f872ca..68ed2883edb2 100644
> --- a/drivers/media/usb/uvc/uvc_queue.c
> +++ b/drivers/media/usb/uvc/uvc_queue.c
> @@ -78,7 +78,7 @@ static int uvc_queue_setup(struct vb2_queue *vq,
>  			   unsigned int sizes[], struct device *alloc_devs[])
>  {
>  	struct uvc_video_queue *queue = vb2_get_drv_priv(vq);
> -	struct uvc_streaming *stream;
> +	struct uvc_streaming *stream = uvc_queue_to_stream(queue, vq->type);
>  	unsigned int size;
>  
>  	switch (vq->type) {
> @@ -87,7 +87,6 @@ static int uvc_queue_setup(struct vb2_queue *vq,
>  		break;
>  
>  	default:
> -		stream = uvc_queue_to_stream(queue);
>  		size = stream->ctrl.dwMaxVideoFrameSize;
>  		break;
>  	}
> @@ -113,7 +112,7 @@ static int uvc_buffer_prepare(struct vb2_buffer *vb)
>  
>  	if (vb->type == V4L2_BUF_TYPE_VIDEO_OUTPUT &&
>  	    vb2_get_plane_payload(vb, 0) > vb2_plane_size(vb, 0)) {
> -		uvc_dbg(uvc_queue_to_stream(queue)->dev, CAPTURE,
> +		uvc_dbg(uvc_queue_to_stream(queue, vb->type)->dev, CAPTURE,
>  			"[E] Bytes used out of bounds\n");
>  		return -EINVAL;
>  	}
> @@ -160,7 +159,7 @@ static void uvc_buffer_finish(struct vb2_buffer *vb)
>  {
>  	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
>  	struct uvc_video_queue *queue = vb2_get_drv_priv(vb->vb2_queue);
> -	struct uvc_streaming *stream = uvc_queue_to_stream(queue);
> +	struct uvc_streaming *stream = uvc_queue_to_stream(queue, vb->type);
>  	struct uvc_buffer *buf = uvc_vbuf_to_buffer(vbuf);
>  
>  	if (vb->state == VB2_BUF_STATE_DONE)
> @@ -170,7 +169,7 @@ static void uvc_buffer_finish(struct vb2_buffer *vb)
>  static int uvc_start_streaming_video(struct vb2_queue *vq, unsigned int count)
>  {
>  	struct uvc_video_queue *queue = vb2_get_drv_priv(vq);
> -	struct uvc_streaming *stream = uvc_queue_to_stream(queue);
> +	struct uvc_streaming *stream = uvc_queue_to_stream(queue, vq->type);
>  	int ret;
>  
>  	lockdep_assert_irqs_enabled();
> @@ -197,11 +196,11 @@ static int uvc_start_streaming_video(struct vb2_queue *vq, unsigned int count)
>  static void uvc_stop_streaming_video(struct vb2_queue *vq)
>  {
>  	struct uvc_video_queue *queue = vb2_get_drv_priv(vq);
> -	struct uvc_streaming *stream = uvc_queue_to_stream(queue);
> +	struct uvc_streaming *stream = uvc_queue_to_stream(queue, vq->type);
>  
>  	lockdep_assert_irqs_enabled();
>  
> -	uvc_video_stop_streaming(uvc_queue_to_stream(queue));
> +	uvc_video_stop_streaming(stream);
>  
>  	uvc_pm_put(stream->dev);
>  
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index 8480d65ecb85..9b4849fda12f 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -703,8 +703,10 @@ static inline int uvc_queue_streaming(struct uvc_video_queue *queue)
>  }
>  
>  static inline struct uvc_streaming *
> -uvc_queue_to_stream(struct uvc_video_queue *queue)
> +uvc_queue_to_stream(struct uvc_video_queue *queue, unsigned int type)
>  {
> +	if (type == V4L2_BUF_TYPE_META_CAPTURE)
> +		return container_of(queue, struct uvc_streaming, meta.queue);
>  	return container_of(queue, struct uvc_streaming, queue);
>  }
>  
>