Hi Hans, On Wed, Nov 14, 2018 at 10:47 PM Hans Verkuil <hverk...@xs4all.nl> wrote: > > From: Hans Verkuil <hansv...@cisco.com> > > As was discussed here (among other places): > > https://lkml.org/lkml/2018/10/19/440 > > using capture queue buffer indices to refer to reference frames is > not a good idea. A better idea is to use a 'tag' where the > application can assign a u64 tag to an output buffer, which is then > copied to the capture buffer(s) derived from the output buffer. >
Thanks for the patches. Please see my comments below. > A u64 is chosen since this allows userspace to also use pointers to > internal structures as 'tag'. > I think this is not true anymore in this version. > The first three patches add core tag support, the next patch document > the tag support, then a new helper function is added to v4l2-mem2mem.c > to easily copy data from a source to a destination buffer that drivers > can use. > > Next a new supports_tags vb2_queue flag is added to indicate that > the driver supports tags. Ideally this should not be necessary, but > that would require that all m2m drivers are converted to using the > new helper function introduced in the previous patch. That takes more > time then I have now since we want to get this in for 4.20. > > Finally the vim2m, vicodec and cedrus drivers are converted to support > tags. > > I also removed the 'pad' fields from the mpeg2 control structs (it > should never been added in the first place) and aligned the structs > to a u32 boundary (u64 for the tag values). u32 in this version > > Note that this might change further (Paul suggested using bitfields). > > Also note that the cedrus code doesn't set the sequence counter, that's > something that should still be added before this driver can be moved > out of staging. > > Note: if no buffer is found for a certain tag, then the dma address > is just set to 0. That happened before as well with invalid buffer > indices. This should be checked in the driver! Note that DMA address 0 may as well be a valid address. Should we have another way of signaling that? > > The previous RFC series was tested successfully with the cedrus driver. > > Regards, > > Hans > > Changes since v1: > > - changed to a u32 tag. Using a 64 bit tag was overly complicated due > to the bad layout of the v4l2_buffer struct, and there is no real > need for it by applications. I hope these won't become famous last words. :) For Chromium it should be okay indeed. Since we've been thinking about a redesign of the buffer struct, perhaps we can live with u32 for now and we can design the new struct to handle u64 nicely? Best regards, Tomasz
