Reimar Döffinger <[email protected]> added the comment:

On Tue, Jul 20, 2010 at 06:19:18PM +0000, Mans Rullgard wrote:
> 
> Mans Rullgard <[email protected]> added the comment:
> 
> Reimar Döffinger <[email protected]> writes:
> 
> > Reimar Döffinger <[email protected]> added the comment:
> >
> > On Tue, Jul 20, 2010 at 06:12:22PM +0000, Ernst Albrecht Köstlin wrote:
> >> --- libavformat/ffmdec.c   2010-07-20 20:06:00.000000000 +0200
> >> +++ libavformat/ffmdec_patched.c   2010-07-20 20:07:02.000000000 +0200
> >> @@ -385,8 +385,10 @@
> >>          st = s->streams[i];
> >>          if (st) {
> >>              av_free(st);
> >> +            s->streams[i] = NULL;
> >>          }
> >
> > Shouldn't that whole block just be
> > av_freep(&s->streams[i]);
> > ?
> 
> Looks like it should.

On second though, that whole code seems like complete
nonsense to me.
First, FFmpeg should free the streams on error,
if it doesn't we probably have a lot of memleaks.
So that would make just removing all this a proper
solution.
However, then we'd have a memleak due to rc_eq not being freed.
Below, completely untested patch might actually be correct I think:
Index: libavformat/ffmdec.c
===================================================================
--- libavformat/ffmdec.c        (revision 24368)
+++ libavformat/ffmdec.c        (working copy)
@@ -251,6 +251,7 @@
     url_fseek(pb, ptr, SEEK_SET);
 }

+static int ffm_close(AVFormatContext *s);

 static int ffm_read_header(AVFormatContext *s, AVFormatParameters *ap)
 {
@@ -381,12 +382,7 @@
     ffm->first_packet = 1;
     return 0;
  fail:
-    for(i=0;i<s->nb_streams;i++) {
-        st = s->streams[i];
-        if (st) {
-            av_free(st);
-        }
-    }
+    ffm_close(s);
     return -1;
 }

________________________________________________
FFmpeg issue tracker <[email protected]>
<https://roundup.ffmpeg.org/issue2003>
________________________________________________

Reply via email to