[gstreamer-bugs] [Bug 385311] New: Memory leak initializing lame mp3 encoder plugin (gstlame.c)

[GStreamer (bugzilla.gnome.org)]

Do not reply to this via email (we are currently unable to handle email
responses and they get discarded).  You can add comments to this bug at
http://bugzilla.gnome.org/show_bug.cgi?id=385311

  GStreamer | gst-plugins-ugly | Ver: HEAD CVS

           Summary: Memory leak initializing lame mp3 encoder plugin
                    (gstlame.c)
           Product: GStreamer
           Version: HEAD CVS
          Platform: Other
        OS/Version: All
            Status: UNCONFIRMED
          Severity: minor
          Priority: Normal
         Component: gst-plugins-ugly
        AssignedTo: gstreamer-bugs at lists.sourceforge.net
        ReportedBy: roland.kay at ox.compsoc.net
         QAContact: gstreamer-bugs at lists.sourceforge.net
     GNOME version: 2.11/2.12
   GNOME milestone: Unspecified


Please describe the problem:
gst_lame_init() creates an encoder context so that it can retrieve default
encoding parameters from LAME. However, it does not call lame_global_flags(),
which is required according to the LAME API docs, see below.

The effect of this is that LAME does not set the ClassID field of the global
flags internal flags structure. lame_close() then presumes that the structure
is invalid and does not free it; resulting in the memory leak below.


Docs for lame_global_flags() from lame.h:

/*
 * REQUIRED:
 * initialize the encoder.  sets default for all encoder parameters,
 * returns NULL if some malloc()'s failed
 * otherwise returns pointer to structure needed for all future
 * API calls.
 */
lame_global_flags * CDECL lame_init(void);
/* obsolete version */
int CDECL lame_init_old(lame_global_flags *);

lame_close() definition:

int lame_close(lame_global_flags * gfp)
{     lame_internal_flags *gfc = gfp->internal_flags;

    if (gfc->Class_ID != LAME_ID)     <--- THIS EVALUATES TO FALSE.
        return -3;

    if (gfp->exp_nspsytune2.pointer[0]) {
      fclose((FILE *)gfp->exp_nspsytune2.pointer[0]);
      gfp->exp_nspsytune2.pointer[0] = NULL;
    }

    gfc->Class_ID = 0;

    /* this routien will free all malloc'd data in gfc, and then free gfc: */
    freegfc(gfc);                     <--- SO THIS NEVER GETS CALLED.

    gfp->internal_flags = NULL;

    if (gfp->lame_allocated_gfp) {
        gfp->lame_allocated_gfp = 0;
        free(gfp);
    }

    return 0;
}


Steps to reproduce:
1. Play http://samples.mplayerhq.hu/real/AC-sipr/autahi-vox.rm in any app that
uses GStreamer under valgrind.


Actual results:
==26636== 148032 (516 direct, 147516 indirect) bytes in 1 blocks are definitely
lost in loss record 22 of 23
==26636==    at 0x1B900B88: calloc (in /usr/lib/valgrind/vgpreload_memcheck.so)
==26636==    by 0x1C1CE2F3: lame_init (in
/usr/local/pkgs/lame-3.97/lib/libmp3lame.so.0.0.0)
==26636==    by 0x1BF8E446: gst_lame_init (gstlame.c:522)
==26636==    by 0x1B9CC9E5: g_type_create_instance (in
/opt/gnome/lib/libgobject-2.0.so.0.800.1)
==26636==    by 0x1B9B1E51: (within /opt/gnome/lib/libgobject-2.0.so.0.800.1)
==26636==    by 0x1B9B2ADE: g_object_newv (in
/opt/gnome/lib/libgobject-2.0.so.0.800.1)
==26636==    by 0x1B9B3696: g_object_new_valist (in
/opt/gnome/lib/libgobject-2.0.so.0.800.1)
==26636==    by 0x1B9B384F: g_object_new (in
/opt/gnome/lib/libgobject-2.0.so.0.800.1)
==26636==    by 0x1B92FB52: gst_element_factory_create
(gstelementfactory.c:381)
==26636==    by 0x1B92FDA5: gst_element_factory_make (gstelementfactory.c:444)
==26636==    by 0x804A02C: make_xc_pipeline_mp3 (gstxc.c:582)
==26636==    by 0x804A2FF: mp3_perform (gstxc.c:682)
==26636==    by 0x804993B: gstxc_perform (gstxc.c:329)
==26636==    by 0x80494BF: main (xcfile.c:42)


Expected results:


Does this happen every time?


Other information:
One could take this up with the LAME developers. However, we are violating
their API docs at the moment and the fix is trivial; see below.


-- 
Configure bugmail: http://bugzilla.gnome.org/userprefs.cgi?tab=email