[PATCH v2] media: uvcvideo: Use heuristic to find stream entity

[PATCH v2] media: uvcvideo: Use heuristic to find stream entity

[Ricardo Ribalda]

Some devices, like the Grandstream GUV3100 webcam, have an invalid UVC
descriptor where multiple entities share the same ID, this is invalid
and makes it impossible to make a proper entity tree without heuristics.

We have recently introduced a change in the way that we handle invalid
entities that has caused a regression on broken devices.

Implement a new heuristic to handle these devices properly.

Reported-by: Angel4005 <ooara1337@gmail.com>
Closes: https://lore.kernel.org/linux-media/CAOzBiVuS7ygUjjhCbyWg-KiNx+HFTYnqH5+GJhd6cYsNLT=DaA@mail.gmail.com/
Fixes: 0e2ee70291e6 ("media: uvcvideo: Mark invalid entities with id UVC_INVALID_ENTITY_ID")
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
I have managed to get my hands into a Grandstream GUV3100 and
implemented a new heuristics. (Thanks Yunke and Hidenori!).

With this heuristics we can use this camera again (see the /dev/video0
in the topology).

I have tested this change in a 6.6 kernel. Because the notebook that I
used for testing has some issues running master. But for the purpose of
this change this test should work.

~ # media-ctl --print-topology
Media controller API version 6.6.99

Media device information
------------------------
driver          uvcvideo
model           GRANDSTREAM GUV3100: GRANDSTREA
serial
bus info        usb-0000:00:14.0-9
hw revision     0x409
driver version  6.6.99

Device topology
- entity 1: GRANDSTREAM GUV3100: GRANDSTREA (1 pad, 1 link)
            type Node subtype V4L flags 1
            device node name /dev/video0
        pad0: SINK
                <- "Extension 3":1 [ENABLED,IMMUTABLE]

- entity 4: GRANDSTREAM GUV3100: GRANDSTREA (0 pad, 0 link)
            type Node subtype V4L flags 0
            device node name /dev/video1

- entity 8: Extension 3 (2 pads, 2 links, 0 routes)
            type V4L2 subdev subtype Unknown flags 0
        pad0: SINK
                <- "Processing 2":1 [ENABLED,IMMUTABLE]
        pad1: SOURCE
                -> "GRANDSTREAM GUV3100: GRANDSTREA":0 [ENABLED,IMMUTABLE]

- entity 11: Processing 2 (2 pads, 3 links, 0 routes)
             type V4L2 subdev subtype Unknown flags 0
        pad0: SINK
                <- "Camera 1":0 [ENABLED,IMMUTABLE]
        pad1: SOURCE
                -> "Extension 3":0 [ENABLED,IMMUTABLE]
                -> "Extension 4":0 [ENABLED,IMMUTABLE]

- entity 14: Extension 4 (2 pads, 1 link, 0 routes)
             type V4L2 subdev subtype Unknown flags 0
        pad0: SINK
                <- "Processing 2":1 [ENABLED,IMMUTABLE]
        pad1: SOURCE

- entity 17: Camera 1 (1 pad, 1 link, 0 routes)
             type V4L2 subdev subtype Sensor flags 0
        pad0: SOURCE
                -> "Processing 2":0 [ENABLED,IMMUTABLE]
