On Thu, Dec 13, 2018 at 9:28 PM Hans Verkuil <hverkuil-cisco@xxxxxxxxx> wrote: > > On 12/12/18 7:28 PM, Jonas Karlman wrote: > > Hi Hans, > > > > Since this function only return DEQUEUED and DONE buffers, > > it cannot be used to find a capture buffer that is both used for > > frame output and is part of the frame reference list. > > E.g. a bottom field referencing a top field that is already > > part of the capture buffer being used for frame output. > > (top and bottom field is output in same buffer) > > > > Jernej Škrabec and me have worked around this issue in cedrus driver by > > first checking > > the tag/timestamp of the current buffer being used for output frame. > > > > > > // field pictures may reference current capture buffer and is not > > returned by vb2_find_tag > > if (v4l2_buf->tag == dpb->tag) > > buf_idx = v4l2_buf->vb2_buf.index; > > else > > buf_idx = vb2_find_tag(cap_q, dpb->tag, 0); > > > > > > What is the recommended way to handle such case? > > That is the right approach for this. Interesting corner case, I hadn't > considered that. > > > Could vb2_find_timestamp be extended to allow QUEUED buffers to be returned? > > No, because only the driver knows what the current buffer is. > > Buffers that are queued to the driver are in state ACTIVE. But there may be > multiple ACTIVE buffers and vb2 doesn't know which buffer is currently > being processed by the driver. > > So this will have to be checked by the driver itself. Hold on, it's a perfectly valid use case to have the buffer queued but still used as a reference for previously queued buffers, e.g. QBUF(O, 0) QBUF(C, 0) REF(ref0, out_timestamp(0)) QBUF(O, 1) QBUF(C, 1) REF(ref0, out_timestamp(0)) QBUF(O, 2) QBUF(C, 2) <- driver returns O(0) and C(0) here <- userspace also knows that any next frame will not reference C(0) anymore REF(ref0, out_timestamp(2)) QBUF(O, 0) QBUF(C, 0) <- driver may pick O(1)+C(1) or O(2)+C(2) to decode here, but C(0) which is the reference for it is already QUEUED. It's a perfectly fine scenario and optimal from pipelining point of view, but if I'm not missing something, the current patch wouldn't allow it. Best regards, Tomasz
