[FFmpeg-devel] [PATCH] ffmpeg: set extra_hw_frames to account for frames held in queues

[Mark Thompson]

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