[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

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



I submitted a patch to SDL_mixer to fix the mp3 problem.

On Sat, Jul 12, 2008 at 7:29 PM, René Dudfield <renesd@xxxxxxxxx> wrote:
> hi,
>
> I changed the version check to 1.2.9 instead... since support is in
> sdl_mixer svn is getting better.
>
> The tests are disabled for loading from file-likes -- so all tests
> pass again (at least they do here).
>
> cheers,
>
>
>
> On Sun, Jul 13, 2008 at 6:58 AM, Forrest Voight <voights@xxxxxxxxx> wrote:
>> I'm also able to play FLAC audio.
>>
>> I think the problem is that SDL_mixer detects MP3 files through the
>> first and second bytes (the first is '\xff'), and files with ID3 tags
>> have a special header first, so SDL_mixer defaults to mikmod decoding,
>> which determines it is invalid.
>>
>> So, some MP3 files work and some don't. I was able to find an MP3 file
>> without tags and it worked. This is an SDL_mixer bug.
>>
>> On Sat, Jul 12, 2008 at 4:44 PM, Forrest Voight <voights@xxxxxxxxx> wrote:
>>> Using SDL_mixer SVN, I'm able to load & play WAV and OGG.
>>>
>>> MP3 gives me the 'Module format not recognized' error. I'm looking
>>> into that now...
>>>
>>> On Sat, Jul 12, 2008 at 3:58 PM, Brian Fisher <brian@xxxxxxxxxxxxxxxxxxx> wrote:
>>>> My automated build machines both use 1.2.8, and they both fail to load mp3
>>>> (what the test is currently trying)
>>>>
>>>> so it seems that 1.2.8 doesn't support MP3. This post seems to indicate that
>>>> OGG and MOD should be supported:
>>>> http://listas.apesol.org/pipermail/sdl-libsdl.org/2007-December/063633.html
>>>> I tried OGG on my Mac though, and it just seemed to hang :(  I don't have a
>>>> mod file to try at the moment.
>>>>
>>>> So Forrest, what formats were you able to load when you were testing the
>>>> patch?
>>>>
>>>>
>>>> On Sat, Jul 12, 2008 at 12:13 PM, Forrest Voight <voights@xxxxxxxxx> wrote:
>>>>>
>>>>> Rene: what version of SDL_mixer are you using?
>>>>>
>>>>> I think I was wrong... SDL_mixer 1.2.8 supports OGG, MP3, MID, and MOD
>>>>> music.
>>>>>
>>>>> Only SVN supports WAV.
>>>>>
>>>>> On Sat, Jul 12, 2008 at 1:40 PM, Brian Fisher <brian@xxxxxxxxxxxxxxxxxxx>
>>>>> wrote:
>>>>> > by svn version do you mean 1.3, or an upcoming bug-fix release of 1.2?
>>>>> >
>>>>> >
>>>>> > On Sat, Jul 12, 2008 at 9:27 AM, Forrest Voight <voights@xxxxxxxxx>
>>>>> > wrote:
>>>>> >>
>>>>> >> The current sdl_mixer svn only supports mikmod. The svn version
>>>>> >> supports all of those formats.
>>>>> >>
>>>>> >> On Sat, Jul 12, 2008 at 4:55 AM, René Dudfield <renesd@xxxxxxxxx>
>>>>> >> wrote:
>>>>> >> > hi,
>>>>> >> >
>>>>> >> > I just wrote a test for it (see test/mixer_test.py).
>>>>> >> >
>>>>> >> > Unfortunately it doesn't seem to be able to load from a file like
>>>>> >> > object for mp3, ogg, or wav files.
>>>>> >> >
>>>>> >> > Any ideas?
>>>>> >> >
>>>>> >> >
>>>>> >> > ======================================================================
>>>>> >> > ERROR: test_mixer_music__load (__main__.MixerModuleTest)
>>>>> >> >
>>>>> >> > ----------------------------------------------------------------------
>>>>> >> > Traceback (most recent call last):
>>>>> >> >  File "test\mixer_test.py", line 207, in test_mixer_music__load
>>>>> >> >    pygame.mixer.music.load(open(musfn))
>>>>> >> > error: Module format not recognized
>>>>> >> >
>>>>> >> >
>>>>> >> > cu,
>>>>> >> >
>>>>> >> > On Thu, Jul 10, 2008 at 11:33 PM, René Dudfield <renesd@xxxxxxxxx>
>>>>> >> > wrote:
>>>>> >> >> 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>
>>>>> >> >>>>>> >>>>
>>>>> >> >>>>>> >>>>
>>>>> >> >>>>>> >>>
>>>>> >> >>>>>> >>
>>>>> >> >>>>>> >
>>>>> >> >>>>>
>>>>> >> >>>>>
>>>>> >> >>>>
>>>>> >> >>>
>>>>> >> >>
>>>>> >> >
>>>>> >
>>>>> >
>>>>
>>>>
>>>
>>
>