On Thu, 29. Jul 14:00, Ming Qian wrote:
> if client start the capture queue without waiting the source change
> event,
> there may be some timing issues.
> For example, in client, the sequence is:
> capture streamon -> source change -> capture streamoff -> capture
> streamon.
> but in driver side, the sequence may be:
> source change -> capture streamon -> capture streamoff -> capture
> streamon.
> Then it may led to some unforeseen and serious problems.
> So to avoid such timing issues, the client should setup the
> capture queue after the first source change event is received.
>
> Signed-off-by: Ming Qian <ming.q...@nxp.com>
> ---
> libavcodec/v4l2_context.c | 6 +++++-
> libavcodec/v4l2_m2m.c | 1 +
> libavcodec/v4l2_m2m.h | 1 +
> libavcodec/v4l2_m2m_dec.c | 34 +++++++++++++++-------------------
> 4 files changed, 22 insertions(+), 20 deletions(-)
>
> diff --git a/libavcodec/v4l2_context.c b/libavcodec/v4l2_context.c
> index df41e982fc56..543fc9523cf1 100644
> --- a/libavcodec/v4l2_context.c
> +++ b/libavcodec/v4l2_context.c
> @@ -179,6 +179,7 @@ static int v4l2_handle_event(V4L2Context *ctx)
> if (evt.type != V4L2_EVENT_SOURCE_CHANGE)
> return 0;
>
> + s->source_change_cnt++;
> ret = ioctl(s->fd, VIDIOC_G_FMT, &cap_fmt);
> if (ret) {
> av_log(logger(ctx), AV_LOG_ERROR, "%s VIDIOC_G_FMT\n",
> s->capture.name);
> @@ -272,7 +273,7 @@ static V4L2Buffer* v4l2_dequeue_v4l2buf(V4L2Context *ctx,
> int timeout)
> }
>
> /* if we are draining and there are no more capture buffers queued in
> the driver we are done */
> - if (!V4L2_TYPE_IS_OUTPUT(ctx->type) && ctx_to_m2mctx(ctx)->draining) {
> + if (!V4L2_TYPE_IS_OUTPUT(ctx->type) && ctx_to_m2mctx(ctx)->draining &&
> ctx->streamon) {
> for (i = 0; i < ctx->num_buffers; i++) {
> /* capture buffer initialization happens during decode hence
> * detection happens at runtime
> @@ -542,6 +543,9 @@ int ff_v4l2_context_set_status(V4L2Context* ctx, uint32_t
> cmd)
> int type = ctx->type;
> int ret;
>
> + if (ctx->streamon == (cmd == VIDIOC_STREAMON))
> + return 0;
> +
> ret = ioctl(ctx_to_m2mctx(ctx)->fd, cmd, &type);
> if (ret < 0)
> return AVERROR(errno);
> diff --git a/libavcodec/v4l2_m2m.c b/libavcodec/v4l2_m2m.c
> index cdfd579810f2..e2d5d8c968a9 100644
> --- a/libavcodec/v4l2_m2m.c
> +++ b/libavcodec/v4l2_m2m.c
> @@ -422,6 +422,7 @@ int ff_v4l2_m2m_create_context(V4L2m2mPriv *priv,
> V4L2m2mContext **s)
> priv->context->output.num_buffers = priv->num_output_buffers;
> priv->context->self_ref = priv->context_ref;
> priv->context->fd = -1;
> + priv->context->source_change_cnt = 0;
>
> priv->context->frame = av_frame_alloc();
> if (!priv->context->frame) {
> diff --git a/libavcodec/v4l2_m2m.h b/libavcodec/v4l2_m2m.h
> index b67b21633109..72f1a579f1ca 100644
> --- a/libavcodec/v4l2_m2m.h
> +++ b/libavcodec/v4l2_m2m.h
> @@ -53,6 +53,7 @@ typedef struct V4L2m2mContext {
> sem_t refsync;
> atomic_uint refcount;
> int reinit;
> + int source_change_cnt;
>
> /* null frame/packet received */
> int draining;
> diff --git a/libavcodec/v4l2_m2m_dec.c b/libavcodec/v4l2_m2m_dec.c
> index 224eb3d5e7be..60eb39deef88 100644
> --- a/libavcodec/v4l2_m2m_dec.c
> +++ b/libavcodec/v4l2_m2m_dec.c
> @@ -51,7 +51,7 @@ static int v4l2_try_start(AVCodecContext *avctx)
> }
> }
>
> - if (capture->streamon)
> + if (capture->streamon || !s->source_change_cnt)
> return 0;
>
> /* 2. get the capture format */
> @@ -146,28 +146,24 @@ static int v4l2_receive_frame(AVCodecContext *avctx,
> AVFrame *frame)
> return ret;
> }
>
> - if (s->draining)
> - goto dequeue;
> -
> - ret = ff_v4l2_context_enqueue_packet(output, &s->buf_pkt);
> - if (ret < 0 && ret != AVERROR(EAGAIN))
> - goto fail;
> -
> - /* if EAGAIN don't unref packet and try to enqueue in the next iteration
> */
> - if (ret != AVERROR(EAGAIN))
> - av_packet_unref(&s->buf_pkt);
> -
> if (!s->draining) {
> - ret = v4l2_try_start(avctx);
> - if (ret) {
> - /* cant recover */
> - if (ret != AVERROR(ENOMEM))
> - ret = 0;
> + ret = ff_v4l2_context_enqueue_packet(output, &s->buf_pkt);
> + if (ret < 0 && ret != AVERROR(EAGAIN))
> goto fail;
> - }
> +
> + /* if EAGAIN don't unref packet and try to enqueue in the next
> iteration */
> + if (ret != AVERROR(EAGAIN))
> + av_packet_unref(&s->buf_pkt);
> + }
> +
> + ret = v4l2_try_start(avctx);
> + if (ret) {
> + /* can't recover */
> + if (ret != AVERROR(ENOMEM))
> + ret = 0;
> + goto fail;
> }
>
> -dequeue:
> return ff_v4l2_context_dequeue_frame(capture, frame, -1);
> fail:
> av_packet_unref(&s->buf_pkt);
> --
> 2.32.0
>
I tested with a sample (RPi4) where the resolution change event is not
triggered at the start, so I think the capture queue is not initialized at all
following this
change. Will share with you the sample.
--
Andriy
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
