Cool, thanks. Committed revision 1482.
On Wed, Jul 9, 2008 at 12:39 PM, Forrest Voight <[EMAIL PROTECTED]> wrote: > All we need to do is make RwopsFromPythonThreaded not try to make a > standard rwop, because it shouldn't. > > And it is correct, RwopsFromPythonThreaded is obviously meant to be > used like this (hence the threaded). > > Attached is the updated patch. > > Forrest > > On Tue, Jul 8, 2008 at 9:38 PM, René Dudfield <[EMAIL PROTECTED]> wrote: >> I guess we need to store a reference to the file object somewhere, and >> release the reference at cleanup. >> >> >> On Wed, Jul 9, 2008 at 9:40 AM, Brian Fisher <[EMAIL PROTECTED]> wrote: >>> Hmmm... from looking at the patch, it seems that it does not fix the crash >>> that Forrest discovered (where the file object falls out of scope and gets >>> deleted so the rwobject ends up having bad pointers) - Is that correct >>> Forrest? >>> >>> If that is the case, it seems to me that the feature implementation isn't >>> finished yet, so the patch as sent is not ready to be applied to pygame >>> 1.8.1. >>> >>> I would imagine that in most cases, people wouldn't be keeping around a >>> python reference to the file object they would pass in (cause it loads the >>> music up in some other function or something), which means most attempts to >>> use this feature would get crashes and bad behavior as the mixer tries to >>> stream the music, but the file object falls out of scope - and the crash >>> would happen at what seems like a random point in time. >>> >>> Also, as I said in an earlier email, I don't think this patch is exposing an >>> existing bug, I think it uses rwobject in a way that's not intended, as >>> other "load-from-file" pygame uses don't require the file-object to exist >>> longer than it takes for the load function to return. >>> >>> >>> >>> On Tue, Jul 8, 2008 at 3:20 PM, René Dudfield <[EMAIL PROTECTED]> wrote: >>>> >>>> Cool, thanks. I'll try and patch it tonight (+ 9 hours). >>>> >>>> cu, >>>> >>>> On Wed, Jul 9, 2008 at 1:09 AM, Forrest Voight <[EMAIL PROTECTED]> wrote: >>>> > I did the version checks. >>>> > >>>> > There are currently no tests for pygame.mixer.music, but I'll try to >>>> > make a test for this. >>>> > >>>> > Also, I found a bug in rwobject. It makes a standard SDL_RWops from >>>> > python file objects but doesn't hold a reference to them. >>>> > This is shown by doing something like: >>>> > >>>> > pygame.mixer.music.load(open('x.mp3')) >>>> > >>>> > Then playing it, and pygame crashes. >>>> > This is not my patch's fault, it just exposes it. >>>> > >>>> > Forrest Voight >>>> > >>>> > On Tue, Jul 8, 2008 at 2:08 AM, René Dudfield <[EMAIL PROTECTED]> wrote: >>>> >> hi, >>>> >> >>>> >> I think this will have to wait until we put the version checks in and >>>> >> have unittests... ie for pygame 1.9. Unless someone can get around to >>>> >> it in the next week. >>>> >> >>>> >> >>>> >> cu, >>>> >> >>>> >> >>>> >> On Wed, Jun 18, 2008 at 2:44 PM, Forrest Voight <[EMAIL PROTECTED]> >>>> >> wrote: >>>> >>> OK, I'll work on unit tests and a version check. >>>> >>> >>>> >>> On Mon, Jun 16, 2008 at 11:52 AM, Lenard Lindstrom <[EMAIL PROTECTED]> >>>> >>> wrote: >>>> >>>> Maybe the test could write a sine wave to a StringIO, load it, then >>>> >>>> use >>>> >>>> get_buffer (sound objects do have get_buffer now, right?*) to check >>>> >>>> it. >>>> >>>> >>>> >>>> Lenard >>>> >>>> >>>> >>>> * Sorry, I don't have access to latest Python/Pygame at the moment. >>>> >>>> >>>> >>>> Quoting René Dudfield <[EMAIL PROTECTED]>: >>>> >>>> >>>> >>>>> Hi, >>>> >>>>> >>>> >>>>> nice patch! This will be very useful :) >>>> >>>>> >>>> >>>>> Do you know which version of sdl_mixer allows rwops for music >>>> >>>>> (Mix_LoadMUS_RW)? Does it require an SDL_mixer version check? >>>> >>>>> >>>> >>>>> Are you able to make make any unit tests for using file likes with >>>> >>>>> the >>>> >>>>> music mixer? We're using unittests for all new code now, and it'd >>>> >>>>> make us feel more safe about adding it in for the 1.8.1 release. >>>> >>>>> >>>> >>>>> Not really sure how best to test it. I guess just loading the music >>>> >>>>> from different filename, and from a python file object would be ok >>>> >>>>> for >>>> >>>>> now. >>>> >>>>> >>>> >>>>> here's a start on a test for it... >>>> >>>>> >>>> >>>>> data_fname = os.path.join('..', 'examples', 'data') >>>> >>>>> #note, I just added house_lo.ogg to svn. >>>> >>>>> oggfn = os.path.join(data_fname, 'house_lo.ogg') >>>> >>>>> >>>> >>>>> pygame.mixer.music.load(oggfn) >>>> >>>>> pygame.mixer.music.load(open(oggfn)) >>>> >>>>> oggf = open(oggfn) >>>> >>>>> pygame.mixer.music.load(oggf) >>>> >>>>> >>>> >>>>> >>>> >>>>> >>>> >>>>> cheers, >>>> >>>>> >>>> >>>>> >>>> >>>>> >>>> >>>>> On Sat, Jun 14, 2008 at 9:53 AM, Forrest Voight <[EMAIL PROTECTED]> >>>> >>>>> wrote: >>>> >>>>> > Thanks! >>>> >>>>> > >>>> >>>>> > On Fri, Jun 13, 2008 at 7:31 PM, Lenard Lindstrom >>>> >>>>> > <[EMAIL PROTECTED]> wrote: >>>> >>>>> >> This is interesting. I am having a look at it. No promise it can >>>> >>>>> >> go into >>>> >>>>> >> 1.8.1 though as this is supposed to be a bug fix. >>>> >>>>> >> >>>> >>>>> >> Lenard >>>> >>>>> >> >>>> >>>>> >> >>>> >>>>> >> Forrest Voight wrote: >>>> >>>>> >>> >>>> >>>>> >>> This patch re-adds support for playing (and queueing) music from >>>> >>>>> >>> python file-like objects. >>>> >>>>> >>> >>>> >>>>> >>> While support for WAV music streams is still in SDL_mixer svn, >>>> >>>>> >>> there >>>> >>>>> >>> is support for mp3, mikmod and other formats already. >>>> >>>>> >>> >>>> >>>>> >>> >>>> >>>>> >> >>>> >>>>> >> >>>> >>>>> > >>>> >>>>> >>>> >>>> >>>> >>>> >>>> >>>> -- >>>> >>>> Lenard Lindstrom >>>> >>>> <[EMAIL PROTECTED]> >>>> >>>> >>>> >>>> >>>> >>> >>>> >> >>>> > >>> >>> >> >