[FFmpeg-devel] [PATCH] Revert "avcodec/decode: copy the output parameters from the last bsf in the chain back to the AVCodecContext"

[James Almer]

On 9/12/2018 6:44 PM, Hendrik Leppkes wrote:
> On Wed, Sep 12, 2018 at 8:15 PM James Almer <jamrial at gmail.com> wrote:
>>
>> This reverts commit f631c328e680a3dd491936b92f69970c20cdcfc7.
>>
>> The avcodec_parameters_to_context() call was freeing and reallocating
>> AVCodecContext->extradata, essentially taking ownership of it, which according
>> to the doxy is user owned. This is an API break and has produces crashes in
>> some library users like Firefox[1].
>> Revert until a better solution is found to internally propagate the filtered
>> extradata back into the decoder context.
>>
>> [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1486080
>>
> 
> This is not the only place where extradata is being
> free'ed/re-allocated by avcodec during decoding, which is why I
> recommended the documentation change when it came up.
> 
> At least this one place is one I know of, maybe there are more:
> http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavcodec/aacdec.c;h=d394700cdc857fa9386fc60ed2509fd869461f7f;hb=HEAD#l331
> 
> - Hendrik

Sounds like it should use an internal buffer in AACContext instead.

The addition to require av_malloc() does not change the implications of
what the doxy said before its addition, and in fact it only limits its
versatility. If the user is meant to set, allocate and free said
extradata, then that means they own it and could just set a pointer to
some buffer they are also using elsewhere or plan to use long after
closing the decoder, be it allocated by av_malloc() or otherwise.
Libavcodec can't just free it and allocate its own replacement
internally, as it would mean virtually taking ownership of it.

Hell, even avcodec_free_context() just frees it without checking if it's
a decoder first, which is probably why Firefox and other cases I've seen
do avcodec_close(avctx) followed by av_freep(&avctx) instead to prevent
it being freed.