AMD64 patch submitted

2004-11-27
2013-04-03
  • christophe choquet

    The following patch makes gplflash plugin running with Firefox 1.0 compiled in 64 bits.
    Some weird random crashes still happens in NPP_DestroyStream().  Needs some more investigations, but should not be 64 bits related - perhaps NPTL or SMP related.
    The plugin is now useable on AMD64.

    Fix type size for AMD64

    --- npapi.h_ORIG    2004-11-26 07:17:23.361879416 +0000
    +++ npapi.h    2004-11-26 07:28:09.477654888 +0000
    @@ -36,7 +36,7 @@
    typedef unsigned short uint16;
    #endif
    #ifndef _UINT32
    -#if defined(__alpha)
    +#if defined(__alpha) || defined(__x86_64__)
    typedef unsigned int uint32;
    #else /* __alpha */
    typedef unsigned long uint32;
    @@ -46,7 +46,7 @@
    typedef short int16;
    #endif
    #ifndef _INT32
    -#if defined(__alpha)
    +#if defined(__alpha) || defined(__x86_64__)
    typedef int int32;
    #else /* __alpha */
    typedef long int32;

                    -----------------------------------------------------
    Fix type size for AMD64

    --- graphic32.cc_ORIG   2004-11-26 07:34:59.122379424 +0000
    +++ graphic32.cc        2004-11-26 07:35:36.089759528 +0000
    @@ -30,7 +30,11 @@

    #define PRINT 0

    +#if defined(__x86_64__)
    +typedef unsigned int TYPE;
    +#else
    typedef unsigned long TYPE;
    +#endif

    GraphicDevice32::GraphicDevice32(FlashDisplay *fd) : GraphicDevice(fd)
    {

                    -----------------------------------------------------
    Fix type size for AMD64

    --- swf.h_ORIG  2004-11-26 07:28:38.748205088 +0000
    +++ swf.h       2004-11-26 07:33:37.975715592 +0000
    @@ -18,6 +18,17 @@
    extern int debug;

    // Global Types
    +#if defined(__x86_64__)
    +typedef unsigned int U32, *P_U32, **PP_U32;
    +typedef signed int S32, *P_S32, **PP_S32;
    +typedef unsigned short U16, *P_U16, **PP_U16;
    +typedef signed short S16, *P_S16, **PP_S16;
    +typedef unsigned char U8, *P_U8, **PP_U8;
    +typedef signed char S8, *P_S8, **PP_S8;
    +typedef signed long SFIXED, *P_SFIXED;
    +typedef signed long SCOORD, *P_SCOORD;
    +typedef unsigned long BOOL;
    +#else
    typedef unsigned long U32, *P_U32, **PP_U32;
    typedef signed long S32, *P_S32, **PP_S32;
    typedef unsigned short U16, *P_U16, **PP_U16;
    @@ -27,6 +38,7 @@
    typedef signed long SFIXED, *P_SFIXED;
    typedef signed long SCOORD, *P_SCOORD;
    typedef unsigned long BOOL;
    +#endif

    #define ZOOM(v,f) ((v)/(f))

                    -----------------------------------------------------
    FIX NULL, when font is not defined: eg: www.emme.com

    --- text.cc_ORIG        2004-11-26 07:45:05.905134424 +0000
    +++ text.cc     2004-11-27 13:31:47.560005720 +0000
    @@ -91,6 +91,10 @@
                    }
            }

    +       // Fix code when Font is null, set a default font
    +       if (font == NULL) {
    +               font = new SwfFont(0);
    +       }
            if (tr->nbGlyphs) {
                    for(n=0; n < tr->nbGlyphs; n++) {
                            tr->glyphs[n].code = font->getGlyphCode(tr->glyphs[n].index);
    @@ -166,7 +170,9 @@
                    fmat.a = fontHeight/1000.0;
                    fmat.d = fontHeight/1000.0;

    -               assert(font != 0);
    +               //Fix case where font is not defined : eg www.emme.com
    +               if (font != NULL) { // TODO:
    +
                    for (g = 0; g < tr->nbGlyphs; g++)
                    {
                            Shape *shape;
    @@ -198,7 +204,7 @@
                    printf("\n");
    #endif
            }
    -
    +}
            if (gd->showMore) {
                    tmat = (*gd->adjust) * (*matrix);

                    -----------------------------------------------------

    Fix random crashes occuring on AMD64/SMP with Firefox 1.0 (64 bits). Still not 100% fixed. Help needed here to understand multithreading issues.
    The code seems to be not thread safe, and is responsible for free() exception and wrong pointers.
    For now, semaphore has been added to limit random crashes. Some allocated memory cannot be released, because it sometimes crashes (don't know why).

    --- plugin.c_ORIG    2004-11-26 08:44:31.722047856 +0000
    +++ plugin.c    2004-11-27 12:41:33.544205768 +0000
    @@ -26,6 +26,7 @@
    #include <X11/IntrinsicP.h>
    #include <X11/Core.h>
    #include <X11/CoreP.h>
    +#include <pthread.h>

    #define PRINT 0
    #define FLASH_XEVENT_MASK (ExposureMask|ButtonReleaseMask|ButtonPressMask|PointerMotionMask|KeyPressMask|KeyReleaseMask)
    @@ -70,6 +71,7 @@
    static long FlashGraphicInitX11(PluginInstance* This);
    static void FlashCopyX11(PluginInstance* This);

    +static pthread_mutex_t synchro;

    #ifdef C6R5
    // Ugly but it makes it work if it is compiled on
    @@ -175,19 +177,18 @@
    NPP_Destroy(NPP instance, NPSavedData** save)
    {
         PluginInstance* This;
    -
         if (instance == NULL)
             return NPERR_INVALID_INSTANCE_ERROR;

         This = (PluginInstance*) instance->pdata;

         if (This != NULL) {
    -        LoadCtxt *l,*prev;
    -
    +        pthread_mutex_lock(&synchro);
             if (This->timer) {
                 XtRemoveTimeOut(This->timer);
                 This->timer = 0;
             }
    +
             if (This->fh) {
                 XShmDetach(This->dpy, &This->segInfo);
                 XSync(This->dpy,False);
    @@ -198,26 +199,10 @@
                 This->fh = 0;
             }

    -/*    If this line is included mozilla crashes 50%-75% of the time when leaving a page
    -    with flash-animation, probably because mozilla already has removed the handler.
    -    Somebody with actual knowledge about this should try to make a real fix.
    -
    -
    -        XtRemoveRawEventHandler(This->widget, FLASH_XEVENT_MASK,
    -                  True, (XtEventHandler) flashEvent, (XtPointer)This);
    -*/
    -
    -        prev = 0;
    -        for(l = This->loading; l; prev = l, l = l->next) {
    -            free(l->data);
    -            free(l->url);
    -            free(prev);
    -        }
    -        free(prev);
    -
    -        NPN_MemFree(instance->pdata);
    -        instance->pdata = NULL;
    -    }
    +            NPN_MemFree(instance->pdata);
    +            instance->pdata = NULL;
    +        pthread_mutex_unlock(&synchro);
    +      }   

         return NPERR_NO_ERROR;
    }
    @@ -272,6 +257,7 @@
    static void
    flashWakeUp(XtPointer client_data, XtIntervalId *id)
    {
    +    pthread_mutex_lock(&synchro);
         PluginInstance* This;
         long cmd;
         long wakeUp;
    @@ -295,6 +281,7 @@
                 This->timer = 0;
             }
         }
    +    pthread_mutex_unlock(&synchro);
    }

    NPError
    @@ -381,7 +368,6 @@
             LoadCtxt *l;

             This = (PluginInstance*) instance->pdata;
    -
             for(l = This->loading; l != 0; l = l->next) {
                 if (l->url && strstr(stream->url, l->url)) {
                     break;
    @@ -395,7 +381,6 @@
             } else {
                 l->data = (char *) realloc(l->data, l->size+len);
             }
    -
             memcpy(&l->data[offset], buffer, len);
             l->size += len;
             status = FlashParse(This->fh, l->level, l->data, l->size);
    @@ -446,7 +431,8 @@
         if (This == 0) return;

         l = (LoadCtxt *)malloc(sizeof(LoadCtxt));
    -    l->next = This->loading;
    +    //????? Cause memory area to be deallocated multiple times. needed?: l->next = This->loading;
    +    l->next = 0;
         This->loading = l;
         l->level = level;
         l->data = 0;
    @@ -495,20 +481,24 @@

         if (This == 0) return NPERR_INVALID_INSTANCE_ERROR;

    -    if (This->fh == 0) {
    -        return NPERR_INVALID_INSTANCE_ERROR;
    -    }
    -
    +    //if (This->fh == 0) {
    +    //    return NPERR_INVALID_INSTANCE_ERROR;
    +    //}
    +    if ((NPP)This->loading == (NPP)instance ) return NPERR_NO_ERROR; // Nothing to deallocate
         for(l = This->loading; l; l = l->next) {
             if (l->url && strstr(stream->url, l->url)) {
    -            free(l->data);
    +            // Troubles here! Sometimes seems to be already freed (firefox).
    +            // Someone knows what's happening here?
    +            //free(l->data);    // Leak!
                 l->data = 0;
    -            free(l->url);
    +            //free(l->url);        // Leak!
                 l->url = 0;
                 break;
             }
         }

    +    // Forbidden: cannot use any X11 commands since context may be invalid
    +#if 0
         if (!This->gInitDone && This->dpy) {
             FlashGraphicInitX11(This);
             XtAddEventHandler(This->widget, FLASH_XEVENT_MASK,
    @@ -517,6 +507,7 @@

             flashWakeUp((XtPointer)This, 0);
         }
    +#endif

         return NPERR_NO_ERROR;
    }

     
    • Tomas Groth

      Tomas Groth - 2004-11-28

      Hi!

      Thanks for making the patch! I'll apply/test it when i get some time, which will hopefully be within the next few days.
      I have a question to the plugin changes, you write it fixes some random crashes on amd64/smp machines, but i assume this will help other archs and non-smp machines...
      BTW, i don't really know the plugin code, so i'm not cabable of helping you out, but thanks anyway, and if you make other fixes please let me know :)

      Tomas

       
    • Tomas Groth

      Tomas Groth - 2004-11-28

      Hi again

      Could I get you to post the patches in the patch-tracker, since they a nightmare to extract from these html-forums, and i'll probably miss something if i try :(
      Also feel free to mail them :)

      Tomas

       
    • christophe choquet

      Hi Tomas,

      I promise i'll try to use the patch tracker next time!.
      Concerning the crashes, I still have some ( around one every 50 pages containing flash images on average).
      They are happening when leaving a page. Using the MALLOC_CHECK_=3 before calling gdb helps.

      If you also have crashes on 32 bits arch and firefox, we may have a look at threading issues. I think firefox spawn threads to get the page rendered faster. This flash implementation is now not thread safe. We probably need some more semaphore during frame initializations. When a flash frame is built, it looks ok (but I still may be wrong)

      Christophe

       
    • christophe choquet

      Just some more things:

      - just found an interresting example of gnome mozilla-bonobo-plugin.c that uses threads (like our flashEvents),

      - before releasing a new version, I think:
        * we need to check if /dev/dsp is in use. I can't browse while xmms playing.

      * init a default SwfFont when none is specified in flash file (or non recognized in new flash format versions).

      I'll spend some time this week on that.
      Christophe

       
    • Tomas Groth

      Tomas Groth - 2004-11-29

      Hi Christophe

      Crashes with firefox and gplflash on 32 bit archs is happening all the time, though i haven't tried your semaphore fix yet. So it may very well be a threading issue.

      About the /dev/dsp being in use, i think it should be fairly easy to make gplflash just throw out the audio if it can't be delivered. I'll look into it when i get some time.

      I don't know much about the font thing (yet)...

      BTW, would you mind re-appling theese patches into the patch-tracker as well?

      Tomas

       
    • christophe choquet

      Hi Tomas,

      I have posted the patch.
      For sound blocking issue, we just need to add | O_NONBLOCK when opening /dev/dsp (sound.cc line 230).

      For undefined font, it look more complex than I initially thought, because the bitmap definition looks to be inside the flash file. It happens for flash version > 4. Could be included in a future work to support new flash versions.
      For now, the null pointer is checked and included in the patch.
      Christophe

       
    • Tomas Groth

      Tomas Groth - 2004-12-05

      Hi Christophe,

      I had a quick look at the patch, and i'm not sure the changes for amd64 is needed, since there already has been made some changes to the typedef in CVS, so if you could test the CVS version that would be great!
      I should get time to have a closer look at the patch (and apply it) in a few days, and perhaps even make a bugfix-release :)
      BTW, i tried to mail you, but i'm not sure it got through...

      Tomas

       
    • christophe choquet

      Hi Tomas,

      With the CVS source files, I got an immediate crash when the plugin is loading:
      Program received signal SIGSEGV, Segmentation fault.
      [Switching to Thread 182942293632 (LWP 18402)]
      NPP_SetWindow (instance=0x111f6a8, window=0xdfd1c8) at plugin.c:318
      318             This->dpy = ws->display;
      (gdb) bt
      #0  NPP_SetWindow (instance=0x111f6a8, window=0xdfd1c8) at plugin.c:318
      #1  0x0000002a9e50340e in Private_SetWindow (instance=0x111f6a8, window=0xdfd1c8) at npunix.c:194
      #2  0x0000002a9cae537b in ns4xPluginInstance::InitializePlugin () from /usr/lib/MozillaFirefox/components/libgkplugin.so

      Christophe

       
    • Tomas Groth

      Tomas Groth - 2004-12-06

      Hi Christophe,
      Well, don't test using the browser plugin, it's known to be ustable, why do you think i like your patches so much :)
      Instead use the player, or apply your tread-patch to the plugin. Remember i haven't commited any of your changes yet. But i will - today or tomorrow.

      Tomas

       

Log in to post a comment.

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:

JavaScript is required for this form.





No, thanks