On 12/18/18 3:21 PM, Paul Kocialkowski wrote: > Hi, > > On Thu, 2018-12-13 at 13:28 +0100, Hans Verkuil 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. > > Interesting corner case indeed, we hadn't considered the possibility of > interlaced pictures refeering to the current capture buffer. > > Hans, do you want to include that change in a future revision of this > series or should that be a separate follow-up patch? > > I'm fine with both options (and could definitely craft the change in > the latter case). If you can make a separate patch for this, then that would be great! Regards, Hans > > Cheers, > > Paul > >>> In our sample code we only keep at most one output, one capture buffer >>> in queue >>> and use buffer indices as tag/timestamp to simplify buffer handling. >>> FFmpeg keeps track of buffers/frames referenced and a buffer will not be >>> reused >>> until the codec and display pipeline have released all references to it. >>> >>> Sample code having interlaced and multi-slice support using previous tag >>> version of this patchset can be found at: >>> https://github.com/jernejsk/LibreELEC.tv/blob/hw_dec_ffmpeg/projects/Allwinner/patches/linux/0025-H264-fixes.patch#L120-L124 >>> https://github.com/Kwiboo/FFmpeg/compare/4.0.3-Leia-Beta5...v4l2-request-hwaccel >>> >>> Regards, >>> Jonas
