Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Mark Thompson <sw@jkqxz.net>
To: ffmpeg-devel@ffmpeg.org
Subject: Re: [FFmpeg-devel] [PATCH] ffmpeg: set extra_hw_frames to account for frames held in queues
Date: Sun, 25 Feb 2024 15:15:27 +0000
Message-ID: <7c629c14-3bde-4433-8b70-c86180b6c976@jkqxz.net> (raw)
In-Reply-To: <f4533df1-ad1f-1406-9497-75bd859aab4c@passwd.hu>

On 25/02/2024 15:01, Marton Balint wrote:
> On Sun, 25 Feb 2024, 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.
> 
> [...]
> 
>> diff --git a/fftools/ffmpeg_sched.h b/fftools/ffmpeg_sched.h
>> index 95f9c1d4db..315053ae42 100644
>> --- a/fftools/ffmpeg_sched.h
>> +++ b/fftools/ffmpeg_sched.h
>> @@ -233,6 +233,13 @@ 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 thread queue, used if thread_queue_size is not set on a
>> + * call to sch_add_mux().
> 
> Not precisely, as this thread queue size is used for both frame queues and packet queues.

Yes, it applies to both - hence the description I added not mentioning frames or packets.

> Historically the thread_queue_size option was introduced for packet queues for demuxed packets, and recently on the output for muxing packets.
> 
> If we want to make the frame queue size adjustable as well, I think it should be a separate option and maybe a separate constant should be added for its default value.

This part is not changing anything about the queue sizes, it is just moving the existing magic number hidden in queue_alloc() to a named constant.

I don't have any motivation to make the frame queue size adjustable; I added the assert so that if someone wants to do that in future they know that they need to take additional action to avoid breaking some decoders again.

>> + */
>> +#define DEFAULT_THREAD_QUEUE_SIZE 8

Would you prefer that I make distinct DEFAULT_FRAME_THREAD_QUEUE_SIZE and DEFAULT_PACKET_THREAD_QUEUE_SIZE (both 8?) and replace the magic number in queue_alloc() with a selection between them based on the type?  I have no strong opinion on that, so I don't mind doing it if you would prefer it.

Thanks,

- Mark
_______________________________________________
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".

  reply	other threads:[~2024-02-25 15:15 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-25 11:18 Mark Thompson
2024-02-25 15:01 ` Marton Balint
2024-02-25 15:15   ` Mark Thompson [this message]
2024-02-25 16:02     ` Marton Balint
2024-02-25 16:17       ` [FFmpeg-devel] [PATCH v2] " 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=7c629c14-3bde-4433-8b70-c86180b6c976@jkqxz.net \
    --to=sw@jkqxz.net \
    --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