Re: [PATCH v3 4/4] uvcvideo: add a metadata device node

[Guennadi Liakhovetski <g.liakhovetski@gmx.de>]

Hi Laurent,

On Fri, 30 Dec 2016, Laurent Pinchart wrote:

> Hi Guennadi,
> 
> On Friday 30 Dec 2016 14:04:34 Guennadi Liakhovetski wrote:
> > On Fri, 30 Dec 2016, Laurent Pinchart wrote:
> > > On Friday 30 Dec 2016 11:43:02 Guennadi Liakhovetski wrote:
> > >> Hi Laurent,
> > >> 
> > >> I'd like to discuss extending this patch a bit, preferably as an
> > >> incremental patch.
> > >> 
> > >> First let me confirm my current understanding of the way the UVC driver
> > >> creates its media device topology. Do I understand it correctly, that
> > >> the driver allocates UVC entities (not media controller entities) for all
> > >> UVC units and terminals, but then uses subdevices for all such UVC
> > >> entities, except terminals, i.e. only for UVC units? struct uvc_entity
> > >> has an embedded struct v4l2_subdev object, but it's unused for UVC
> > >> terminals. Instead terminals are associated to video devices, which are
> > >> then linked into the MC topology? Is this my understanding correct?
> > > 
> > > That's correct, but looking at the code now, I think the driver should use
> > > a struct media_entity directly instead of a struct v4l2_subdev as it
> > > doesn't need any of the infrastructure provided by subdevs.
> > > 
> > >> I have a problem with the current version of this patch, that there is
> > >> no way to associate video device nodes with respepctive metadata nodes.
> > >> Would it be acceptable to use an MC link for this association?
> > > 
> > > No, links describe data connections.
> > 
> > Well, it is data - it's metadata, extracted from USB buffers.

A further argument to this cause: currently the metadata node isn't 
connected to anything, it's "floating freely," which is wrong too. It does 
stream data and we have to bind it somewhere. I see 2 possibilities: 
either link them to the same entity, as the actual video data, adding more 
endpoints to it, or link them to video nodes. The latter way would allow 
for easy matching. If we use the former approach, we could still agree to 
have pad #2*n and #2*n+1 to be matching video and metadata pads, but that 
would be less straight-forward.

Thanks
Guennadi

> > >> Is it allowed for video device MC entities to have source pads
> > >> additionally to their (usually single) sink pad(s) (in case of input
> > >> video devices)? If that would be acceptable, I could create an additional
> > >> patch to add a source pad to output terminal video nodes to link it to
> > >> metadata nodes.
> > > 
> > > That's a hack, I don't think it's a good idea.
> > 
> > Ok, would a completely specialised one-off sysfs solution be better? Maybe
> > a link under the metadata node to the main node?
> 
> Come on, I know you're better than that. Stop thinking short term about the 
> quickest hack that can provide the feature you need.
> 
> -- 
> Regards,
> 
> Laurent Pinchart
>