| Message ID | 20181212123901.34109-7-hverkuil-cisco@xs4all.nl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vb2/cedrus: use timestamps to identify buffers | expand |
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? Could vb2_find_timestamp be extended to allow QUEUED buffers to be returned? 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 On 2018-12-12 13:38, hverkuil-cisco@xs4all.nl wrote: > From: Hans Verkuil <hverkuil-cisco@xs4all.nl> > > Use v4l2_timeval_to_ns instead of timeval_to_ns to ensure that > both kernelspace and userspace will use the same conversion > function. > > Next add a new vb2_find_timestamp() function to find buffers > with a specific timestamp. > > This function will only look at DEQUEUED and DONE buffers, i.e. > buffers that are already processed. > > Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > --- > .../media/common/videobuf2/videobuf2-v4l2.c | 22 +++++++++++++++++-- > include/media/videobuf2-v4l2.h | 19 +++++++++++++++- > 2 files changed, 38 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c > index 1244c246d0c4..8d1231c2da65 100644 > --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c > +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c > @@ -143,7 +143,7 @@ static void __copy_timestamp(struct vb2_buffer *vb, const void *pb) > * and the timecode field and flag if needed. > */ > if (q->copy_timestamp) > - vb->timestamp = timeval_to_ns(&b->timestamp); > + vb->timestamp = v4l2_timeval_to_ns(&b->timestamp); > vbuf->flags |= b->flags & V4L2_BUF_FLAG_TIMECODE; > if (b->flags & V4L2_BUF_FLAG_TIMECODE) > vbuf->timecode = b->timecode; > @@ -460,7 +460,8 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, void *pb) > b->flags = vbuf->flags; > b->field = vbuf->field; > b->timestamp = ns_to_timeval(vb->timestamp); > - b->timecode = vbuf->timecode; > + if (b->flags & V4L2_BUF_FLAG_TIMECODE) > + b->timecode = vbuf->timecode; > b->sequence = vbuf->sequence; > b->reserved2 = 0; > b->request_fd = 0; > @@ -586,6 +587,23 @@ static const struct vb2_buf_ops v4l2_buf_ops = { > .copy_timestamp = __copy_timestamp, > }; > > +int vb2_find_timestamp(const struct vb2_queue *q, u64 timestamp, > + unsigned int start_idx) > +{ > + unsigned int i; > + > + for (i = start_idx; i < q->num_buffers; i++) { > + struct vb2_buffer *vb = q->bufs[i]; > + > + if ((vb->state == VB2_BUF_STATE_DEQUEUED || > + vb->state == VB2_BUF_STATE_DONE) && > + vb->timestamp == timestamp) > + return i; > + } > + return -1; > +} > +EXPORT_SYMBOL_GPL(vb2_find_timestamp); > + > /* > * vb2_querybuf() - query video buffer information > * @q: videobuf queue > diff --git a/include/media/videobuf2-v4l2.h b/include/media/videobuf2-v4l2.h > index 727855463838..80f1afa0edad 100644 > --- a/include/media/videobuf2-v4l2.h > +++ b/include/media/videobuf2-v4l2.h > @@ -32,7 +32,7 @@ > * &enum v4l2_field. > * @timecode: frame timecode. > * @sequence: sequence count of this frame. > - * @request_fd: the request_fd associated with this buffer > + * @request_fd: the request_fd associated with this buffer. > * @planes: plane information (userptr/fd, length, bytesused, data_offset). > * > * Should contain enough information to be able to cover all the fields > @@ -55,6 +55,23 @@ struct vb2_v4l2_buffer { > #define to_vb2_v4l2_buffer(vb) \ > container_of(vb, struct vb2_v4l2_buffer, vb2_buf) > > +/** > + * vb2_find_timestamp() - Find buffer with given timestamp in the queue > + * > + * @q: pointer to &struct vb2_queue with videobuf2 queue. > + * @timestamp: the timestamp to find. Only buffers in state DEQUEUED or DONE > + * are considered. > + * @start_idx: the start index (usually 0) in the buffer array to start > + * searching from. Note that there may be multiple buffers > + * with the same timestamp value, so you can restart the search > + * by setting @start_idx to the previously found index + 1. > + * > + * Returns the buffer index of the buffer with the given @timestamp, or > + * -1 if no buffer with @timestamp was found. > + */ > +int vb2_find_timestamp(const struct vb2_queue *q, u64 timestamp, > + unsigned int start_idx); > + > int vb2_querybuf(struct vb2_queue *q, struct v4l2_buffer *b); > > /**
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. Regards, Hans > > > 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
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). 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
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
On Thu, Dec 13, 2018 at 9:28 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> 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
On 2018-12-19 06:10, Tomasz Figa wrote: > On Thu, Dec 13, 2018 at 9:28 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> 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. 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
On Wed, Dec 19, 2018 at 4:04 PM Jonas Karlman <jonas@kwiboo.se> wrote: > > On 2018-12-19 06:10, Tomasz Figa wrote: > > On Thu, Dec 13, 2018 at 9:28 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> 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
On 2018-12-19 08:16, Tomasz Figa wrote: > On Wed, Dec 19, 2018 at 4:04 PM Jonas Karlman <jonas@kwiboo.se> wrote: >> On 2018-12-19 06:10, Tomasz Figa wrote: >>> On Thu, Dec 13, 2018 at 9:28 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> 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? 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
On Wed, Dec 19, 2018 at 6:18 PM Jonas Karlman <jonas@kwiboo.se> wrote: > > On 2018-12-19 08:16, Tomasz Figa wrote: > > On Wed, Dec 19, 2018 at 4:04 PM Jonas Karlman <jonas@kwiboo.se> wrote: > >> On 2018-12-19 06:10, Tomasz Figa wrote: > >>> On Thu, Dec 13, 2018 at 9:28 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> 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
On 12/19/2018 08:16 AM, Tomasz Figa wrote: > On Wed, Dec 19, 2018 at 4:04 PM Jonas Karlman <jonas@kwiboo.se> wrote: >> >> On 2018-12-19 06:10, Tomasz Figa wrote: >>> On Thu, Dec 13, 2018 at 9:28 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> 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) When C(0) is queued its timestamp field is zeroed by vb2. So it can no longer be used as a reference. For now I want to keep the behavior like that (i.e. once you requeue a capture buffer it can no longer be used as a reference frame for the decoder). This limitation is something we might want to lift in the future, but this would require more work internally. Regards, Hans > <- 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
diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c index 1244c246d0c4..8d1231c2da65 100644 --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c @@ -143,7 +143,7 @@ static void __copy_timestamp(struct vb2_buffer *vb, const void *pb) * and the timecode field and flag if needed. */ if (q->copy_timestamp) - vb->timestamp = timeval_to_ns(&b->timestamp); + vb->timestamp = v4l2_timeval_to_ns(&b->timestamp); vbuf->flags |= b->flags & V4L2_BUF_FLAG_TIMECODE; if (b->flags & V4L2_BUF_FLAG_TIMECODE) vbuf->timecode = b->timecode; @@ -460,7 +460,8 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, void *pb) b->flags = vbuf->flags; b->field = vbuf->field; b->timestamp = ns_to_timeval(vb->timestamp); - b->timecode = vbuf->timecode; + if (b->flags & V4L2_BUF_FLAG_TIMECODE) + b->timecode = vbuf->timecode; b->sequence = vbuf->sequence; b->reserved2 = 0; b->request_fd = 0; @@ -586,6 +587,23 @@ static const struct vb2_buf_ops v4l2_buf_ops = { .copy_timestamp = __copy_timestamp, }; +int vb2_find_timestamp(const struct vb2_queue *q, u64 timestamp, + unsigned int start_idx) +{ + unsigned int i; + + for (i = start_idx; i < q->num_buffers; i++) { + struct vb2_buffer *vb = q->bufs[i]; + + if ((vb->state == VB2_BUF_STATE_DEQUEUED || + vb->state == VB2_BUF_STATE_DONE) && + vb->timestamp == timestamp) + return i; + } + return -1; +} +EXPORT_SYMBOL_GPL(vb2_find_timestamp); + /* * vb2_querybuf() - query video buffer information * @q: videobuf queue diff --git a/include/media/videobuf2-v4l2.h b/include/media/videobuf2-v4l2.h index 727855463838..80f1afa0edad 100644 --- a/include/media/videobuf2-v4l2.h +++ b/include/media/videobuf2-v4l2.h @@ -32,7 +32,7 @@ * &enum v4l2_field. * @timecode: frame timecode. * @sequence: sequence count of this frame. - * @request_fd: the request_fd associated with this buffer + * @request_fd: the request_fd associated with this buffer. * @planes: plane information (userptr/fd, length, bytesused, data_offset). * * Should contain enough information to be able to cover all the fields @@ -55,6 +55,23 @@ struct vb2_v4l2_buffer { #define to_vb2_v4l2_buffer(vb) \ container_of(vb, struct vb2_v4l2_buffer, vb2_buf) +/** + * vb2_find_timestamp() - Find buffer with given timestamp in the queue + * + * @q: pointer to &struct vb2_queue with videobuf2 queue. + * @timestamp: the timestamp to find. Only buffers in state DEQUEUED or DONE + * are considered. + * @start_idx: the start index (usually 0) in the buffer array to start + * searching from. Note that there may be multiple buffers + * with the same timestamp value, so you can restart the search + * by setting @start_idx to the previously found index + 1. + * + * Returns the buffer index of the buffer with the given @timestamp, or + * -1 if no buffer with @timestamp was found. + */ +int vb2_find_timestamp(const struct vb2_queue *q, u64 timestamp, + unsigned int start_idx); + int vb2_querybuf(struct vb2_queue *q, struct v4l2_buffer *b); /**