[FFmpeg-devel] [PATCH] fix memleak in ogg demuxer

[Måns Rullgård]

Reimar D?ffinger <Reimar.Doeffinger at gmx.de> writes:

> Hello,
> the sample in http://bugzilla.mplayerhq.hu/show_bug.cgi?id=1148 cause a
> memleak in the ogg demuxer.
> That is because oggparsevorbis caches a few packets, and if we close
> the demuxer before reaching the end of the header packets they will
> never be freed.
> Patch below that adds and uses a close function for the codec-specific
> ogg header parser fixes it.
> I have not check the others whether they have a similar issue.
> Index: ffmpeg/libavformat/oggdec.c
> ===================================================================
> --- ffmpeg/libavformat/oggdec.c (revision 25928)
> +++ ffmpeg/libavformat/oggdec.c (working copy)
> @@ -588,6 +588,8 @@
>
>      for (i = 0; i < ogg->nstreams; i++){
>          av_free (ogg->streams[i].buf);
> +        if (ogg->streams[i].codec && ogg->streams[i].codec->close)
> +            ogg->streams[i].codec->close(s, i);

Why not pass the priv pointer directly here,

>          av_free (ogg->streams[i].private);
>      }
>      av_free (ogg->streams);
> Index: ffmpeg/libavformat/oggdec.h
> ===================================================================
> --- ffmpeg/libavformat/oggdec.h (revision 25928)
> +++ ffmpeg/libavformat/oggdec.h (working copy)
> @@ -47,6 +47,10 @@
>       */
>      uint64_t (*gptopts)(AVFormatContext *, int, uint64_t, int64_t *dts);
>      /**
> +     * Close the codec, freeing any internally allocated data if necessary.
> +     */
> +    void (*close)(AVFormatContext *, int);
> +    /**
>       * 1 if granule is the start time of the associated packet.
>       * 0 if granule is the end time of the associated packet.
>       */
> Index: ffmpeg/libavformat/oggparsevorbis.c
> ===================================================================
> --- ffmpeg/libavformat/oggparsevorbis.c (revision 25928)
> +++ ffmpeg/libavformat/oggparsevorbis.c (working copy)
> @@ -162,6 +162,16 @@
>      unsigned char *packet[3];
>  };
>
> +static void vorbis_close(AVFormatContext *s, int idx)
> +{
> +    int i;
> +    struct ogg *ogg = s->priv_data;
> +    struct oggvorbis_private *priv = ogg->streams[idx].private;

and save some work here?

> +    if (!priv)
> +        return;
> +    for (i = 0; i < 3; i++)
> +        av_freep(&priv->packet[i]);
> +}
>
>  static unsigned int
>  fixup_vorbis_headers(AVFormatContext * as, struct oggvorbis_private *priv,
> @@ -265,5 +275,6 @@
>  const struct ogg_codec ff_vorbis_codec = {
>      .magic = "\001vorbis",
>      .magicsize = 7,
> -    .header = vorbis_header
> +    .header = vorbis_header,
> +    .close = vorbis_close,
>  };
>

-- 
M?ns Rullg?rd
mans at mansr.com