Re: [pygame] Patch - Re-add music support for file-like objects

["René Dudfield" <renesd@xxxxxxxxx>]

Cool, thanks.

Committed revision 1482.

On Wed, Jul 9, 2008 at 12:39 PM, Forrest Voight <voights@xxxxxxxxx> 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 <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>
>>>> >>>>
>>>> >>>>
>>>> >>>
>>>> >>
>>>> >
>>>
>>>
>>
>