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

[Clément Bœsch]

On Wed, Mar 13, 2013 at 06:15:57PM +0100, Clément Bœsch wrote:
> 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.
> 

Please disregard this non-sense. Patch LGTM if it passes fate.

-- 
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/f1195668/attachment.asc>