James Almer:
> It's a user-set parameter shared with AVCodecParameters, so it should only
> be freed by avcodec_free_context().
>
> Signed-off-by: James Almer <jamr...@gmail.com>
> ---
> libavcodec/avcodec.c | 11 ++++++-----
> libavcodec/options.c | 4 ++++
> 2 files changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
> index fc8a40e4db..e560efff6a 100644
> --- a/libavcodec/avcodec.c
> +++ b/libavcodec/avcodec.c
> @@ -458,11 +458,12 @@ av_cold void ff_codec_close(AVCodecContext *avctx)
>
> av_freep(&avctx->internal);
> }
> -
> - for (i = 0; i < avctx->nb_coded_side_data; i++)
> - av_freep(&avctx->coded_side_data[i].data);
> - av_freep(&avctx->coded_side_data);
> - avctx->nb_coded_side_data = 0;
> + if (av_codec_is_encoder(avctx->codec)) {
> + for (i = 0; i < avctx->nb_coded_side_data; i++)
> + av_freep(&avctx->coded_side_data[i].data);
> + av_freep(&avctx->coded_side_data);
> + avctx->nb_coded_side_data = 0;
> + }
>
> av_buffer_unref(&avctx->hw_frames_ctx);
> av_buffer_unref(&avctx->hw_device_ctx);
> diff --git a/libavcodec/options.c b/libavcodec/options.c
> index 0c3b40a186..7c32a71275 100644
> --- a/libavcodec/options.c
> +++ b/libavcodec/options.c
> @@ -177,6 +177,10 @@ void avcodec_free_context(AVCodecContext **pavctx)
> av_freep(&avctx->inter_matrix);
> av_freep(&avctx->rc_override);
> av_channel_layout_uninit(&avctx->ch_layout);
> + for (int i = 0; i < avctx->nb_coded_side_data; i++)
> + av_freep(&avctx->coded_side_data[i].data);
> + av_freep(&avctx->coded_side_data);
> + avctx->nb_coded_side_data = 0;
> av_frame_side_data_free(
> &avctx->decoded_side_data, &avctx->nb_decoded_side_data);
>
1. ff_codec_close() already has an "if is_encoder" branch, it does not
need another one.
2. The code in ff_codec_close() will be redundant as soon as
FF_API_AVCODEC_CLOSE is no more, so it should be inside
FF_API_AVCODEC_CLOSE.
3. The documentation of this field does not mention ownership, but given
that this field existed for a long time lavc's previous behaviour
established the implicit contract that lavc will free this in
avcodec_close(). You are breaking this implicit contract for no good
reason apart from this new principle that user-set stuff shared with
AVCodecParameters should not be freed in avcodec_close(). Which is crazy
given that the relevant AVCodecParameters field has only been added
recently.
- Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
