On Wed, Dec 19, 2018 at 6:18 PM Jonas Karlman <jonas@xxxxxxxxx> wrote: > > On 2018-12-19 08:16, Tomasz Figa wrote: > > 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. > I still think this may be an invalid use-case or otherwise bad handling > from userspace. Since userspace knows that C(0) won't be referenced in > next frame it should also know that it still has two frames queued for > decoding that references C(0) frame and still have not been returned > from decoder. > And if the driver have not started decoding the requests/frames that > reference C(0) how would it know that it cannot pick the now queued C(0) > as output for next frame it decodes? Because the application has queued 2 other CAPTURE buffers before C(0), but indeed, if we assume that the driver can skip buffers (due to some errors that I don't see how could happen in any real life scenario), then that could fail. > In FFmpeg case we only re-queue the buffer once all frames that > references that buffer/frame have been output from decoder, I guess the > main difference is that FFmpeg currently only keep one request in flight > at the time (wait on request to finish before queue next), this may > change in future as we continue to improve the v4l2request hwaccel. > > Regards, > Jonas > > > >> 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