---
Changes in v2:
- Fix : invalid reference to the index variable of the iterator.
- Link to v1: https://lore.kernel.org/r/20251021-uvc-grandstream-v1-1-801e3d08b271@chromium.org
---
 drivers/media/usb/uvc/uvc_driver.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index fb6afb8e84f00961f86fd8f840fba48d706d7a9a..ee4f54d6834962414979a046afc59c5036455124 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -167,13 +167,26 @@ static struct uvc_entity *uvc_entity_by_reference(struct uvc_device *dev,
 
 static struct uvc_streaming *uvc_stream_by_id(struct uvc_device *dev, int id)
 {
-	struct uvc_streaming *stream;
+	struct uvc_streaming *stream, *last_stream;
+	unsigned int count = 0;
 
 	list_for_each_entry(stream, &dev->streams, list) {
+		count += 1;
+		last_stream = stream;
 		if (stream->header.bTerminalLink == id)
 			return stream;
 	}
 
+	/*
+	 * If the streaming entity is referenced by an invalid ID, notify the
+	 * user and use heuristics to guess the correct entity.
+	 */
+	if (count == 1 && id == UVC_INVALID_ENTITY_ID) {
+		dev_warn(&dev->intf->dev,
+			 "UVC non compliance: Invalid USB header. The streaming entity has an invalid ID, guessing the correct one.");
+		return last_stream;
+	}
+
 	return NULL;
 }
 

---
base-commit: ea299a2164262ff787c9d33f46049acccd120672
change-id: 20251021-uvc-grandstream-05ecf0288f62

Best regards,
-- 
Ricardo Ribalda <ribalda@chromium.org>

Re: [PATCH v2] media: uvcvideo: Use heuristic to find stream entity

[Hans de Goede]

Hi,

On 21-Oct-25 12:36, Ricardo Ribalda wrote:
> Some devices, like the Grandstream GUV3100 webcam, have an invalid UVC
> descriptor where multiple entities share the same ID, this is invalid
> and makes it impossible to make a proper entity tree without heuristics.
> 
> We have recently introduced a change in the way that we handle invalid
> entities that has caused a regression on broken devices.
> 
> Implement a new heuristic to handle these devices properly.
> 
> Reported-by: Angel4005 <ooara1337@gmail.com>
> Closes: https://lore.kernel.org/linux-media/CAOzBiVuS7ygUjjhCbyWg-KiNx+HFTYnqH5+GJhd6cYsNLT=DaA@mail.gmail.com/
> Fixes: 0e2ee70291e6 ("media: uvcvideo: Mark invalid entities with id UVC_INVALID_ENTITY_ID")
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hansg@kernel.org>

Regards,

Hans



> ---
> I have managed to get my hands into a Grandstream GUV3100 and
> implemented a new heuristics. (Thanks Yunke and Hidenori!).
> 
> With this heuristics we can use this camera again (see the /dev/video0
> in the topology).
> 
> I have tested this change in a 6.6 kernel. Because the notebook that I
> used for testing has some issues running master. But for the purpose of
> this change this test should work.
> 
> ~ # media-ctl --print-topology
> Media controller API version 6.6.99
> 
> Media device information
> ------------------------
> driver          uvcvideo
> model           GRANDSTREAM GUV3100: GRANDSTREA
> serial
> bus info        usb-0000:00:14.0-9
> hw revision     0x409
> driver version  6.6.99
> 
> Device topology
> - entity 1: GRANDSTREAM GUV3100: GRANDSTREA (1 pad, 1 link)
>             type Node subtype V4L flags 1
>             device node name /dev/video0
>         pad0: SINK
>                 <- "Extension 3":1 [ENABLED,IMMUTABLE]
> 
> - entity 4: GRANDSTREAM GUV3100: GRANDSTREA (0 pad, 0 link)
>             type Node subtype V4L flags 0
>             device node name /dev/video1
> 
> - entity 8: Extension 3 (2 pads, 2 links, 0 routes)
>             type V4L2 subdev subtype Unknown flags 0
>         pad0: SINK
>                 <- "Processing 2":1 [ENABLED,IMMUTABLE]
>         pad1: SOURCE
>                 -> "GRANDSTREAM GUV3100: GRANDSTREA":0 [ENABLED,IMMUTABLE]
> 
> - entity 11: Processing 2 (2 pads, 3 links, 0 routes)
>              type V4L2 subdev subtype Unknown flags 0
>         pad0: SINK
>                 <- "Camera 1":0 [ENABLED,IMMUTABLE]
>         pad1: SOURCE
>                 -> "Extension 3":0 [ENABLED,IMMUTABLE]
>                 -> "Extension 4":0 [ENABLED,IMMUTABLE]
> 
> - entity 14: Extension 4 (2 pads, 1 link, 0 routes)
>              type V4L2 subdev subtype Unknown flags 0
>         pad0: SINK
>                 <- "Processing 2":1 [ENABLED,IMMUTABLE]
>         pad1: SOURCE
> 
> - entity 17: Camera 1 (1 pad, 1 link, 0 routes)
>              type V4L2 subdev subtype Sensor flags 0
>         pad0: SOURCE
>                 -> "Processing 2":0 [ENABLED,IMMUTABLE]
> ---
> Changes in v2:
> - Fix : invalid reference to the index variable of the iterator.
> - Link to v1: https://lore.kernel.org/r/20251021-uvc-grandstream-v1-1-801e3d08b271@chromium.org
> ---
>  drivers/media/usb/uvc/uvc_driver.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index fb6afb8e84f00961f86fd8f840fba48d706d7a9a..ee4f54d6834962414979a046afc59c5036455124 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -167,13 +167,26 @@ static struct uvc_entity *uvc_entity_by_reference(struct uvc_device *dev,
>  
>  static struct uvc_streaming *uvc_stream_by_id(struct uvc_device *dev, int id)
>  {
> -	struct uvc_streaming *stream;
> +	struct uvc_streaming *stream, *last_stream;
> +	unsigned int count = 0;
>  
>  	list_for_each_entry(stream, &dev->streams, list) {
> +		count += 1;
> +		last_stream = stream;
>  		if (stream->header.bTerminalLink == id)
>  			return stream;
>  	}
>  
> +	/*
> +	 * If the streaming entity is referenced by an invalid ID, notify the
> +	 * user and use heuristics to guess the correct entity.
> +	 */
> +	if (count == 1 && id == UVC_INVALID_ENTITY_ID) {
> +		dev_warn(&dev->intf->dev,
> +			 "UVC non compliance: Invalid USB header. The streaming entity has an invalid ID, guessing the correct one.");
> +		return last_stream;
> +	}
> +
>  	return NULL;
>  }
>  
> 
> ---
> base-commit: ea299a2164262ff787c9d33f46049acccd120672
> change-id: 20251021-uvc-grandstream-05ecf0288f62
> 
> Best regards,

Re: [PATCH v2] media: uvcvideo: Use heuristic to find stream entity

[Ricardo Ribalda]

Hi Hans(es)

On Tue, 21 Oct 2025 at 18:07, Hans de Goede <hansg@kernel.org> wrote:
>
> Hi,
>
> On 21-Oct-25 12:36, Ricardo Ribalda wrote:
> > Some devices, like the Grandstream GUV3100 webcam, have an invalid UVC
> > descriptor where multiple entities share the same ID, this is invalid
> > and makes it impossible to make a proper entity tree without heuristics.
> >
> > We have recently introduced a change in the way that we handle invalid
> > entities that has caused a regression on broken devices.
> >
> > Implement a new heuristic to handle these devices properly.
> >
> > Reported-by: Angel4005 <ooara1337@gmail.com>
> > Closes: https://lore.kernel.org/linux-media/CAOzBiVuS7ygUjjhCbyWg-KiNx+HFTYnqH5+GJhd6cYsNLT=DaA@mail.gmail.com/
> > Fixes: 0e2ee70291e6 ("media: uvcvideo: Mark invalid entities with id UVC_INVALID_ENTITY_ID")
> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
>
> Thanks, patch looks good to me:
>
> Reviewed-by: Hans de Goede <hansg@kernel.org>

Thanks for the prompt reply.

@Hverkil I think you are planning to push to /fixes soon. I believe
this patch should also land there.

>
> Regards,
>
> Hans
>
>
>
> > ---
> > I have managed to get my hands into a Grandstream GUV3100 and
> > implemented a new heuristics. (Thanks Yunke and Hidenori!).
> >
> > With this heuristics we can use this camera again (see the /dev/video0
> > in the topology).
> >
> > I have tested this change in a 6.6 kernel. Because the notebook that I
> > used for testing has some issues running master. But for the purpose of
> > this change this test should work.
> >
> > ~ # media-ctl --print-topology
> > Media controller API version 6.6.99
> >
> > Media device information
> > ------------------------
> > driver          uvcvideo
> > model           GRANDSTREAM GUV3100: GRANDSTREA
> > serial
> > bus info        usb-0000:00:14.0-9
> > hw revision     0x409
> > driver version  6.6.99
> >
> > Device topology
> > - entity 1: GRANDSTREAM GUV3100: GRANDSTREA (1 pad, 1 link)
> >             type Node subtype V4L flags 1
> >             device node name /dev/video0
> >         pad0: SINK
> >                 <- "Extension 3":1 [ENABLED,IMMUTABLE]
> >
> > - entity 4: GRANDSTREAM GUV3100: GRANDSTREA (0 pad, 0 link)
> >             type Node subtype V4L flags 0
> >             device node name /dev/video1
> >
> > - entity 8: Extension 3 (2 pads, 2 links, 0 routes)
> >             type V4L2 subdev subtype Unknown flags 0
> >         pad0: SINK
> >                 <- "Processing 2":1 [ENABLED,IMMUTABLE]
> >         pad1: SOURCE
> >                 -> "GRANDSTREAM GUV3100: GRANDSTREA":0 [ENABLED,IMMUTABLE]
> >
> > - entity 11: Processing 2 (2 pads, 3 links, 0 routes)
> >              type V4L2 subdev subtype Unknown flags 0
> >         pad0: SINK
> >                 <- "Camera 1":0 [ENABLED,IMMUTABLE]
> >         pad1: SOURCE
> >                 -> "Extension 3":0 [ENABLED,IMMUTABLE]
> >                 -> "Extension 4":0 [ENABLED,IMMUTABLE]
> >
> > - entity 14: Extension 4 (2 pads, 1 link, 0 routes)
> >              type V4L2 subdev subtype Unknown flags 0
> >         pad0: SINK
> >                 <- "Processing 2":1 [ENABLED,IMMUTABLE]
> >         pad1: SOURCE
> >
> > - entity 17: Camera 1 (1 pad, 1 link, 0 routes)
> >              type V4L2 subdev subtype Sensor flags 0
> >         pad0: SOURCE
> >                 -> "Processing 2":0 [ENABLED,IMMUTABLE]
> > ---
> > Changes in v2:
> > - Fix : invalid reference to the index variable of the iterator.
> > - Link to v1: https://lore.kernel.org/r/20251021-uvc-grandstream-v1-1-801e3d08b271@chromium.org
> > ---
> >  drivers/media/usb/uvc/uvc_driver.c | 15 ++++++++++++++-
> >  1 file changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> > index fb6afb8e84f00961f86fd8f840fba48d706d7a9a..ee4f54d6834962414979a046afc59c5036455124 100644
> > --- a/drivers/media/usb/uvc/uvc_driver.c
> > +++ b/drivers/media/usb/uvc/uvc_driver.c
> > @@ -167,13 +167,26 @@ static struct uvc_entity *uvc_entity_by_reference(struct uvc_device *dev,
> >
> >  static struct uvc_streaming *uvc_stream_by_id(struct uvc_device *dev, int id)
> >  {
> > -     struct uvc_streaming *stream;
> > +     struct uvc_streaming *stream, *last_stream;
> > +     unsigned int count = 0;
> >
> >       list_for_each_entry(stream, &dev->streams, list) {
> > +             count += 1;
> > +             last_stream = stream;
> >               if (stream->header.bTerminalLink == id)
> >                       return stream;
> >       }
> >
> > +     /*
> > +      * If the streaming entity is referenced by an invalid ID, notify the
> > +      * user and use heuristics to guess the correct entity.
> > +      */
> > +     if (count == 1 && id == UVC_INVALID_ENTITY_ID) {
> > +             dev_warn(&dev->intf->dev,
> > +                      "UVC non compliance: Invalid USB header. The streaming entity has an invalid ID, guessing the correct one.");
> > +             return last_stream;
> > +     }
> > +
> >       return NULL;
> >  }
> >
> >
> > ---
> > base-commit: ea299a2164262ff787c9d33f46049acccd120672
> > change-id: 20251021-uvc-grandstream-05ecf0288f62
> >
> > Best regards,
>


-- 
Ricardo Ribalda

Re: [PATCH v2] media: uvcvideo: Use heuristic to find stream entity

[Laurent Pinchart]

On Tue, Oct 21, 2025 at 06:15:54PM +0200, Ricardo Ribalda wrote:
> Hi Hans(es)
> 
> On Tue, 21 Oct 2025 at 18:07, Hans de Goede <hansg@kernel.org> wrote:
> >
> > Hi,
> >
> > On 21-Oct-25 12:36, Ricardo Ribalda wrote:
> > > Some devices, like the Grandstream GUV3100 webcam, have an invalid UVC
> > > descriptor where multiple entities share the same ID, this is invalid
> > > and makes it impossible to make a proper entity tree without heuristics.
> > >
> > > We have recently introduced a change in the way that we handle invalid
> > > entities that has caused a regression on broken devices.
> > >
> > > Implement a new heuristic to handle these devices properly.
> > >
> > > Reported-by: Angel4005 <ooara1337@gmail.com>
> > > Closes: https://lore.kernel.org/linux-media/CAOzBiVuS7ygUjjhCbyWg-KiNx+HFTYnqH5+GJhd6cYsNLT=DaA@mail.gmail.com/
> > > Fixes: 0e2ee70291e6 ("media: uvcvideo: Mark invalid entities with id UVC_INVALID_ENTITY_ID")
> > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> >
> > Thanks, patch looks good to me:
> >
> > Reviewed-by: Hans de Goede <hansg@kernel.org>
> 
> Thanks for the prompt reply.
> 
> @Hverkil I think you are planning to push to /fixes soon. I believe
> this patch should also land there.

Review is still ongoing :-) I've just sent comments, let's finish the
discussion first.

> > > ---
> > > I have managed to get my hands into a Grandstream GUV3100 and
> > > implemented a new heuristics. (Thanks Yunke and Hidenori!).
> > >
> > > With this heuristics we can use this camera again (see the /dev/video0
> > > in the topology).
> > >
> > > I have tested this change in a 6.6 kernel. Because the notebook that I
> > > used for testing has some issues running master. But for the purpose of
> > > this change this test should work.
> > >
> > > ~ # media-ctl --print-topology
> > > Media controller API version 6.6.99
> > >
> > > Media device information
> > > ------------------------
> > > driver          uvcvideo
> > > model           GRANDSTREAM GUV3100: GRANDSTREA
> > > serial
> > > bus info        usb-0000:00:14.0-9
> > > hw revision     0x409
> > > driver version  6.6.99
> > >
> > > Device topology
> > > - entity 1: GRANDSTREAM GUV3100: GRANDSTREA (1 pad, 1 link)
> > >             type Node subtype V4L flags 1
> > >             device node name /dev/video0
> > >         pad0: SINK
> > >                 <- "Extension 3":1 [ENABLED,IMMUTABLE]
> > >
> > > - entity 4: GRANDSTREAM GUV3100: GRANDSTREA (0 pad, 0 link)
> > >             type Node subtype V4L flags 0
> > >             device node name /dev/video1
> > >
> > > - entity 8: Extension 3 (2 pads, 2 links, 0 routes)
> > >             type V4L2 subdev subtype Unknown flags 0
> > >         pad0: SINK
> > >                 <- "Processing 2":1 [ENABLED,IMMUTABLE]
> > >         pad1: SOURCE
> > >                 -> "GRANDSTREAM GUV3100: GRANDSTREA":0 [ENABLED,IMMUTABLE]
> > >
> > > - entity 11: Processing 2 (2 pads, 3 links, 0 routes)
> > >              type V4L2 subdev subtype Unknown flags 0
> > >         pad0: SINK
> > >                 <- "Camera 1":0 [ENABLED,IMMUTABLE]
> > >         pad1: SOURCE
> > >                 -> "Extension 3":0 [ENABLED,IMMUTABLE]
> > >                 -> "Extension 4":0 [ENABLED,IMMUTABLE]
> > >
> > > - entity 14: Extension 4 (2 pads, 1 link, 0 routes)
> > >              type V4L2 subdev subtype Unknown flags 0
> > >         pad0: SINK
> > >                 <- "Processing 2":1 [ENABLED,IMMUTABLE]
> > >         pad1: SOURCE
> > >
> > > - entity 17: Camera 1 (1 pad, 1 link, 0 routes)
> > >              type V4L2 subdev subtype Sensor flags 0
> > >         pad0: SOURCE
> > >                 -> "Processing 2":0 [ENABLED,IMMUTABLE]
> > > ---
> > > Changes in v2:
> > > - Fix : invalid reference to the index variable of the iterator.
> > > - Link to v1: https://lore.kernel.org/r/20251021-uvc-grandstream-v1-1-801e3d08b271@chromium.org
> > > ---
> > >  drivers/media/usb/uvc/uvc_driver.c | 15 ++++++++++++++-
> > >  1 file changed, 14 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> > > index fb6afb8e84f00961f86fd8f840fba48d706d7a9a..ee4f54d6834962414979a046afc59c5036455124 100644
> > > --- a/drivers/media/usb/uvc/uvc_driver.c
> > > +++ b/drivers/media/usb/uvc/uvc_driver.c
> > > @@ -167,13 +167,26 @@ static struct uvc_entity *uvc_entity_by_reference(struct uvc_device *dev,
> > >
> > >  static struct uvc_streaming *uvc_stream_by_id(struct uvc_device *dev, int id)
> > >  {
> > > -     struct uvc_streaming *stream;
> > > +     struct uvc_streaming *stream, *last_stream;
> > > +     unsigned int count = 0;
> > >
> > >       list_for_each_entry(stream, &dev->streams, list) {
> > > +             count += 1;
> > > +             last_stream = stream;
> > >               if (stream->header.bTerminalLink == id)
> > >                       return stream;
> > >       }
> > >
> > > +     /*
> > > +      * If the streaming entity is referenced by an invalid ID, notify the
> > > +      * user and use heuristics to guess the correct entity.
> > > +      */
> > > +     if (count == 1 && id == UVC_INVALID_ENTITY_ID) {
> > > +             dev_warn(&dev->intf->dev,
> > > +                      "UVC non compliance: Invalid USB header. The streaming entity has an invalid ID, guessing the correct one.");
> > > +             return last_stream;
> > > +     }
> > > +
> > >       return NULL;
> > >  }
> > >
> > >
> > > ---
> > > base-commit: ea299a2164262ff787c9d33f46049acccd120672
> > > change-id: 20251021-uvc-grandstream-05ecf0288f62

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2] media: uvcvideo: Use heuristic to find stream entity

[Laurent Pinchart]

Hi Ricardo,

Thank you for the patch.

On Tue, Oct 21, 2025 at 10:36:17AM +0000, Ricardo Ribalda wrote:
> Some devices, like the Grandstream GUV3100 webcam, have an invalid UVC
> descriptor where multiple entities share the same ID, this is invalid
> and makes it impossible to make a proper entity tree without heuristics.
> 
> We have recently introduced a change in the way that we handle invalid
> entities that has caused a regression on broken devices.
> 
> Implement a new heuristic to handle these devices properly.
> 
> Reported-by: Angel4005 <ooara1337@gmail.com>
> Closes: https://lore.kernel.org/linux-media/CAOzBiVuS7ygUjjhCbyWg-KiNx+HFTYnqH5+GJhd6cYsNLT=DaA@mail.gmail.com/
> Fixes: 0e2ee70291e6 ("media: uvcvideo: Mark invalid entities with id UVC_INVALID_ENTITY_ID")
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
> I have managed to get my hands into a Grandstream GUV3100 and
> implemented a new heuristics. (Thanks Yunke and Hidenori!).
> 
> With this heuristics we can use this camera again (see the /dev/video0
> in the topology).
> 
> I have tested this change in a 6.6 kernel. Because the notebook that I
> used for testing has some issues running master. But for the purpose of
> this change this test should work.
> 
> ~ # media-ctl --print-topology
> Media controller API version 6.6.99
> 
> Media device information
> ------------------------
> driver          uvcvideo
> model           GRANDSTREAM GUV3100: GRANDSTREA
> serial
> bus info        usb-0000:00:14.0-9
> hw revision     0x409
> driver version  6.6.99
> 
> Device topology
> - entity 1: GRANDSTREAM GUV3100: GRANDSTREA (1 pad, 1 link)
>             type Node subtype V4L flags 1
>             device node name /dev/video0
>         pad0: SINK
>                 <- "Extension 3":1 [ENABLED,IMMUTABLE]
> 
> - entity 4: GRANDSTREAM GUV3100: GRANDSTREA (0 pad, 0 link)
>             type Node subtype V4L flags 0
>             device node name /dev/video1
> 
> - entity 8: Extension 3 (2 pads, 2 links, 0 routes)
>             type V4L2 subdev subtype Unknown flags 0
>         pad0: SINK
>                 <- "Processing 2":1 [ENABLED,IMMUTABLE]
>         pad1: SOURCE
>                 -> "GRANDSTREAM GUV3100: GRANDSTREA":0 [ENABLED,IMMUTABLE]
> 
> - entity 11: Processing 2 (2 pads, 3 links, 0 routes)
>              type V4L2 subdev subtype Unknown flags 0
>         pad0: SINK
>                 <- "Camera 1":0 [ENABLED,IMMUTABLE]
>         pad1: SOURCE
>                 -> "Extension 3":0 [ENABLED,IMMUTABLE]
>                 -> "Extension 4":0 [ENABLED,IMMUTABLE]
> 
> - entity 14: Extension 4 (2 pads, 1 link, 0 routes)
>              type V4L2 subdev subtype Unknown flags 0
>         pad0: SINK
>                 <- "Processing 2":1 [ENABLED,IMMUTABLE]
>         pad1: SOURCE
> 
> - entity 17: Camera 1 (1 pad, 1 link, 0 routes)
>              type V4L2 subdev subtype Sensor flags 0
>         pad0: SOURCE
>                 -> "Processing 2":0 [ENABLED,IMMUTABLE]
> ---
> Changes in v2:
> - Fix : invalid reference to the index variable of the iterator.
> - Link to v1: https://lore.kernel.org/r/20251021-uvc-grandstream-v1-1-801e3d08b271@chromium.org
> ---
>  drivers/media/usb/uvc/uvc_driver.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index fb6afb8e84f00961f86fd8f840fba48d706d7a9a..ee4f54d6834962414979a046afc59c5036455124 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -167,13 +167,26 @@ static struct uvc_entity *uvc_entity_by_reference(struct uvc_device *dev,
>  
>  static struct uvc_streaming *uvc_stream_by_id(struct uvc_device *dev, int id)
>  {
> -	struct uvc_streaming *stream;
> +	struct uvc_streaming *stream, *last_stream;
> +	unsigned int count = 0;
>  
>  	list_for_each_entry(stream, &dev->streams, list) {
> +		count += 1;
> +		last_stream = stream;
>  		if (stream->header.bTerminalLink == id)
>  			return stream;
>  	}
>  
> +	/*
> +	 * If the streaming entity is referenced by an invalid ID, notify the
> +	 * user and use heuristics to guess the correct entity.
> +	 */
> +	if (count == 1 && id == UVC_INVALID_ENTITY_ID) {
> +		dev_warn(&dev->intf->dev,
> +			 "UVC non compliance: Invalid USB header. The streaming entity has an invalid ID, guessing the correct one.");
> +		return last_stream;
> +	}

As far as I understand, the reason why we can't find the streaming
interface here is because we have set the streaming output terminal ID
to UVC_INVALID_ENTITY_ID, due to the extension unit being previously
registered with the same ID. We're therefore adding a workaround on top
of another workaround.

Looking at the UVC descriptors for this camera, ID 4 is shared between
an extension unit with GUID ffe52d21-8030-4e2c-82d9-f587d00540bd and the
streaming output terminal. That GUID is apparently a Logitech GUID
(according to https://github.com/soyersoyer/cameractrls/blob/c920e/cameractrls.py).
I don't know if the XU actually works in that camera, but it could be
useful to keep it functional.

I think we could make the handling of non-unique IDs more clever. If an
output streaming terminal and a unit share the same ID, we could
allocate an unused ID for the output streaming terminal, and patch the
bTerminalLink of the stream headers accordingly. The ID remapping should
not cause other issues, as

- streaming output terminals have no controls, so the ID isn't used to
  reference controls

- the units graph is build from sink to source, with UVC descriptors
  containing the IDs of source units, so streaming output terminals are
  never referenced by ID from descriptors except for the bTerminalLink.

This would produce a valid graph without duplicated IDs in this case.

> +
>  	return NULL;
>  }
>  
> 
> ---
> base-commit: ea299a2164262ff787c9d33f46049acccd120672
> change-id: 20251021-uvc-grandstream-05ecf0288f62

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2] media: uvcvideo: Use heuristic to find stream entity

[Ricardo Ribalda]

Hi Laurent

On Tue, 21 Oct 2025 at 18:44, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Ricardo,
>
> Thank you for the patch.
>
> On Tue, Oct 21, 2025 at 10:36:17AM +0000, Ricardo Ribalda wrote:
> > Some devices, like the Grandstream GUV3100 webcam, have an invalid UVC
> > descriptor where multiple entities share the same ID, this is invalid
> > and makes it impossible to make a proper entity tree without heuristics.
> >
> > We have recently introduced a change in the way that we handle invalid
> > entities that has caused a regression on broken devices.
> >
> > Implement a new heuristic to handle these devices properly.
> >
> > Reported-by: Angel4005 <ooara1337@gmail.com>
> > Closes: https://lore.kernel.org/linux-media/CAOzBiVuS7ygUjjhCbyWg-KiNx+HFTYnqH5+GJhd6cYsNLT=DaA@mail.gmail.com/
> > Fixes: 0e2ee70291e6 ("media: uvcvideo: Mark invalid entities with id UVC_INVALID_ENTITY_ID")
> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > ---
> > I have managed to get my hands into a Grandstream GUV3100 and
> > implemented a new heuristics. (Thanks Yunke and Hidenori!).
> >
> > With this heuristics we can use this camera again (see the /dev/video0
> > in the topology).
> >
> > I have tested this change in a 6.6 kernel. Because the notebook that I
> > used for testing has some issues running master. But for the purpose of
> > this change this test should work.
> >
> > ~ # media-ctl --print-topology
> > Media controller API version 6.6.99
> >
> > Media device information
> > ------------------------
> > driver          uvcvideo
> > model           GRANDSTREAM GUV3100: GRANDSTREA
> > serial
> > bus info        usb-0000:00:14.0-9
> > hw revision     0x409
> > driver version  6.6.99
> >
> > Device topology
> > - entity 1: GRANDSTREAM GUV3100: GRANDSTREA (1 pad, 1 link)
> >             type Node subtype V4L flags 1
> >             device node name /dev/video0
> >         pad0: SINK
> >                 <- "Extension 3":1 [ENABLED,IMMUTABLE]
> >
> > - entity 4: GRANDSTREAM GUV3100: GRANDSTREA (0 pad, 0 link)
> >             type Node subtype V4L flags 0
> >             device node name /dev/video1
> >
> > - entity 8: Extension 3 (2 pads, 2 links, 0 routes)
> >             type V4L2 subdev subtype Unknown flags 0
> >         pad0: SINK
> >                 <- "Processing 2":1 [ENABLED,IMMUTABLE]
> >         pad1: SOURCE
> >                 -> "GRANDSTREAM GUV3100: GRANDSTREA":0 [ENABLED,IMMUTABLE]
> >
> > - entity 11: Processing 2 (2 pads, 3 links, 0 routes)
> >              type V4L2 subdev subtype Unknown flags 0
> >         pad0: SINK
> >                 <- "Camera 1":0 [ENABLED,IMMUTABLE]
> >         pad1: SOURCE
> >                 -> "Extension 3":0 [ENABLED,IMMUTABLE]
> >                 -> "Extension 4":0 [ENABLED,IMMUTABLE]
> >
> > - entity 14: Extension 4 (2 pads, 1 link, 0 routes)
> >              type V4L2 subdev subtype Unknown flags 0
> >         pad0: SINK
> >                 <- "Processing 2":1 [ENABLED,IMMUTABLE]
> >         pad1: SOURCE
> >
> > - entity 17: Camera 1 (1 pad, 1 link, 0 routes)
> >              type V4L2 subdev subtype Sensor flags 0
> >         pad0: SOURCE
> >                 -> "Processing 2":0 [ENABLED,IMMUTABLE]
> > ---
> > Changes in v2:
> > - Fix : invalid reference to the index variable of the iterator.
> > - Link to v1: https://lore.kernel.org/r/20251021-uvc-grandstream-v1-1-801e3d08b271@chromium.org
> > ---
> >  drivers/media/usb/uvc/uvc_driver.c | 15 ++++++++++++++-
> >  1 file changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> > index fb6afb8e84f00961f86fd8f840fba48d706d7a9a..ee4f54d6834962414979a046afc59c5036455124 100644
> > --- a/drivers/media/usb/uvc/uvc_driver.c
> > +++ b/drivers/media/usb/uvc/uvc_driver.c
> > @@ -167,13 +167,26 @@ static struct uvc_entity *uvc_entity_by_reference(struct uvc_device *dev,
> >
> >  static struct uvc_streaming *uvc_stream_by_id(struct uvc_device *dev, int id)
> >  {
> > -     struct uvc_streaming *stream;
> > +     struct uvc_streaming *stream, *last_stream;
> > +     unsigned int count = 0;
> >
> >       list_for_each_entry(stream, &dev->streams, list) {
> > +             count += 1;
> > +             last_stream = stream;
> >               if (stream->header.bTerminalLink == id)
> >                       return stream;
> >       }
> >
> > +     /*
> > +      * If the streaming entity is referenced by an invalid ID, notify the
> > +      * user and use heuristics to guess the correct entity.
> > +      */
> > +     if (count == 1 && id == UVC_INVALID_ENTITY_ID) {
> > +             dev_warn(&dev->intf->dev,
> > +                      "UVC non compliance: Invalid USB header. The streaming entity has an invalid ID, guessing the correct one.");
> > +             return last_stream;
> > +     }
>
> As far as I understand, the reason why we can't find the streaming
> interface here is because we have set the streaming output terminal ID
> to UVC_INVALID_ENTITY_ID, due to the extension unit being previously
> registered with the same ID. We're therefore adding a workaround on top
> of another workaround.
>
> Looking at the UVC descriptors for this camera, ID 4 is shared between
> an extension unit with GUID ffe52d21-8030-4e2c-82d9-f587d00540bd and the
> streaming output terminal. That GUID is apparently a Logitech GUID
> (according to https://github.com/soyersoyer/cameractrls/blob/c920e/cameractrls.py).
> I don't know if the XU actually works in that camera, but it could be
> useful to keep it functional.
>
> I think we could make the handling of non-unique IDs more clever. If an
> output streaming terminal and a unit share the same ID, we could
> allocate an unused ID for the output streaming terminal, and patch the
> bTerminalLink of the stream headers accordingly. The ID remapping should
> not cause other issues, as
>
> - streaming output terminals have no controls, so the ID isn't used to
>   reference controls
>
> - the units graph is build from sink to source, with UVC descriptors
>   containing the IDs of source units, so streaming output terminals are
>   never referenced by ID from descriptors except for the bTerminalLink.
>
> This would produce a valid graph without duplicated IDs in this case.

I will try to work on a solution like you propose in this kernel
cycle, but I do not feel very confident about landing that big change
in "fixes" without being in linux-next for a couple of weeks. I'd
rather go for a simple solution that fixes the regression.

With this fix, the media controller topology looks identical as before
we introduce the bug, so we do not lose any functionality.

So if you agree I think the safest path is to land this fix in fixes
now. And try to get a better logic landed in next in this kernel
cycle.

What do you think?

Regards!


>
> > +
> >       return NULL;
> >  }
> >
> >
> > ---
> > base-commit: ea299a2164262ff787c9d33f46049acccd120672
> > change-id: 20251021-uvc-grandstream-05ecf0288f62
>
> --
> Regards,
>
> Laurent Pinchart



-- 
Ricardo Ribalda

Re: [PATCH v2] media: uvcvideo: Use heuristic to find stream entity

[Laurent Pinchart]

On Tue, Oct 21, 2025 at 06:57:00PM +0200, Ricardo Ribalda wrote:
> On Tue, 21 Oct 2025 at 18:44, Laurent Pinchart wrote:
> > On Tue, Oct 21, 2025 at 10:36:17AM +0000, Ricardo Ribalda wrote:
> > > Some devices, like the Grandstream GUV3100 webcam, have an invalid UVC
> > > descriptor where multiple entities share the same ID, this is invalid
> > > and makes it impossible to make a proper entity tree without heuristics.
> > >
> > > We have recently introduced a change in the way that we handle invalid
> > > entities that has caused a regression on broken devices.
> > >
> > > Implement a new heuristic to handle these devices properly.
> > >
> > > Reported-by: Angel4005 <ooara1337@gmail.com>
> > > Closes: https://lore.kernel.org/linux-media/CAOzBiVuS7ygUjjhCbyWg-KiNx+HFTYnqH5+GJhd6cYsNLT=DaA@mail.gmail.com/
> > > Fixes: 0e2ee70291e6 ("media: uvcvideo: Mark invalid entities with id UVC_INVALID_ENTITY_ID")
> > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > > ---
> > > I have managed to get my hands into a Grandstream GUV3100 and
> > > implemented a new heuristics. (Thanks Yunke and Hidenori!).
> > >
> > > With this heuristics we can use this camera again (see the /dev/video0
> > > in the topology).
> > >
> > > I have tested this change in a 6.6 kernel. Because the notebook that I
> > > used for testing has some issues running master. But for the purpose of
> > > this change this test should work.
> > >
> > > ~ # media-ctl --print-topology
> > > Media controller API version 6.6.99
> > >
> > > Media device information
> > > ------------------------
> > > driver          uvcvideo
> > > model           GRANDSTREAM GUV3100: GRANDSTREA
> > > serial
> > > bus info        usb-0000:00:14.0-9
> > > hw revision     0x409
> > > driver version  6.6.99
> > >
> > > Device topology
> > > - entity 1: GRANDSTREAM GUV3100: GRANDSTREA (1 pad, 1 link)
> > >             type Node subtype V4L flags 1
> > >             device node name /dev/video0
> > >         pad0: SINK
> > >                 <- "Extension 3":1 [ENABLED,IMMUTABLE]
> > >
> > > - entity 4: GRANDSTREAM GUV3100: GRANDSTREA (0 pad, 0 link)
> > >             type Node subtype V4L flags 0
> > >             device node name /dev/video1
> > >
> > > - entity 8: Extension 3 (2 pads, 2 links, 0 routes)
> > >             type V4L2 subdev subtype Unknown flags 0
> > >         pad0: SINK
> > >                 <- "Processing 2":1 [ENABLED,IMMUTABLE]
> > >         pad1: SOURCE
> > >                 -> "GRANDSTREAM GUV3100: GRANDSTREA":0 [ENABLED,IMMUTABLE]
> > >
> > > - entity 11: Processing 2 (2 pads, 3 links, 0 routes)
> > >              type V4L2 subdev subtype Unknown flags 0
> > >         pad0: SINK
> > >                 <- "Camera 1":0 [ENABLED,IMMUTABLE]
> > >         pad1: SOURCE
> > >                 -> "Extension 3":0 [ENABLED,IMMUTABLE]
> > >                 -> "Extension 4":0 [ENABLED,IMMUTABLE]
> > >
> > > - entity 14: Extension 4 (2 pads, 1 link, 0 routes)
> > >              type V4L2 subdev subtype Unknown flags 0
> > >         pad0: SINK
> > >                 <- "Processing 2":1 [ENABLED,IMMUTABLE]
> > >         pad1: SOURCE
> > >
> > > - entity 17: Camera 1 (1 pad, 1 link, 0 routes)
> > >              type V4L2 subdev subtype Sensor flags 0
> > >         pad0: SOURCE
> > >                 -> "Processing 2":0 [ENABLED,IMMUTABLE]
> > > ---
> > > Changes in v2:
> > > - Fix : invalid reference to the index variable of the iterator.
> > > - Link to v1: https://lore.kernel.org/r/20251021-uvc-grandstream-v1-1-801e3d08b271@chromium.org
> > > ---
> > >  drivers/media/usb/uvc/uvc_driver.c | 15 ++++++++++++++-
> > >  1 file changed, 14 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> > > index fb6afb8e84f00961f86fd8f840fba48d706d7a9a..ee4f54d6834962414979a046afc59c5036455124 100644
> > > --- a/drivers/media/usb/uvc/uvc_driver.c
> > > +++ b/drivers/media/usb/uvc/uvc_driver.c
> > > @@ -167,13 +167,26 @@ static struct uvc_entity *uvc_entity_by_reference(struct uvc_device *dev,
> > >
> > >  static struct uvc_streaming *uvc_stream_by_id(struct uvc_device *dev, int id)
> > >  {
> > > -     struct uvc_streaming *stream;
> > > +     struct uvc_streaming *stream, *last_stream;
> > > +     unsigned int count = 0;
> > >
> > >       list_for_each_entry(stream, &dev->streams, list) {
> > > +             count += 1;
> > > +             last_stream = stream;
> > >               if (stream->header.bTerminalLink == id)
> > >                       return stream;
> > >       }
> > >
> > > +     /*
> > > +      * If the streaming entity is referenced by an invalid ID, notify the
> > > +      * user and use heuristics to guess the correct entity.
> > > +      */
> > > +     if (count == 1 && id == UVC_INVALID_ENTITY_ID) {
> > > +             dev_warn(&dev->intf->dev,
> > > +                      "UVC non compliance: Invalid USB header. The streaming entity has an invalid ID, guessing the correct one.");
> > > +             return last_stream;
> > > +     }
> >
> > As far as I understand, the reason why we can't find the streaming
> > interface here is because we have set the streaming output terminal ID
> > to UVC_INVALID_ENTITY_ID, due to the extension unit being previously
> > registered with the same ID. We're therefore adding a workaround on top
> > of another workaround.
> >
> > Looking at the UVC descriptors for this camera, ID 4 is shared between
> > an extension unit with GUID ffe52d21-8030-4e2c-82d9-f587d00540bd and the
> > streaming output terminal. That GUID is apparently a Logitech GUID
> > (according to https://github.com/soyersoyer/cameractrls/blob/c920e/cameractrls.py).
> > I don't know if the XU actually works in that camera, but it could be
> > useful to keep it functional.
> >
> > I think we could make the handling of non-unique IDs more clever. If an
> > output streaming terminal and a unit share the same ID, we could
> > allocate an unused ID for the output streaming terminal, and patch the
> > bTerminalLink of the stream headers accordingly. The ID remapping should
> > not cause other issues, as
> >
> > - streaming output terminals have no controls, so the ID isn't used to
> >   reference controls
> >
> > - the units graph is build from sink to source, with UVC descriptors
> >   containing the IDs of source units, so streaming output terminals are
> >   never referenced by ID from descriptors except for the bTerminalLink.
> >
> > This would produce a valid graph without duplicated IDs in this case.
> 
> I will try to work on a solution like you propose in this kernel
> cycle, but I do not feel very confident about landing that big change
> in "fixes" without being in linux-next for a couple of weeks. I'd
> rather go for a simple solution that fixes the regression.
> 
> With this fix, the media controller topology looks identical as before
> we introduce the bug, so we do not lose any functionality.
> 
> So if you agree I think the safest path is to land this fix in fixes
> now. And try to get a better logic landed in next in this kernel
> cycle.
> 
> What do you think?

I'm fine with that timelie. Thanks for agreeing to try and implement the
above proposal.

> > > +
> > >       return NULL;
> > >  }
> > >
> > >
> > > ---
> > > base-commit: ea299a2164262ff787c9d33f46049acccd120672
> > > change-id: 20251021-uvc-grandstream-05ecf0288f62

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2] media: uvcvideo: Use heuristic to find stream entity

[Laurent Pinchart]

On Tue, Oct 21, 2025 at 08:06:38PM +0300, Laurent Pinchart wrote:
> On Tue, Oct 21, 2025 at 06:57:00PM +0200, Ricardo Ribalda wrote:
> > On Tue, 21 Oct 2025 at 18:44, Laurent Pinchart wrote:
> > > On Tue, Oct 21, 2025 at 10:36:17AM +0000, Ricardo Ribalda wrote:
> > > > Some devices, like the Grandstream GUV3100 webcam, have an invalid UVC
> > > > descriptor where multiple entities share the same ID, this is invalid
> > > > and makes it impossible to make a proper entity tree without heuristics.
> > > >
> > > > We have recently introduced a change in the way that we handle invalid
> > > > entities that has caused a regression on broken devices.
> > > >
> > > > Implement a new heuristic to handle these devices properly.
> > > >
> > > > Reported-by: Angel4005 <ooara1337@gmail.com>
> > > > Closes: https://lore.kernel.org/linux-media/CAOzBiVuS7ygUjjhCbyWg-KiNx+HFTYnqH5+GJhd6cYsNLT=DaA@mail.gmail.com/
> > > > Fixes: 0e2ee70291e6 ("media: uvcvideo: Mark invalid entities with id UVC_INVALID_ENTITY_ID")
> > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > > > ---
> > > > I have managed to get my hands into a Grandstream GUV3100 and
> > > > implemented a new heuristics. (Thanks Yunke and Hidenori!).
> > > >
> > > > With this heuristics we can use this camera again (see the /dev/video0
> > > > in the topology).
> > > >
> > > > I have tested this change in a 6.6 kernel. Because the notebook that I
> > > > used for testing has some issues running master. But for the purpose of
> > > > this change this test should work.
> > > >
> > > > ~ # media-ctl --print-topology
> > > > Media controller API version 6.6.99
> > > >
> > > > Media device information
> > > > ------------------------
> > > > driver          uvcvideo
> > > > model           GRANDSTREAM GUV3100: GRANDSTREA
> > > > serial
> > > > bus info        usb-0000:00:14.0-9
> > > > hw revision     0x409
> > > > driver version  6.6.99
> > > >
> > > > Device topology
> > > > - entity 1: GRANDSTREAM GUV3100: GRANDSTREA (1 pad, 1 link)
> > > >             type Node subtype V4L flags 1
> > > >             device node name /dev/video0
> > > >         pad0: SINK
> > > >                 <- "Extension 3":1 [ENABLED,IMMUTABLE]
> > > >
> > > > - entity 4: GRANDSTREAM GUV3100: GRANDSTREA (0 pad, 0 link)
> > > >             type Node subtype V4L flags 0
> > > >             device node name /dev/video1
> > > >
> > > > - entity 8: Extension 3 (2 pads, 2 links, 0 routes)
> > > >             type V4L2 subdev subtype Unknown flags 0
> > > >         pad0: SINK
> > > >                 <- "Processing 2":1 [ENABLED,IMMUTABLE]
> > > >         pad1: SOURCE
> > > >                 -> "GRANDSTREAM GUV3100: GRANDSTREA":0 [ENABLED,IMMUTABLE]
> > > >
> > > > - entity 11: Processing 2 (2 pads, 3 links, 0 routes)
> > > >              type V4L2 subdev subtype Unknown flags 0
> > > >         pad0: SINK
> > > >                 <- "Camera 1":0 [ENABLED,IMMUTABLE]
> > > >         pad1: SOURCE
> > > >                 -> "Extension 3":0 [ENABLED,IMMUTABLE]
> > > >                 -> "Extension 4":0 [ENABLED,IMMUTABLE]
> > > >
> > > > - entity 14: Extension 4 (2 pads, 1 link, 0 routes)
> > > >              type V4L2 subdev subtype Unknown flags 0
> > > >         pad0: SINK
> > > >                 <- "Processing 2":1 [ENABLED,IMMUTABLE]
> > > >         pad1: SOURCE
> > > >
> > > > - entity 17: Camera 1 (1 pad, 1 link, 0 routes)
> > > >              type V4L2 subdev subtype Sensor flags 0
> > > >         pad0: SOURCE
> > > >                 -> "Processing 2":0 [ENABLED,IMMUTABLE]
> > > > ---
> > > > Changes in v2:
> > > > - Fix : invalid reference to the index variable of the iterator.
> > > > - Link to v1: https://lore.kernel.org/r/20251021-uvc-grandstream-v1-1-801e3d08b271@chromium.org
> > > > ---
> > > >  drivers/media/usb/uvc/uvc_driver.c | 15 ++++++++++++++-
> > > >  1 file changed, 14 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> > > > index fb6afb8e84f00961f86fd8f840fba48d706d7a9a..ee4f54d6834962414979a046afc59c5036455124 100644
> > > > --- a/drivers/media/usb/uvc/uvc_driver.c
> > > > +++ b/drivers/media/usb/uvc/uvc_driver.c
> > > > @@ -167,13 +167,26 @@ static struct uvc_entity *uvc_entity_by_reference(struct uvc_device *dev,
> > > >
> > > >  static struct uvc_streaming *uvc_stream_by_id(struct uvc_device *dev, int id)
> > > >  {
> > > > -     struct uvc_streaming *stream;
> > > > +     struct uvc_streaming *stream, *last_stream;
> > > > +     unsigned int count = 0;
> > > >
> > > >       list_for_each_entry(stream, &dev->streams, list) {
> > > > +             count += 1;
> > > > +             last_stream = stream;
> > > >               if (stream->header.bTerminalLink == id)
> > > >                       return stream;
> > > >       }
> > > >
> > > > +     /*
> > > > +      * If the streaming entity is referenced by an invalid ID, notify the
> > > > +      * user and use heuristics to guess the correct entity.
> > > > +      */
> > > > +     if (count == 1 && id == UVC_INVALID_ENTITY_ID) {
> > > > +             dev_warn(&dev->intf->dev,
> > > > +                      "UVC non compliance: Invalid USB header. The streaming entity has an invalid ID, guessing the correct one.");
> > > > +             return last_stream;
> > > > +     }
> > >
> > > As far as I understand, the reason why we can't find the streaming
> > > interface here is because we have set the streaming output terminal ID
> > > to UVC_INVALID_ENTITY_ID, due to the extension unit being previously
> > > registered with the same ID. We're therefore adding a workaround on top
> > > of another workaround.
> > >
> > > Looking at the UVC descriptors for this camera, ID 4 is shared between
> > > an extension unit with GUID ffe52d21-8030-4e2c-82d9-f587d00540bd and the
> > > streaming output terminal. That GUID is apparently a Logitech GUID
> > > (according to https://github.com/soyersoyer/cameractrls/blob/c920e/cameractrls.py).
> > > I don't know if the XU actually works in that camera, but it could be
> > > useful to keep it functional.
> > >
> > > I think we could make the handling of non-unique IDs more clever. If an
> > > output streaming terminal and a unit share the same ID, we could
> > > allocate an unused ID for the output streaming terminal, and patch the
> > > bTerminalLink of the stream headers accordingly. The ID remapping should
> > > not cause other issues, as
> > >
> > > - streaming output terminals have no controls, so the ID isn't used to
> > >   reference controls
> > >
> > > - the units graph is build from sink to source, with UVC descriptors
> > >   containing the IDs of source units, so streaming output terminals are
> > >   never referenced by ID from descriptors except for the bTerminalLink.
> > >
> > > This would produce a valid graph without duplicated IDs in this case.

Thinking some more about this, I wonder if the fix couldn't be as simple
as putting the streaming OTTs in a separate ID namespace:

diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index fb6afb8e84f0..17a8cc96e6ed 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -969,6 +969,7 @@ static int uvc_parse_standard_control(struct uvc_device *dev,
 	struct usb_host_interface *alts = dev->intf->cur_altsetting;
 	unsigned int i, n, p, len;
 	const char *type_name;
+	unsigned int id;
 	u16 type;
 
 	switch (buffer[2]) {
@@ -1107,8 +1108,14 @@ static int uvc_parse_standard_control(struct uvc_device *dev,
 			return 0;
 		}
 
+		id = buffer[3];
+
+		/* Give streaming output terminals their own ID namespace. */
+		if (type & TT_STREAMING)
+			id |= UVC_TERM_OUTPUT;
+
 		term = uvc_alloc_new_entity(dev, type | UVC_TERM_OUTPUT,
-					    buffer[3], 1, 0);
+					    id, 1, 0);
 		if (IS_ERR(term))
 			return PTR_ERR(term);
 
@@ -2105,7 +2112,7 @@ static int uvc_register_terms(struct uvc_device *dev,
 		if (UVC_ENTITY_TYPE(term) != UVC_TT_STREAMING)
 			continue;
 
-		stream = uvc_stream_by_id(dev, term->id);
+		stream = uvc_stream_by_id(dev, term->id & ~UVC_TERM_OUTPUT);
 		if (stream == NULL) {
 			dev_info(&dev->intf->dev,
 				 "No streaming interface found for terminal %u.",


This is untested, and requires better comments to explain the
heuristics, as well as possibly a macro to convert between the "raw" ID
and the "namespaced" ID.

> > I will try to work on a solution like you propose in this kernel
> > cycle, but I do not feel very confident about landing that big change
> > in "fixes" without being in linux-next for a couple of weeks. I'd
> > rather go for a simple solution that fixes the regression.
> > 
> > With this fix, the media controller topology looks identical as before
> > we introduce the bug, so we do not lose any functionality.
> > 
> > So if you agree I think the safest path is to land this fix in fixes
> > now. And try to get a better logic landed in next in this kernel
> > cycle.
> > 
> > What do you think?
> 
> I'm fine with that timelie. Thanks for agreeing to try and implement the
> above proposal.
> 
> > > > +
> > > >       return NULL;
> > > >  }
> > > >
> > > >
> > > > ---
> > > > base-commit: ea299a2164262ff787c9d33f46049acccd120672
> > > > change-id: 20251021-uvc-grandstream-05ecf0288f62

-- 
Regards,

Laurent Pinchart