On Wed, Dec 19, 2018 at 4:04 PM Jonas Karlman <jonas@xxxxxxxxx> wrote: > > On 2018-12-19 06:10, Tomasz Figa wrote: > > 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. > > This scenario should never happen with FFmpeg + v4l2request hwaccel + > Kodi userspace. > FFmpeg would only QBUF O(0)+C(0) again after it has been presented on > screen and Kodi have released the last reference to the AVFrame. > I skipped the display in the example indeed, but we can easily add it: QBUF(O, 0) QBUF(C, 0) REF(ref0, out_timestamp(0)) QBUF(O, 1) QBUF(C, 1) <- driver returns O(0) and C(0) here <- userspace displays C(0) REF(ref0, out_timestamp(0)) QBUF(O, 2) QBUF(C, 2) REF(ref0, out_timestamp(0)) QBUF(O, 3) QBUF(C, 3) <- driver returns O(1) and C(1) here <- userspace displays C(1) and reclaims C(0) <- userspace also knows that any next frame will not reference C(0) anymore REF(ref0, out_timestamp(3)) QBUF(O, 0) QBUF(C, 0) <- driver may pick O(3)+C(3) to decode here, but C(0) which is the reference for it is already QUEUED. Also the fact that FFmpeg wouldn't trigger such case doesn't mean that it's an invalid one. If I remember correctly, Chromium would actually trigger such, since we attempt to pipeline things as much as possible. > The v4l2request hwaccel will keep a AVFrame pool with preallocated > frames, AVFrame(x) is keeping userspace ref to O(x)+C(x). > An AVFrame will not be released back to the pool until FFmpeg have > removed it from DPB and Kodi have released it after it no longer is > being presented on screen. > > E.g. an IPBIPB sequense with display order 0 2 1 3 5 4 > > FFmpeg: AVFrame(0) > QBUF: O(0)+C(0) > DQBUF: O(0)+C(0) > Kodi: AVFrame(0) returned from FFmpeg and presented on screen > FFmpeg: AVFrame(1) with ref to AVFrame(0) > QBUF: O(1)+C(1) with ref to timestamp(0) > DQBUF: O(1)+C(1) > FFmpeg: AVFrame(2) with ref to AVFrame(0)+AVFrame(1) > QBUF: O(2)+C(2) with ref to timestamp(0)+timestamp(1) > DQBUF: O(2)+C(2) > Kodi: AVFrame(2) returned from FFmpeg and presented on screen > Kodi: AVFrame(0) released (no longer presented) > FFmpeg: AVFrame(3) > QBUF: O(3)+C(3) > DQBUF: O(3)+C(3) > Kodi: AVFrame(1) returned from FFmpeg and presented on screen > Kodi: AVFrame(2) released (no longer presented) > FFmpeg: AVFrame(2) returned to pool > FFmpeg: AVFrame(2) with ref to AVFrame(3) > QBUF: O(2)+C(2) with ref to timestamp(3) > DQBUF: O(2)+C(2) > Kodi: AVFrame(3) returned from FFmpeg and presented on screen > Kodi: AVFrame(1) released (no longer presented) > FFmpeg: AVFrame(0)+AVFrame(1) returned to pool (no longer referenced) > FFmpeg: AVFrame(0) with ref to AVFrame(3)+AVFrame(2) > QBUF: O(0)+C(0) with ref to timestamp(3)+timestamp(2) > DQBUF: O(0)+C(0) > Kodi: AVFrame(0) returned from FFmpeg and presented on screen > Kodi: AVFrame(3) released (no longer presented) > and so on > > Here we can see that O(0)+C(0) will not be QBUF until after FFmpeg + > Kodi have released all userspace refs to AVFrame(0). > Above example was simplified, Kodi will normally keep a few decoded > frames in buffer before being presented and FFmpeg will CREATE_BUF > anytime the pool is empty and new O/C buffers is needed. > > Regards, > Jonas > > > Best regards, > > Tomasz
