[FFmpeg-devel] [PATCH] ffmpeg: release the last_frame before the decoders are closed

[Steve Lhomme]

On 16/10/2018 16:59, Hendrik Leppkes wrote:
> On Tue, Oct 16, 2018 at 4:02 PM James Almer <jamrial at gmail.com> wrote:
>> On 10/16/2018 9:16 AM, Steve Lhomme wrote:
>>> If the decoder provides its own buffers it might not be able to release its
>>> buffers once it has been closed. (this is the case with dav1d).
>>> ---
>>>   fftools/ffmpeg.c | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
>>> index da4259a9a8..faf62475a2 100644
>>> --- a/fftools/ffmpeg.c
>>> +++ b/fftools/ffmpeg.c
>>> @@ -4738,6 +4738,7 @@ static int transcode(void)
>>>           if (ost->encoding_needed) {
>>>               av_freep(&ost->enc_ctx->stats_in);
>>>           }
>>> +        av_frame_unref(ost->last_frame);
>>>           total_packets_written += ost->packets_written;
>>>       }
>> I'm not against this change, but this issue should be solved within
>> dav1d as well. Either the caller fully owns the picture returned by
>> dav1d, or it doesn't. "Partially owns it while some other context is
>> still valid" is not really acceptable.

You can't ask any library to own content even after you have closed it 
(and potentially unloaded the DLL).

> I agree. AVFrames guarantee to remain valid independently of which
> component they ever came out of. So if dav1d cannot provide that,
> either we need to solve that in the dav1d wrapper (ie. by keeping  an
> instance to the dav1d context tied to the frame like we do for some
> hardware frames), or dav1d fixes that.

If we provide a picture allocator to dav1d that wraps AVFrame we 
allocate it should not be an issue. We would own them no matter what.