[libav-devel] [PATCH] Revert "decode: copy the output parameters from the last bsf in the chain back to the AVCodecContext"
jamrial at gmail.com
Sat Sep 15 14:59:55 CEST 2018
On 9/13/2018 9:35 AM, Luca Barbato wrote:
> On 12/09/2018 20:24, James Almer wrote:
>> This reverts commit 662558f985f50834eebe82d6b6854c66f33ab320.
>> 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.
>> Revert until a better solution is found to internally propagate the filtered
>> extradata back into the decoder context.
> The doxy says:
> * - decoding: Set/allocated/freed by user.
Yes, meaning the user could just set this pointer to some buffer they
intend to reuse and free long after they closed and freed the
AVCodecContext. If libavcodec goes and tries to replace it, taking
ownership of it, then things can go very wrong.
For that matter, commit fd056029f45a9f6d213d9fce8165632042511d4f already
made that doxy obsolete by introducing avcodec_free_context() which
unconditionally frees extradata right after calling avcodec_close(), a
function that was doing it as well but only when it's an encoder.
The reason that commit didn't generate any widespread issues like this
one is because said users never migrated to avcodec_free_context() to
free the AVCodecContext.
Any suggestion on what to do? The above commit by Anton is four years
old. Do we make it official in the doxy that extradata is to be
allocated by the user (using av_malloc() functions) but then ownership
is passed to libavcodec after an avcodec_open2() call? It would require
to change how avcodec_close() frees the extradata as well.
> We could be more explicit on what you can do with it though.
>> Signed-off-by: James Almer <jamrial at gmail.com>
>> See https://bugzilla.mozilla.org/show_bug.cgi?id=1486080
>> Suggestions to work around it are very welcome, of course. While no bitstream
>> filter currently autoinserted by a decoder requires the filtered extradata to
>> be propagated to work properly, we don't know what may be needed in the future,
>> and without this the usability of bsfs in decoders is potentially limited.
> We already have AV_PKT_DATA_NEW_EXTRADATA to deal with extradata changes
> at the demuxer level, we might reuse it.
> I'm fine with the revert.
> libav-devel mailing list
> libav-devel at libav.org
More information about the libav-devel