[vlc-devel] [PATCH] libgoom_plugin.dll mem leak patch

[xxcv]

Michael McEll wrote:
> On MS Windows libgoom_plugin.dll, when enabled, generates a 2.5meg 
> memleak when a track playback ends. Problem is visible on VLC 0.9.x 
> right through to current git master.
>
> Here is a patch for vlc/modules/visualization/goom.c which works 
> around the the failings of libgoom2k4 ability to free its resources.
> Instead of creating a new instance of libgoom2k4 on each track start, 
> with this patch a single instance will be used throughout.
Hi,
Is your patch tested ? I'm reading your patch but I see non-standard 
coding style according to VLC and you have made the p_plugin_info a 
global variable and a dllmain function is that really necessary no other 
workaround ? There was already memory leak fixed in goom2k4 library by 
Remi Duraffort and the fact you noticed this memory leak is it really 
coming from libgoom_plugin.dll?
I can see that your patch hacks specifically for win32 platform but it 
is a vlc module, is there possibly a better solution than this for all 
platforms if the memory leak does exist.
 From the user point of view the memory leak probably should be 
neglected unless you don't ever shutdown vlc for that reason it will 
probably run out of memory in a few hours or days ?
Here I tested with vlc-1.1.0-git without your patch while playing a mp3 
file I can see the memory usage is increasing from 23.4MB initially 
without goom enabled it increase slowly all the way to the end of song 
by 3MB which is exactly the size of the mp3 file... Then I enable goom 
the memory usage went from  23.4MB to 47.1MB and upon pressing the stop 
button it drops to 28.868MB which equals approximately 5MB of memory 
leak by enable goom then stop playback, if I press play again goom is 
automatically loaded memory usage jumps to 44MB and stop drops it back 
down to 30.464MB which would probably be close to your result. If I 
repeat this again it jumps to 48.5MB then drop back down to 31.544MB.
Now I test your patch it jumps from 23.6MB to 42MB by enable goom on 
press stop fall back to 32.228MB, more memory leak introduced 8.628MB 
leak? Press play it loads goom and goes to 46MB stop drop it down to 
33.664MB repeat procedure again 43.4MB to 32.7MB (repeat once more to 
verify results 47MB to 34.452MB).
All this test is done with a vlc-1.1.0-git build by myself I chose 
gcc-4.4.0 as compiler, so the result may vary.
So in conclusion, I must say the results are strange... and the memory 
leak is always there, if you follow my procedure for tests should have a 
similar result.

>
>
> --- goom.c    2009-06-10 17:21:22.000000000 +0100
> +++ mod_goom.c    2009-06-10 18:07:11.796875000 +0100
> @@ -90,6 +90,10 @@
>   
> *****************************************************************************/
>  #define MAX_BLOCKS 100
>  #define GOOM_DELAY 400000
> +
> +#ifdef WIN32
> +static PluginInfo *p_plugin_info;
> +#endif
>  
>  typedef struct
>  {
> @@ -326,7 +330,9 @@
>      audio_date_t i_pts;
>      int16_t p_data[2][512];
>      int i_data = 0, i_count = 0;
> -    PluginInfo *p_plugin_info;
> +#ifndef WIN32
> +    PluginInfo *p_plugin_info;
> +#endif
>      int canc = vlc_savecancel ();
>  
>      width = var_GetInteger( p_this, "goom-width" );
> @@ -335,8 +341,12 @@
>      speed = var_CreateGetInteger( p_thread, "goom-speed" );
>      speed = MAX_SPEED - speed;
>      if( speed < 0 ) speed = 0;
> -
> -    p_plugin_info = goom_init( width, height );
> +
> +#ifdef WIN32
> +    goom_set_resolution(p_plugin_info, width, height);
> +#else
> +    p_plugin_info = goom_init( width, height );
> +#endif
>  
>      while( vlc_object_alive (p_thread) )
>      {
> @@ -375,8 +385,9 @@
>          p_pic->date = aout_DateGet( &i_pts ) + GOOM_DELAY;
>          vout_DisplayPicture( p_thread->p_vout, p_pic );
>      }
> -
> +#ifndef WIN32
>      goom_close( p_plugin_info );
> +#endif
>      vlc_restorecancel (canc);
>      return NULL;
>  }
> @@ -448,3 +459,23 @@
>  
>      return psz_title;
>  }
> +
> +#ifdef WIN32
> +BOOL WINAPI DllMain (HINSTANCE hinstDLL, DWORD fdwReason, LPVOID 
> lpvReserved);
> +
> +BOOL WINAPI DllMain (HINSTANCE hinstDLL, DWORD fdwReason, LPVOID 
> lpvReserved)
> +{
> +    (void)hinstDLL;
> +    (void)lpvReserved;
> +   
> +    if ( fdwReason == DLL_PROCESS_ATTACH )
> +    {
> +        p_plugin_info = goom_init(320, 240);

> +    }
> +    else if ( fdwReason == DLL_PROCESS_DETACH )
> +    {
> +        goom_close(p_plugin_info);
> +    }
> +    return 1;
> +}
> +#endif
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20090612/395a503c/attachment.html>