GNOME Bugzilla – Bug 345679
fix to avoid goom core dumping
Last modified: 2006-07-11 22:29:02 UTC
I ntoice that on Solaris, goom core dumps. It dies in getPixelRGB_ on this line 299 c->b = *(unsigned char *) (tmp8 = (unsigned char *) (buffer + x)); Because x has an extremely large value. The call is happening here in the function zoomFilterFastRGB: 556 getPixelRGB_ (pix1, pos10[position], &col1, goomdata->resolx, 557 goomdata->resoly); Note that x in getPixelRGB_ corresponds to pos10[position]. Running in a debugger, I notice that zoomFilterFastRGB is not initializing the pos10 array the first time it is being called. The attached patch causes goom to always initialize the array the first time it is called (using a static boolean to keep track of the fact it has been called the first time). This seems to fix the goom core dumping problem on Solaris.
Created attachment 67859 [details] [review] patch to fix Solaris core dumping issue
Created attachment 67883 [details] [review] New patch Brian, can you try this patch? It uses a variable in the Goom data rather than a static variable, which preserves the reentrancy of the element and allows multiple instances (otherwise I think yours would crash when changing from goom to another visualisation and back again).
Created attachment 67885 [details] [review] clear memory before using it or this patch..
Thanks Wim - your patch works great - just tested it. Goom no longer core dumps. Probably a good thing to initialize the memory, so I hope this goes into the codebase. Although, I should say that goom isn't really functional on Solaris even with this patch. It seems that there are some serious performance issues. With goom turned on, it seems to cause the audio to stutter as if the CPU is being overloaded. As you might remember, the sunaudiosink hardcodes spec->segsize to 4096 and spec->segtotal to 128 in the prepare function, so we are using a fairly high buffer size. This was needed because with smaller buffer sizes, I was noticing that a similar stuttering problem was causing audio playback problems. Unfortunately, setting the buffer so large causes a number of performance issues (such as a several second lag when changing volume in programs like totem/rhythmbox and makes some buttons less responsive such as fast-forward and rewind or trying to scroll to a different time in the song). I notice that when goom is enabled the large buffer also seems to cause totem to pause for several seconds before playing the audio file, which makes it look like totem isn't doing anything. Also annoying. I did try setting the sunaudiosink buffer to a larger size to see if it would fix the stuttering problem with goom, but this didn't seem to help. To be honest, I'm not really sure how to attack this problem. Perhaps GStreamer needs some work to be performant on Solaris (since I notice, for example, Real doesn't seem to have these sort of problems - and we didn't have these problems with GStreamer 0.8). Perhaps thread priorities or something should be used to ensure that the goom plugin doesn't suck up so much processor time? I ran totem with goom playing an ogg file and I notice that 6.815% of user CPU time is spent in zoomFilterFastRGB and 4.933% in getPixelRGB_, both goom functions. I believe that Sun provided Fluendo with a few Sun machines so that you could provide the Fluendo plugins on the Solaris platform. I don't suppose you could take a look at this and see if you can identify why GStreamer performance seems to have these issues? If you would, that would be great. Thanks.
Wim, any reason not to commit your patch (or Jan's)? (btw, would calloc be faster?)
Using calloc as suggested by Tim: * gst/goom/filters.c: (zoomFilterSetResolution): Avoid goom coredumping by clearing memory. Fixes 345679.