On Nov 1, 2011, at 4:58 PM, Carsten Neumann wrote: > Hello Patrick, > > On 11/01/2011 02:28 PM, Patrick Hartling wrote: >> I have been tracking down some memory corruption crashes related to >> loading animated GIFs with OpenSG 2, and I think I have narrowed down at >> least one cause. The GIF I have been using to cause a crash in the GIF >> loader itself has two frames of different dimensions where the first is >> smaller than the second. Valgrind tells me that there is an invalid >> write to the memory returned by OSG::Image::editData(), and that leads >> me to believe that the buffer used by OSG::Image is the wrong size. > > it's quite possible, I didn't know it's valid to have animated GIFs with > different frame sizes and it's likely that case was never tested before. > >> My current fix for this is to modify the GIF loader so that the width >> and height passed to OSG::Image::set() come from the GIFStream object >> rather than from the GIFData object for the first frame. This seems to >> work because the image reports its width and height as being that of the >> largest frame. The width and height for the current GIFData still need >> to be used for reading the bytes for that frame, and I think I am doing >> that correctly. > > I'm not up to speed on the GIF spec or the loader, but your description > sounds absolutely reasonable to me.
I cannot claim to be terribly knowledgeable about the GIF spec. We just encountered these crashes, and it occurs to me now that we may have never had customers using GIFs prior to this. As for this usage of animated GIFs, what I can say is that the GIMP can load and save GIF images with varying frame sizes, and every viewer I have tried renders the animations the way that I would expect. To put it another way, I haven't read the GIF89a spec, but I gather that this use case is valid--but possibly unusual. >> Does this sound like a valid approach? I don't know if a patch would be >> helpful since we're using a version of OpenSG that is about 6 months out >> of date. I can submit one if it would help clarify what I have >> described. If nothing else, I have attached a GIF that demonstrates the >> crash in the GIF loader. > > I just ran a git blame on the GIF loader and with the exception of some > cosmetic changes the code was last touched in 2006 or before, so if you > have a patch it is likely to apply with no problems and I'd be very > happy to apply it. Thanks! I have attached the patch. I am not 100% certain that the pointer math that accounts for the X offsets of each line in the image data is correct. The images *look* correct when rendered by OpenSG. More importantly, valgrind reports no invalid writes, and that ended up being my real benchmark. The use of const pointers is incidental to accounting for the differing frame sizes. I added that mainly to have the compiler help me narrow down potential places where memory blocks were being modified. -Patrick -- Patrick L. Hartling Senior Software Engineer, Priority 5 http://www.priority5.com/ The information transmitted in this communication is intended only for the person or entity to which it is addressed and contains proprietary material. Any review, retransmission, dissemination or other use of, or taking of any action in reliance upon, this information by persons or entities other than the intended recipient is prohibited. If you received this in error, please destroy any copies, contact the sender and delete the material from any computer.
anim_gif.patch
Description: Binary data
------------------------------------------------------------------------------ RSA® Conference 2012 Save $700 by Nov 18 Register now! http://p.sf.net/sfu/rsa-sfdev2dev1
_______________________________________________ Opensg-users mailing list Opensg-users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensg-users
