Re: [pygame] PATCH: movie_set_display hangs on movie from file-like object

2007-05-01 Thread René Dudfield

Thanks Martin.

I've added the movie changes to subversion.

Committed revision 994.

I haven't tried out the pygame.h changes yet.



On 2/27/07, Martin [EMAIL PROTECTED] wrote:

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 

Re: [pygame] PATCH: movie_set_display hangs on movie from file-like object

2007-05-01 Thread Charles Joseph Christie II
On Tuesday 01 May 2007 07:17:08 pm René Dudfield wrote:

 Committed revision 994.


Working hard or hardly working? ;)

Well, it looks like you got a lot done today. 5 fixes in an hour. Nice.


Re: [pygame] PATCH: movie_set_display hangs on movie from file-like object

2007-05-01 Thread René Dudfield

Hardly working :)  We're just doing this once a week for a few hours
each time, so don't expect all that much to get done.  But as you say,
I think the last two mini sprints have been quite productive.

On 5/2/07, Charles Joseph Christie II [EMAIL PROTECTED] wrote:

On Tuesday 01 May 2007 07:17:08 pm René Dudfield wrote:

 Committed revision 994.


Working hard or hardly working? ;)

Well, it looks like you got a lot done today. 5 fixes in an hour. Nice.



Re: [pygame] PATCH: movie_set_display hangs on movie from file-like object

2007-03-03 Thread Lenard Lindstrom

Martin wrote:

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.




I applied the movie.c patch to pygame 1.8.0 and it fixes the lockup with 
StringIO.


--
Lenard Lindstrom
[EMAIL PROTECTED]