Hello world,

I've solved a little problem a had using pygame 1.6 on Win32 Python 2.4.
I initialized a Movie with a file-like object (a cStringIO.StringIO
containing MPEG data), and my script hung when set_display was called on
that Movie.


Note that the current movie module documentation claims that movie won't
work on Win32 due to smpeg, and that pygame-1.7.1release.win32-py2.4
emphasizes on this by not containing movie(.pyd).
Using pygame 1.7.1 with SDL.dll from SDL 1.2.11 and the current sources
of SMPEG and pygame.movie, however, I managed to get it work, and I
think, even better than before. (I'll detail that later.)


Now, here's my fix for the function movie_set_display:

Index: movie.c
===================================================================
--- movie.c    (Revision 983)
+++ movie.c    (Working Copy)
@@ -148,32 +148,40 @@

        if(posobj == NULL)
        {
+            Py_BEGIN_ALLOW_THREADS
            SMPEG_Info info;
            SMPEG_getinfo(movie, &info);
            SMPEG_scaleXY(movie, info.width, info.height);
+            Py_END_ALLOW_THREADS
            x = y = 0;
        }
        else if(TwoIntsFromObj(posobj, &x, &y))
        {
+            Py_BEGIN_ALLOW_THREADS
            SMPEG_Info info;
            SMPEG_getinfo(movie, &info);
            SMPEG_scaleXY(movie, info.width, info.height);
+            Py_END_ALLOW_THREADS
        }
        else if((rect = GameRect_FromObject(posobj, &temp)))
        {
            x = rect->x;
            y = rect->y;
+            Py_BEGIN_ALLOW_THREADS
            SMPEG_scaleXY(movie, rect->w, rect->h);
+            Py_END_ALLOW_THREADS
        }
        else
            return RAISE(PyExc_TypeError, "Invalid position argument");

        surf = PySurface_AsSurface(surfobj);

+        Py_BEGIN_ALLOW_THREADS
            SMPEG_getinfo(movie, &info);
        SMPEG_enablevideo(movie, 1);
        SMPEG_setdisplay(movie, surf, NULL, NULL);
        SMPEG_move(movie, x, y);
+        Py_END_ALLOW_THREADS
    }
    else
    {


Reason: When the Movie object is constructed, it creates an SDL_RWops to
read the MPEG specified as its parameter. For file names and true file
objects, the implementation of SDL_RWops used is based on FILE* and
therefore is completely independent of Python.

Since a file-like object is Python data, of course Python's global
interpreter lock must be acquired to access it. The SDL_RWops
implementation used is found in rwobject.c of pygame. See also pygame.h
and movie.c (grep RWopsFromPythonThreaded).

When I called movie_set_display, it reached the call to
SMPEG_getinfo(movie, &info); (see above, near the end of the diff).
Since the call was not preceeded by Py_BEGIN_ALLOW_THREADS, the global
interpreter lock remained on the current thread.

SMPEG_getinfo called the seek function pointed to by the movie's
SDL_RWops, which was rw_seek_th from rwobject.c. rw_seek_th in turn
called PyEval_AcquireLock to protect its access to the file-like object.
PyEval_AcquireLock then deadlocked as documented (the lock had been
acquired twice by the same thread).


I think that my proposed solution is harmless, because
- the objects immediately accessed between the inserted
Py_{BEGIN,END}_ALLOW_THREADS are not Python data, and
- the same practice can already be observed on _any_ other call of an
SMPEG_* function in movie.c.


I would also like to suggest some minor changes here:

Index: pygame.h
===================================================================
--- pygame.h    (Revision 983)
+++ pygame.h    (Working Copy)
@@ -56,7 +56,8 @@
     **/
#include <Python.h>

-#ifdef MS_WIN32 /*Python gives us MS_WIN32, SDL needs just WIN32*/
+#if defined(MS_WIN32) && !defined(WIN32)
+/*Python gives us MS_WIN32, SDL needs just WIN32*/
#define WIN32
#endif

@@ -362,6 +363,8 @@
/*last platform compiler stuff*/
#if defined(macintosh) && defined(__MWERKS__)
#define PYGAME_EXPORT __declspec(export)
+#elif defined(MS_WIN32)
+#define PYGAME_EXPORT __declspec(dllexport)
#else
#define PYGAME_EXPORT
#endif


The first change fixes a warning I got about WIN32 being redefined.

The second one should be reviewed. I guess it might not work on every
compiler. I needed it to make pygame.movie and others compile in my
environment (MinGW and Dev-C++, and note that I did not use configure -
maybe that's why).


I promised to tell you what I found improved after using the new
versions of SDL and  SMPEG:
- With the versions from pygame 1.6, I had not been able to scale movies
freely. 1x and 2x worked, all else crashed. The new versions fix this.
- Also, movies had only been visible on fullscreen surfaces. With SDL
1.2.11, I can finally watch them in a window.
- SMPEG up to 0.4.3 muted some audio streams, others played fine. With
the current sources, the previously muted audio streams do play, however
they are stuttering. Not really an improvement, but a step towards a
fix, I think. And the other streams still play fine.

Note that set_volume(0) seems not to work with the new SMPEG (mind the
stuttering). But hey, my speakers have that knob...


I hope I've been helpful, and not too verbose.
Please consider re-adding movie.pyd on the next Win32 pygame release.

Bye!

Martin



Reply via email to