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

[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.

-- 
Anton Khirnov