Vorbis dev - Sep 2000 - Possible memory leak / xmms input plugin

Hi list,

It seems that when playing several OggVorbis files under xmms, there is a
memory leak. It's a rather annoying problem because when you have a
playlist of 100+ files, xmms will at the end eat something like 100M or
more..

Such a memory leak have been reported inside xmms-1.2.2 code and I could
see it playing Ogg files and mp3 files ; upgrading to 1.2.3 solves the
problem with mp3 files but still the leak playing ogg files is here. I
shortly tried to play the files with ogg123, and here it seems that there
is no memory leak, which is strange :-(.

Anyway, if anyone have information I should be glad to hear it. I'm going
to investigate but I have little knowledge of OggVorbis code..


-- 
Guillaume Cottenceau -- Distribution Developer for MandrakeSoft
http://www.mandrakesoft.com/~gc/

--- >8 ----
List archives:  http://www.xiph.org/archives/
Ogg project homepage: http://www.xiph.org/ogg/
To unsubscribe from this list, send a message to
'vorbis-dev-request@xiph.org'
containing only the word 'unsubscribe' in the body.  No subject is
needed.
Unsubscribe messages sent to the list will be ignored/filtered.

Guillaume Cottenceau <gc@mandrakesoft.com> writes:
> Hi list,
> 
> It seems that when playing several OggVorbis files under xmms, there is a
> memory leak. It's a rather annoying problem because when you have a
> playlist of 100+ files, xmms will at the end eat something like 100M or
> more..
[...]

Ok -- after some investigation, it seems that the memory leak comes
weirdly from some code in xmms, and not in OggVorbis code ;-).

In the meantime, I fixed a problem in the plugin: when /dev/dsp is not
available for any reason, it causes xmms to crash (required error code
given back to xmms in the plugin, is not handled).

I wrote a patch to current code for this, if someone with cvs access could
be interested you can tell me.


-- 
Guillaume Cottenceau -- Distribution Developer for MandrakeSoft
http://www.mandrakesoft.com/~gc/

--- >8 ----
List archives:  http://www.xiph.org/archives/
Ogg project homepage: http://www.xiph.org/ogg/
To unsubscribe from this list, send a message to
'vorbis-dev-request@xiph.org'
containing only the word 'unsubscribe' in the body.  No subject is
needed.
Unsubscribe messages sent to the list will be ignored/filtered.

>Ok -- after some investigation, it seems that the memory leak comes
>weirdly from some code in xmms, and not in OggVorbis code ;-).
Hmm.. That's a real pain. Do you think there's any way to work around
this
from the plugin side? 
>
>
>In the meantime, I fixed a problem in the plugin: when /dev/dsp is not
>available for any reason, it causes xmms to crash (required error code
>given back to xmms in the plugin, is not handled).
>
>I wrote a patch to current code for this, if someone with cvs access could
>be interested you can tell me.
Please, send the patch, I'll apply it (in general, any reasonably small
patches should just go straight to the list.)

Michael

--- >8 ----
List archives:  http://www.xiph.org/archives/
Ogg project homepage: http://www.xiph.org/ogg/
To unsubscribe from this list, send a message to
'vorbis-dev-request@xiph.org'
containing only the word 'unsubscribe' in the body.  No subject is
needed.
Unsubscribe messages sent to the list will be ignored/filtered.

Michael Smith <msmith@labyrinth.net.au> writes:
> >Ok -- after some investigation, it seems that the memory leak comes
> >weirdly from some code in xmms, and not in OggVorbis code ;-).
> 
> Hmm.. That's a real pain. Do you think there's any way to work
around this
> from the plugin side? 
>From the plugin side, I think not..
Strangely enough, the memory leak seems to not appear with a
Linux-Mandrake 7.1 (was out in June 2000), whereas it appears with the
upcomming distrib we're currently stabilizing (Linux-Mandrake 7.2). So it
maybe comes from gtk, or even worse from libc or something..
> >In the meantime, I fixed a problem in the plugin: when /dev/dsp is not
> >available for any reason, it causes xmms to crash (required error code
> >given back to xmms in the plugin, is not handled).
> >
> >I wrote a patch to current code for this, if someone with cvs access
could
> >be interested you can tell me.
> 
> Please, send the patch, I'll apply it (in general, any reasonably small
> patches should just go straight to the list.)
Ok, thanks, here it is:

