avcodec_free_context frees extradata

[jyavenard]

According to the AVCodecContext specification:

AVCodecContext::extradata is "decoding: Set/allocated/freed by user. "

only for encoding is extradata managed by avcodec.

And avcodec_close does the right thing, and only free extradata field if the AVCodecContext instantiated an encoder.

However, avcodec_free_context always free extradata if set:

void avcodec_free_context(AVCodecContext **pavctx)
{
    AVCodecContext *avctx = *pavctx;

    if (!avctx)
        return;

    avcodec_close(avctx);

    av_freep(&avctx->extradata);
    av_freep(&avctx->subtitle_header);
    av_freep(&avctx->intra_matrix);
    av_freep(&avctx->inter_matrix);
    av_freep(&avctx->rc_override);

    av_freep(pavctx);
}

This is incorrect, it assumes that the ownership of extradata is by avcodec for either the decoder or the encoder.

Now the problem has been there for quite a while and it may be too late.

As such, either avcodec_free_context should be made obsolete and replaced with avcodec_free_context2 which would do the right thing.

Or the documentation needs to be updated, and the ownership of extradata now being by avcodec, this assumes that avcodec_close would also free extradata.

[Carl Eugen Hoyos]

Wouldn't the change you suggest (but unfortunately didn't send to the development mailing list) introduce a memleak?
Could you explain you usecase?

[Gerald Squelart]

Since the code has been there for a while, I think the best first move would be to update the documentation to at least match the current implementation, i.e.: 'extradata' (when decoding) is owned by the user *except* when using avcodec_free_context; the user should reset the pointer beforehand if they do not want their extradata to be destroyed.

But I agree with jyavenard that a replacement API avcodec_free_context2 that respects the external ownership of extradata would be more consistent with the documentation and other functions.

[Carl Eugen Hoyos]

Replying to squelart:

But I agree with jyavenard that a replacement API avcodec_free_context2 that respects the external ownership of extradata would be more consistent with the documentation and other functions.

Wouldn't this new function avcodec_free_context2() always lead to a memleak? What would be its usecase?

[jyavenard]

Why would it leak?

extradata was *never* supposed to have been freed automatically by avcodec when in decoder mode.
That it is now freed within avcodec_free_context *is* the bug.

This is per the extradata documentation.

The use case, is respecting the API as it's always been: that is the ownership of extradata is by the user, not avcodec.

Particular use case is that extradata is shared information.