[FFmpeg-devel] [PATCH 1/2] avcodec: remove AVCodecContext->metadata

[Clément Bœsch]

On Wed, Mar 13, 2013 at 05:51:06PM +0100, Hendrik Leppkes wrote:
> This field was only ever set and freed from avcodec, and not otherwise
> used. However, because frames are refcounted now, avcodec cannot make any
> assumptions about the lifetime of the frame metadata, which can result in
> double-frees or leaked memory.
> ---
>  libavcodec/avcodec.h | 7 -------
>  libavcodec/utils.c   | 3 ---
>  2 files changed, 10 deletions(-)
> 
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 7145a46..a46a8d6 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -2793,13 +2793,6 @@ typedef struct AVCodecContext {
>      int64_t pts_correction_last_dts;       /// DTS of the last frame
>  
>      /**
> -     * Current frame metadata.
> -     * - decoding: maintained and used by libavcodec, not intended to be used by user apps
> -     * - encoding: unused
> -     */
> -    AVDictionary *metadata;
> -
> -    /**
>       * Character encoding of the input subtitles file.
>       * - decoding: set by user
>       * - encoding: unused
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index 4ed64c9..6cd8026 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -1846,7 +1846,6 @@ static int add_metadata_from_side_data(AVCodecContext *avctx, AVFrame *frame)
>      const uint8_t *side_metadata;
>      const uint8_t *end;
>  
> -    av_dict_free(&avctx->metadata);
>      side_metadata = av_packet_get_side_data(avctx->pkt,
>                                              AV_PKT_DATA_STRINGS_METADATA, &size);
>      if (!side_metadata)
> @@ -1861,7 +1860,6 @@ static int add_metadata_from_side_data(AVCodecContext *avctx, AVFrame *frame)
>          side_metadata = val + strlen(val) + 1;
>      }
>  end:
> -    avctx->metadata = av_frame_get_metadata(frame);
>      return ret;
>  }

I think that whole code might not be necessary anymore: its purpose was to
make a gate between the lavfi device and the AVFrame, see 6fb2fd89.

My guess is that this glue code is not necessary anymore. And the fact
that the scene detect metadata test passes while
add_metadata_from_side_data() call isn't present in the decode video
function kind of confirms this. It shouldn't be necessary for audio where
you may be able to drop it as well.

>  
> @@ -2282,7 +2280,6 @@ av_cold int avcodec_close(AVCodecContext *avctx)
>              av_buffer_pool_uninit(&pool->pools[i]);
>          av_freep(&avctx->internal->pool);
>          av_freep(&avctx->internal);
> -        av_dict_free(&avctx->metadata);
>      }
>  
>      if (avctx->priv_data && avctx->codec && avctx->codec->priv_class)

Otherwise LGTM

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130313/5661af52/attachment.asc>