[FFmpeg-devel] [PATCH v2 02/27] avcodec/decode: Add new ProgressFrame API

[Andreas Rheinhardt]

Anton Khirnov:
> Quoting Andreas Rheinhardt (2024-04-10 10:02:55)
>> Anton Khirnov:
>>> Quoting Andreas Rheinhardt (2024-04-10 09:09:00)
>>>> Anton Khirnov:
>>>>> Quoting Andreas Rheinhardt (2024-04-08 22:13:40)
>>>>>> diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
>>>>>> index fd356bd190..6b2c4312e0 100644
>>>>>> --- a/libavcodec/pthread_frame.c
>>>>>> +++ b/libavcodec/pthread_frame.c
>>>>>> @@ -779,6 +779,7 @@ static av_cold int init_thread(PerThreadContext *p, int *threads_to_free,
>>>>>>      if (!copy->internal)
>>>>>>          return AVERROR(ENOMEM);
>>>>>>      copy->internal->thread_ctx = p;
>>>>>> +    copy->internal->progress_frame_pool = avctx->internal->progress_frame_pool;
>>>>>
>>>>> I'd still prefer every thread to have its own reference.
>>>>>
>>>>> Looks good otherwise.
>>>>>
>>>>
>>>> The opaque of this pool is the main AVCodecContext; if the main
>>>> AVCodecContext is destroyed, the pool is in a state where one can no
>>>> longer get new entries from it. So giving every thread its own reference
>>>> is pretending to make it an equal co-owner of the pool, but it is not as
>>>> the pool must not outlive the main AVCodecContext.
>>>
>>> But the only use of that opaque is checking whether frame threading is
>>> in use, which is a constant during decoder lifetime. Might be cleaner to
>>> avoid using AVCodecContext as opaque.
>>> In any case, this is not important, feel free to leave it as is.
>>>
>>
>> But whether frame threading is in use is only determined after
>> ff_decode_preinit().
> 
> I realize, which is why I'm not insisting you change this.
> 
> But it could be handled, e.g. by allocating the pool later, or splitting
> off the part of thread init that determines active_thread_type.
> 

Which is IMO less elegant than simply doing what I did here.

- Andreas