[FFmpeg-devel] [PATCH] avcodec/avcodec: don't uninitialize ch_layout in avcodec_close()

[Andreas Rheinhardt]

James Almer:
> The function is not meant to clear codec parameters, and the lavf demux code
> relies on this behavior.
> Regression since 327efa66331ebdc0087c6b656059a8df2f404019.
> 
> Signed-off-by: James Almer <jamrial at gmail.com>
> ---
>  libavcodec/avcodec.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
> index 38bdaad4fa..253c9f56cc 100644
> --- a/libavcodec/avcodec.c
> +++ b/libavcodec/avcodec.c
> @@ -469,6 +469,7 @@ void avsubtitle_free(AVSubtitle *sub)
>  
>  av_cold int avcodec_close(AVCodecContext *avctx)
>  {
> +    AVChannelLayout ch_layout;
>      int i;
>  
>      if (!avctx)
> @@ -524,7 +525,12 @@ av_cold int avcodec_close(AVCodecContext *avctx)
>  
>      if (avctx->priv_data && avctx->codec && avctx->codec->priv_class)
>          av_opt_free(avctx->priv_data);
> +    /* av_opt_free() will uninitialize avctx->ch_layout, but we want to keep it.
> +       It will be uninitialized in avcodec_free_context() */
> +    ch_layout = avctx->ch_layout;
> +    memset(&avctx->ch_layout, 0, sizeof(avctx->ch_layout));
>      av_opt_free(avctx);
> +    avctx->ch_layout = ch_layout;
>      av_freep(&avctx->priv_data);
>      if (av_codec_is_encoder(avctx->codec)) {
>          av_freep(&avctx->extradata);

avcodec_close() has always* called av_opt_free() and therefore
uninitialized allocated options (the documentation of avcodec_close()
states that it "frees all the data associated with it"); the new channel
layout API is (potentially) based on allocations, so ch_layout belongs
to this group of elements.
If the demux code wants to preserve ch_layout, then the demux code
should do it itself; it should not life in avcodec_close() (where it
would be a hack).

- Andreas

*: or at least for a very long time -- I didn't do archeology for this
statement.