On Tue, Sep 11, 2018 at 5:48 PM Hans Verkuil <[email protected]> wrote: > > On 09/11/18 10:40, Alexandre Courbot wrote: > >> I am not sure whether this should be documented, but there are some > >> additional > >> restrictions w.r.t. reference frames: > >> > >> Since decoders need access to the decoded reference frames there are some > >> corner > >> cases that need to be checked: > >> > >> 1) V4L2_MEMORY_USERPTR cannot be used for the capture queue: the driver > >> does not > >> know when a malloced but dequeued buffer is freed, so the reference > >> frame > >> could suddenly be gone. > > > > It it is confirmed that we cannot use USERPTR buffers as CAPTURE then > > this probably needs to be documented. I wonder if there isn't a way to > > avoid this by having vb2 keep a reference to the pages in such a way > > that they would not be recycled after after userspace calls free() on > > the buffer. Is that possible with user-allocated memory? > > vb2 keeps a reference while the buffer is queued, but that reference is > released once the buffer is dequeued. Correctly, IMHO. If you provide > USERPTR, than userspace is responsible for the memory. Changing this > would require changing the API, since USERPTR has always worked like > this.
That would be a userspace bug wouldn't it? The next try to get user pages would fail in that case and we could just fail such decode request, couldn't we? (Personally I'm not a big fan of USERPTR, though.) > > > > > Not that I think that forbidding USERPTR buffers in this scenario > > would be a big problem. > > I think it is perfectly OK to forbid this. Ideally I would like to have > some test in v4l2-compliance or (even better) v4l2-mem2mem.c for this, > but it is actually not that easy to identify drivers like this. > > Suggestions for this on a postcard... > > > > >> > >> 2) V4L2_MEMORY_DMABUF can be used, but drivers should check that the dma > >> buffer is > >> still available AND increase the dmabuf refcount while it is used by > >> the HW. > > > > Yeah, with DMABUF the above problem can easily be avoided at least. > > > >> > >> 3) What to do if userspace has requeued a buffer containing a reference > >> frame, > >> and you want to decode a B/P-frame that refers to that buffer? We need > >> to > >> check against that: I think that when you queue a capture buffer whose > >> index > >> is used in a pending request as a reference frame, than that should > >> fail with > >> an error. And trying to queue a request referring to a buffer that has > >> been > >> requeued should also fail. > >> > >> We might need to add some support for this in v4l2-mem2mem.c or vb2. > > > > Sounds good, and we should document this as well. > > > > Right. And test it! I'm not convinced that we should be enforcing this. Moreover, requeuing a buffer containing a reference frame for a pending request is not bound to be an error. It might be a legit case when the same entry in the reference list is replaced with a different key frame decoded into the same buffer as the reference list entry pointed until now. Best regards, Tomasz