--- ./xmms/vorbis.c.gege	Fri Sep 29 16:59:00 2000
+++ ./xmms/vorbis.c	Fri Sep 29 16:59:20 2000
@@ -31,6 +31,8 @@
 static void vorbis_get_song_info(char *filename, char **title, int *length);
 static gchar *vorbis_generate_title(vorbis_comment *comment, char *fn);
 
+static gboolean audio_error = FALSE;
+
 InputPlugin vorbis_ip  {
    NULL,
@@ -102,6 +104,7 @@
 static void
 *vorbis_play_loop(void *arg)
 {
+  gboolean output_opened = FALSE;
    char *filename = (char *)arg;
    char pcmout[4096];
    vorbis_comment *comment;
@@ -221,8 +224,16 @@
                              /* This resets the seek slider!  Thus we save the
timeoffset
                                 below to add back to the slider when we hit the
next link
                                 of a chain.*/
-			     vorbis_ip.output->close_audio();
-			     vorbis_ip.output->open_audio(FMT_S16_LE, vi->rate,
vi->channels);
+			     if (output_opened && !audio_error)
+			       vorbis_ip.output->close_audio();
+			     output_opened = TRUE;
+			     if (!vorbis_ip.output->open_audio(FMT_S16_LE, vi->rate,
vi->channels))
+			       {
+				 audio_error = TRUE;
+				 pthread_mutex_unlock(&vf_mutex);
+				 vorbis_eos = 1;
+				 goto play_cleanup;
+			       }
                              timeoffset=ov_time_tell(&vf);
 
                              /* set total play time, bitrate, rate, and
channels of current
@@ -273,7 +284,8 @@
              xmms_usleep(20000);
      }
    }
-   vorbis_ip.output->close_audio();
+   if (output_opened && !audio_error)
+     vorbis_ip.output->close_audio();
    /* fall through intentional */
 
  play_cleanup:
@@ -323,6 +335,8 @@
 static int
 vorbis_time(void)
 {
+  if (audio_error)
+    return -2;
   if(vorbis_eos && !vorbis_ip.output->buffer_playing())
           return -1;
   return timeoffset * 1000 + vorbis_ip.output->output_time();


-- 
Guillaume Cottenceau -- Distribution Developer for MandrakeSoft
http://www.mandrakesoft.com/~gc/

--- >8 ----
List archives:  http://www.xiph.org/archives/
Ogg project homepage: http://www.xiph.org/ogg/
To unsubscribe from this list, send a message to
'vorbis-dev-request@xiph.org'
containing only the word 'unsubscribe' in the body.  No subject is
needed.
Unsubscribe messages sent to the list will be ignored/filtered.

Guillaume Cottenceau <gc@mandrakesoft.com> writes:

[...]
> Strangely enough, the memory leak seems to not appear with a
> Linux-Mandrake 7.1 (was out in June 2000), whereas it appears with the
> upcomming distrib we're currently stabilizing (Linux-Mandrake 7.2). So
it
> maybe comes from gtk, or even worse from libc or something..
Ok, FYI I finally got it: it comes from a bug (in xmms code) in releasing
memory with gdk_font_unref on a GdkFont created by a gdk_fontset_load,
when running with XFree86-4 libs.

Actually it has nothing to do in particular with the oggvorbis plugin :-).


-- 
Guillaume Cottenceau -- Distribution Developer for MandrakeSoft
http://www.mandrakesoft.com/~gc/

--- >8 ----
List archives:  http://www.xiph.org/archives/
Ogg project homepage: http://www.xiph.org/ogg/
To unsubscribe from this list, send a message to
'vorbis-dev-request@xiph.org'
containing only the word 'unsubscribe' in the body.  No subject is
needed.
Unsubscribe messages sent to the list will be ignored/filtered.