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 <jamr...@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. > > lu > > _______________________________________________ > libav-devel mailing list > libav-devel@libav.org > https://lists.libav.org/mailman/listinfo/libav-devel > _______________________________________________ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
