| Message ID | d7263f8fa822307371cbef7e4b41ad03b7bc19ea.1648101782.git.ming.qian@nxp.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/2] avcodec/v4l2_m2m_dec: add a dequeue_timeout parameter | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
24 Mar 2022, 07:14 by ming.qian@nxp.com: > the dequeue of capture queue will be blocked until > decoded frame available or an input buffer is ready to be dequeued. > but it may cause death waiting in some case. > For example, it has enqueued the first input frame, > and then blocks at ff_v4l2_context_dequeue_frame. > For some reason, the decoder can't decode the first frame, > also it can't return the input buffer. The decoder needs more input data > to decode, so the decoder is just waiting. > This creates some kind of deadlock. > > And in linux/Documentation/userspace-api/media/v4l/dev-decoder.rst, > there are some descriptor like below: > The client must not assume any direct relationship between CAPTURE and > OUTPUT buffers and any specific timing of buffers becoming available to dequeue. > Specifically: > [snip] > - a buffer queued to OUTPUT may result in a buffer being produced on CAPTURE > later into decode process, and/or after processing further OUTPUT buffers, > or be returned out of order, e.g. if display reordering is used, > > And I meet a similar case, there are 16 output buffers, but only one > buffer is queued to driver, then blocks at > ff_v4l2_context_dequeue_frame. > But the decoder need more frame data, otherwise it won't decode it. > > So I think the client should keep processing OUTPUT buffers if there are > available buffers until the end. > > To resolve it, I think we can set a reasonable timeout instead of -1. > So I add a parameter dequeue_timeout, and it works on my side. > Adding a timeout isn't a fix, it's a hack. Can't the driver return EAGAIN to ask for more data, or ENOBUFS if it has nothing to decode into? Why hasn't this been needed so far?
diff --git a/libavcodec/v4l2_m2m.h b/libavcodec/v4l2_m2m.h index 16e0a6d6b90f..5c7e7c955383 100644 --- a/libavcodec/v4l2_m2m.h +++ b/libavcodec/v4l2_m2m.h @@ -38,7 +38,9 @@ #define V4L_M2M_DEFAULT_OPTS \ { "num_output_buffers", "Number of buffers in the output context",\ - OFFSET(num_output_buffers), AV_OPT_TYPE_INT, { .i64 = 16 }, 6, INT_MAX, FLAGS } + OFFSET(num_output_buffers), AV_OPT_TYPE_INT, { .i64 = 16 }, 6, INT_MAX, FLAGS }, \ + { "dequeue_timeout", "timeout of dequeue in the capture context",\ + OFFSET(dequeue_timeout), AV_OPT_TYPE_INT, { .i64 = 32 }, -1, INT_MAX, FLAGS } typedef struct V4L2m2mContext { char devname[PATH_MAX]; @@ -76,6 +78,7 @@ typedef struct V4L2m2mPriv { int num_output_buffers; int num_capture_buffers; + int dequeue_timeout; } V4L2m2mPriv; /** diff --git a/libavcodec/v4l2_m2m_dec.c b/libavcodec/v4l2_m2m_dec.c index 4c521eba343c..e386e09e3e1e 100644 --- a/libavcodec/v4l2_m2m_dec.c +++ b/libavcodec/v4l2_m2m_dec.c @@ -138,6 +138,7 @@ static int v4l2_receive_frame(AVCodecContext *avctx, AVFrame *frame) V4L2m2mContext *s = ((V4L2m2mPriv*)avctx->priv_data)->context; V4L2Context *const capture = &s->capture; V4L2Context *const output = &s->output; + int timeout = ((V4L2m2mPriv*)avctx->priv_data)->dequeue_timeout; int ret; if (!s->buf_pkt.size) { @@ -172,7 +173,9 @@ static int v4l2_receive_frame(AVCodecContext *avctx, AVFrame *frame) } dequeue: - return ff_v4l2_context_dequeue_frame(capture, frame, -1); + if (s->draining) + timeout = -1; + return ff_v4l2_context_dequeue_frame(capture, frame, timeout); fail: av_packet_unref(&s->buf_pkt); return ret;
the dequeue of capture queue will be blocked until decoded frame available or an input buffer is ready to be dequeued. but it may cause death waiting in some case. For example, it has enqueued the first input frame, and then blocks at ff_v4l2_context_dequeue_frame. For some reason, the decoder can't decode the first frame, also it can't return the input buffer. The decoder needs more input data to decode, so the decoder is just waiting. This creates some kind of deadlock. And in linux/Documentation/userspace-api/media/v4l/dev-decoder.rst, there are some descriptor like below: The client must not assume any direct relationship between CAPTURE and OUTPUT buffers and any specific timing of buffers becoming available to dequeue. Specifically: [snip] - a buffer queued to OUTPUT may result in a buffer being produced on CAPTURE later into decode process, and/or after processing further OUTPUT buffers, or be returned out of order, e.g. if display reordering is used, And I meet a similar case, there are 16 output buffers, but only one buffer is queued to driver, then blocks at ff_v4l2_context_dequeue_frame. But the decoder need more frame data, otherwise it won't decode it. So I think the client should keep processing OUTPUT buffers if there are available buffers until the end. To resolve it, I think we can set a reasonable timeout instead of -1. So I add a parameter dequeue_timeout, and it works on my side. Signed-off-by: Ming Qian <ming.qian@nxp.com> --- libavcodec/v4l2_m2m.h | 5 ++++- libavcodec/v4l2_m2m_dec.c | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-)