Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: "Xiang, Haihao" <haihao.xiang-at-intel.com@ffmpeg.org>
To: "ffmpeg-devel@ffmpeg.org" <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH v3 1/2] ffmpeg: set extra_hw_frames to account for frames held in queues
Date: Mon, 18 Mar 2024 05:35:53 +0000
Message-ID: <4303f7bb70b5d702029c54327e07b5216602ffc8.camel@intel.com> (raw)
In-Reply-To: <c78d2af8-ff5f-4c9d-b723-a87928fbb031@jkqxz.net>

On So, 2024-03-17 at 20:49 +0000, Mark Thompson wrote:
> Since e0da916b8f5b079a4865eef7f64863f50785463d the ffmpeg utility has
> held multiple frames output by the decoder in internal queues without
> telling the decoder that it is going to do so.  When the decoder has a
> fixed-size pool of frames (common in some hardware APIs where the output
> frames must be stored as an array texture) this could lead to the pool
> being exhausted and the decoder getting stuck.  Fix this by telling the
> decoder to allocate additional frames according to the queue size.
> ---
> Rebased but otherwise unchanged since previous version.
> 
>   fftools/ffmpeg_dec.c   | 13 +++++++++++++
>   fftools/ffmpeg_sched.c | 16 +++++++++++++++-
>   fftools/ffmpeg_sched.h | 12 ++++++++++++
>   3 files changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/fftools/ffmpeg_dec.c b/fftools/ffmpeg_dec.c
> index c41c5748e5..ed411b6bf8 100644
> --- a/fftools/ffmpeg_dec.c
> +++ b/fftools/ffmpeg_dec.c
> @@ -1207,6 +1207,19 @@ static int dec_open(DecoderPriv *dp, AVDictionary
> **dec_opts,
>           return ret;
>       }
> 
> +    if (dp->dec_ctx->hw_device_ctx) {
> +        // Update decoder extra_hw_frames option to account for the
> +        // frames held in queues inside the ffmpeg utility.  This is
> +        // called after avcodec_open2() because the user-set value of
> +        // extra_hw_frames becomes valid in there, and we need to add
> +        // this on top of it.
> +        int extra_frames = DEFAULT_FRAME_THREAD_QUEUE_SIZE;
> +        if (dp->dec_ctx->extra_hw_frames >= 0)
> +            dp->dec_ctx->extra_hw_frames += extra_frames;
> +        else
> +            dp->dec_ctx->extra_hw_frames = extra_frames;
> +    }
> +
>       ret = check_avoptions(*dec_opts);
>       if (ret < 0)
>           return ret;
> diff --git a/fftools/ffmpeg_sched.c b/fftools/ffmpeg_sched.c
> index f739066921..ec88017e21 100644
> --- a/fftools/ffmpeg_sched.c
> +++ b/fftools/ffmpeg_sched.c
> @@ -365,7 +365,21 @@ static int queue_alloc(ThreadQueue **ptq, unsigned
> nb_streams, unsigned queue_si
>       ThreadQueue *tq;
>       ObjPool *op;
> 
> -    queue_size = queue_size > 0 ? queue_size : 8;
> +    if (queue_size <= 0) {
> +        if (type == QUEUE_FRAMES)
> +            queue_size = DEFAULT_FRAME_THREAD_QUEUE_SIZE;
> +        else
> +            queue_size = DEFAULT_PACKET_THREAD_QUEUE_SIZE;
> +    }
> +
> +    if (type == QUEUE_FRAMES) {
> +        // This queue length is used in the decoder code to ensure that
> +        // there are enough entries in fixed-size frame pools to account
> +        // for frames held in queues inside the ffmpeg utility.  If this
> +        // can ever dynamically change then the corresponding decode
> +        // code needs to be updated as well.
> +        av_assert0(queue_size == DEFAULT_FRAME_THREAD_QUEUE_SIZE);
> +    }
> 
>       op = (type == QUEUE_PACKETS) ? objpool_alloc_packets() :
>                                      objpool_alloc_frames();
> diff --git a/fftools/ffmpeg_sched.h b/fftools/ffmpeg_sched.h
> index a9190bd3d1..e51c26cec9 100644
> --- a/fftools/ffmpeg_sched.h
> +++ b/fftools/ffmpeg_sched.h
> @@ -233,6 +233,18 @@ int sch_add_filtergraph(Scheduler *sch, unsigned
> nb_inputs, unsigned nb_outputs,
>    */
>   int sch_add_mux(Scheduler *sch, SchThreadFunc func, int (*init)(void *),
>                   void *ctx, int sdp_auto, unsigned thread_queue_size);
> +
> +/**
> + * Default size of a packet thread queue.  For muxing this can be overridden
> by
> + * the thread_queue_size option as passed to a call to sch_add_mux().
> + */
> +#define DEFAULT_PACKET_THREAD_QUEUE_SIZE 8
> +
> +/**
> + * Default size of a frame thread queue.
> + */
> +#define DEFAULT_FRAME_THREAD_QUEUE_SIZE 8
> +
>   /**
>    * Add a muxed stream for a previously added muxer.

LGTM

- Haihao

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

  parent reply	other threads:[~2024-03-18  5:36 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-17 20:49 Mark Thompson
2024-03-17 20:51 ` [FFmpeg-devel] [PATCH v3 2/2] lavc/get_buffer: Add a warning on failed allocation from a fixed pool Mark Thompson
2024-03-18  5:53   ` Xiang, Haihao
2024-03-18 21:33     ` Mark Thompson
2024-03-19  4:16       ` Xiang, Haihao
2024-03-19 22:52         ` Mark Thompson
2024-03-25  7:35           ` Xiang, Haihao
2024-03-25 21:12             ` Mark Thompson
2024-03-18  5:35 ` Xiang, Haihao [this message]
2024-03-19 23:18   ` [FFmpeg-devel] [PATCH v3 1/2] ffmpeg: set extra_hw_frames to account for frames held in queues Mark Thompson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4303f7bb70b5d702029c54327e07b5216602ffc8.camel@intel.com \
    --to=haihao.xiang-at-intel.com@ffmpeg.org \
    --cc=ffmpeg-devel@ffmpeg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git