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 <renesd@xxxxxxxxx> 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 <brian@xxxxxxxxxxxxxxxxxxx> 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 <renesd@xxxxxxxxx> wrote:
>>>
>>> Cool, thanks. I'll try and patch it tonight (+ 9 hours).
>>>
>>> cu,
>>>
>>> On Wed, Jul 9, 2008 at 1:09 AM, Forrest Voight <voights@xxxxxxxxx> 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 <renesd@xxxxxxxxx> 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 <voights@xxxxxxxxx>
>>> >> wrote:
>>> >>> OK, I'll work on unit tests and a version check.
>>> >>>
>>> >>> On Mon, Jun 16, 2008 at 11:52 AM, Lenard Lindstrom <len-l@xxxxxxxxx>
>>> >>> 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 <renesd@xxxxxxxxx>:
>>> >>>>
>>> >>>>> 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 <voights@xxxxxxxxx>
>>> >>>>> wrote:
>>> >>>>> > Thanks!
>>> >>>>> >
>>> >>>>> > On Fri, Jun 13, 2008 at 7:31 PM, Lenard Lindstrom
>>> >>>>> > <len-l@xxxxxxxxx> 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
>>> >>>> <len_l@xxxxxxxxx>
>>> >>>>
>>> >>>>
>>> >>>
>>> >>
>>> >
>>
>>
>
Attachment:
pymusic.diff
Description: Binary data