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

[Michael Niedermayer]

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

patch applied

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130313/e6cb54f7/attachment.asc>