Pyglet media fails to close PulseAudio instances

[GoogleCodeExporter]

Using latest tip.zip on Ubuntu 13.10. Running the following code to play a 
bunch of sounds simultaneously causes an exception:

import pyglet
pyglet.options['audio'] = ('pulse',)
beep = pyglet.media.load('beep.mp3', streaming=False)
for _ in range(100):
    print(_)
    x = pyglet.media.Player()
    x.queue(beep)
    x.play()
    x.pause()
    # x._audio_player.delete()

Traceback (most recent call last):
  File "/home/larsoner/Desktop/untitled0.py", line 14, in <module>
    x.play()
  File "/home/larsoner/.local/lib/python2.7/site-packages/pyglet/media/__init__.py", line 1009, in play
    self._set_playing(True)
  File "/home/larsoner/.local/lib/python2.7/site-packages/pyglet/media/__init__.py", line 990, in _set_playing
    self._create_audio_player()
  File "/home/larsoner/.local/lib/python2.7/site-packages/pyglet/media/__init__.py", line 1070, in _create_audio_player
    self._audio_player = audio_driver.create_audio_player(group, self)
  File "/home/larsoner/.local/lib/python2.7/site-packages/pyglet/media/drivers/pulse/__init__.py", line 41, in create_audio_player
    player = PulseAudioPlayer(source_group, player)
  File "/home/larsoner/.local/lib/python2.7/site-packages/pyglet/media/drivers/pulse/__init__.py", line 258, in __init__
    check(-1)
  File "/home/larsoner/.local/lib/python2.7/site-packages/pyglet/media/drivers/pulse/__init__.py", line 21, in check
    raise MediaException(pa.pa_strerror(error))
pyglet.media.MediaException: Too large

Based on the `print(_)`, it looks like it dies on the 32nd try. So I guess 
there's an upper limit on the number of PulseAudio streams you can open at 
once, which makes sense.

This is a silly use case, but I have a real use case in mind. In my program, 
users can trigger the playback of an audio sample at random times. Whenever 
this is triggered, I create a new Player instance, cue a sound, and tell it to 
play similar to what happens inside the `for` loop above. It's possible that 
users might have a few sounds going simultaneously, so queuing additional 
sounds for sequential playback would not work.

In my use case, I wouldn't hit problems if there were a way to properly clean 
up the previous instances of Player once the streams completede, since I won't 
ever really have more than a few sounds ever going at once. However, it doesn't 
seem like the previous instances of Player ever get garbage collected and the 
PulseAudio streams shut off, because eventually I hit the same error as above. 
If I un-comment the `x._audio_player.delete()` line above, the code works. 
However, this "fix" won't work in practice in my code -- since the playback is 
asynchronous, I won't know the proper time to call the `.delete()` method.

Is there a better way to play simultaneous sounds (e.g., triggered whenever a 
user presses a button)? If there isn't, would it be possible to improve the 
cleanup of these objects once they are  done playing? I see from the code that 
it's supposed to be handled by the `__del__` attribute of `PulseAudioPlayer`, 
but even using `gc.collect()` it does not seem to take effect.

P.S. I just used this as the test sound (renamed to `beep.mp3`) in case anyone 
else wants to try:

http://soundbible.com/1252-Bleep.html

Original issue reported on code.google.com by larson.e...@gmail.com on 16 Apr 2014 at 9:01

[GoogleCodeExporter]

Thanks for your report!

Can you tell us which pulse version are your using?

Possibly not related to this, but pulseaudio support has some issues and some 
of them are not pyglet specific (eg, 
https://code.google.com/p/pyglet/issues/detail?id=651), and because that I tend 
to use openal in all my projects (helps portability too: openal works the same 
in windows, linux and os x).

You're probably right regarding the cleaning up (it has in fact some issues in 
windows with 2.7 because the garbage collector behaviour is lazier than in 
linux).

Have you tried creating the player out of the loop and then queuing your 100 
samples before hitting play?

Original comment by useboxnet on 17 Apr 2014 at 5:44

• Changed state: Accepted
• Added labels: OpSys-Linux, Type-Defect

[GoogleCodeExporter]

Bwt, I could use some help testing this: 
https://code.google.com/p/pyglet/issues/detail?id=716

;)

Original comment by useboxnet on 17 Apr 2014 at 6:03

[GoogleCodeExporter]

Do'h didn't realize it was your report too! Oh, dear; I'll work in 716 TODAY!

Original comment by useboxnet on 17 Apr 2014 at 6:03

[GoogleCodeExporter]

I fully expect queuing 100 samples and then hitting play to work, since it's 
the creating and destruction of streams that seems to be at the heart of the 
issue. (I will try in about an hour when I'm back on Linux just to be safe.) 
However, this won't play all 100 samples simultaneously, but instead 
sequentially, right? In which case, it's accomplishing a different outcome.

BTW there is something funny about the garbage collection, because I have tried 
adding `gc.collect` and adding `print` statements in the `__del__` method, and 
they don't ever really show up, even if I make the sound sample short, and put 
gaps between the additional calls to `Player` (which should allow for previous 
iterations to clean up the ones that were created). It's almost like they're 
being registered in a global var somewhere that prevents their ref count to go 
to zero, but I didn't see anything in the code that did this. Do you know of 
such a thing? It would make sense to have such a registry while the sound was 
playing to prevent the C library from referencing something that got GC'ed, but 
it would need to be tweaked to remove itself once it was done playing.

Original comment by larson.e...@gmail.com on 17 Apr 2014 at 1:51

[GoogleCodeExporter]

You're right, it won't play the samples at the same time.

I'm not very familiar with that part of the code, I just troubleshooted a 
couple of issues with Pulse. Mind you trying your simultaneous test with 
openal? So we can discard any issue with Pulseaudio or the Pulse driver.

Original comment by useboxnet on 17 Apr 2014 at 2:03

[GoogleCodeExporter]

OpenAL does not crash with my little code snippet. However, actual `OpenAL` 
playback is pretty brutal on my system -- lots of weird oddities that sound 
like buffer fills were missed, or just silence over long periods (during a clip 
with continuous sound). :(

Original comment by larson.e...@gmail.com on 17 Apr 2014 at 2:40

[GoogleCodeExporter]

And queuing 100 of them (as opposed to starting 100 of them simultaneously) 
does not cause a crash with either driver.

Original comment by larson.e...@gmail.com on 17 Apr 2014 at 2:43

[GoogleCodeExporter]

Thanks for the follow up!

That's weird. My experience is the opposite, I get all kinds of crazy stuff 
with Pulse driver :) Also it's cool when you code behaves more or less the same 
in Windows, Linux and Mac. In my experience Directsound has its own problems, 
so OpelAL looks like the best bet.

There's always the chance that something in Ubuntu 13.10 makes things funny 
(Pulseaudio is usually better implemented in Fedora), or the fact that it is an 
mp3 (if it is a short sample, would be worth trying a wav file -- or even an 
ogg file).

Original comment by useboxnet on 17 Apr 2014 at 2:45

[GoogleCodeExporter]

My actual use case involves playing arbitrary generated sound data (e.g., 2xN 
numpy arrays), so I've tried it with other sound samples. BTW, is there a 
supported way to play raw data like this? I created a simple `DataSource` class 
that subclassed `StaticSource` to accomplish this in about 20 lines. If there 
isn't currently a good way to play arbitrary data in Pyglet, I'd be happy to 
submit a patch.

BTW, I just remembered why I don't use OpenAL in Linux. It's mono, according to 
the docs and my ears :). This is a big problem for my use case, since I'm doing 
auditory psychophysics research. (We use a different system for auditory 
playback during actual experiments, but we use Pyglet audio playback for 
debugging on non-production tests.)

Original comment by larson.e...@gmail.com on 17 Apr 2014 at 2:54

[GoogleCodeExporter]

I don't know, but well... OSS, you can do whatever you need and if it's general 
and useful for other people, it will get into the project ;)

Yes, the docs may not be up to date in that regard, but I think it is mono. 
"auditory psychophysics research", that sounds cool! :)

Original comment by useboxnet on 17 Apr 2014 at 2:58

[GoogleCodeExporter]

I opened a new issue for the `ndarray` support, so that this one doesn't get 
too derailed :)

In any case, do you have any idea why the old Player instances would never get 
garbage collected? Is there a registry somewhere in the sound code that would 
prevent it? Maybe the ctypes access in Pulseaudio actually prevents it 
somehow...

Original comment by larson.e...@gmail.com on 17 Apr 2014 at 3:49

[GoogleCodeExporter]

My script bombs after creating 31 such instances, and it turns out none of them 
can be garbage collected as suspected:

>>> import gc
>>> gc.collect()
224
>>> len(gc.garbage)
31
>>> gc.garbage[0]
<pyglet.media.drivers.pulse.PulseAudioPlayer object at 0x7fe22e0be6d0>
>>> 

I'll try to figure out what's preventing it...

Original comment by larson.e...@gmail.com on 21 Apr 2014 at 2:53

[GoogleCodeExporter]

I commented out lines in the `pulse` code until I found what appears to be the 
culprit.

Turns out this is probably not a `pulse`-specific problem. Looks like there is 
a reference cycle due to the `Player` design. Line 1108 of 
`pyglet/media/__init__.py` in `Player._create_audio_player` calls:

    self._audio_player = audio_driver.create_audio_player(group, self)

`create_audio_player` gets wrapped (on my system) to calling `player = 
PulseAudioPlayer(source_group, player)`, and in `__init__` the first thing that 
happens is `AbstractAudioPlayer.__init__` gets called (which is presumably 
called by every backend), which does this:

    self.player = player

In other words, `Player` stores a reference to `PulseAudioPlayer` via the 
original call, and `PulseAudioPlayer` stores a reference to `Player` via this 
line. I think this prevents garbage collection. I've never had to deal with 
reference cycles before, but I'll try to come up with a solution. Does anyone 
have ideas?

Original comment by larson.e...@gmail.com on 21 Apr 2014 at 3:19

[GoogleCodeExporter]

Looks like changing that second line to:

    self.player = weakref.ref(player)

Gets rid of the reference cycle. Not 100% sure it's correct, but I think it 
should work.

If I leave all the other code in the PulseAudioPlayer commented out other than 
the AbstractAudioPlayer.__init__, things get garbage collected. However, after 
uncommenting the code it fails to get collected, so there must *also* be a 
problem within PulseAudio specifically. I'll see if I can track that down.

Original comment by larson.e...@gmail.com on 21 Apr 2014 at 3:40

[GoogleCodeExporter]

Good catch! I don't what would be the best approach to fix this, I'll give it a 
look when I have the time.

Original comment by useboxnet on 21 Apr 2014 at 3:40

[GoogleCodeExporter]

Okay, I made some good progress:

https://code.google.com/r/larsonericd-clone/source/detail?r=a33a776d131a9953786e
0082cea3e71659dccd33&name=fixpulse

The changes to `media/__init__.py` take care of the circular ref between 
`Player` and the underlying PulseAudioPlayer driver.

The changes to `media/drivers/pulse/__init__.py` take care of all but two of 
the lingering circular references. I've noted them with the line: "# NOTE: 
These two functions prevent garbage collection!"

The following example now works on my system:

import numpy as np
import pyglet
from time import sleep
pyglet.options['audio'] = ('pulse',)
beep = pyglet.media.NdarraySource(np.zeros((2, 441)), 44100)  # 10 ms
for _ in range(100):
    print(_)
    x = pyglet.media.Player()
    x.queue(beep)
    x.play()
    sleep(0.01)  # 10 ms
    x._audio_player.stop()
    del x._audio_player._write_cb_func
    del x._audio_player._underflow_cb_func

In other words, all we need to do is have a way to delete `self._write_cb_func` 
and `self._underflow_cb_func` once we know the PulseAudioPlayer is done 
playing, and garbage collection will commence. (Note that making 
`self._write_cb_func` and the other call into local vars in `__init__()` like 
`_write_cb_func` will cause a segfault, presumably because it gets garbage 
collected somehow.)

However, there is something odd going on here. Basically, if I tell a sound to 
play, the stream should be garbage collected *only once the sound is complete*. 
Otherwise, the sound will stop prematurely, which is not good. Or Pulse may try 
to call a function which no longer exists, which is worse (possible segfault). 
This means we need some way to permit garbage collection only once the sound 
has completed playing, and there is no more use for that instance of 
PulseAudioPlayer.

What I think would work is that when PulseAudioPlayer detects that it's done 
playing the stream (e.g., through its callbacks), it should check to see if its 
reference count is 2 (because it has two circular refs this *should* be the 
min), and if so, delete these last two attributes. This would enable automatic 
GC.

WDYT? I tried to implement this solution, but underflow/write/EOS code was too 
complex for me to decipher. (EOS was only triggered sometimes, so I wasn't sure 
if there was any way to determine when the stream was actually done.)

Original comment by larson.e...@gmail.com on 21 Apr 2014 at 8:42

[GoogleCodeExporter]

Just thought of a simpler solution that might help with this issue. I just 
noticed that `Player` itself calls this when going from one source to the next 
(I think?):

    self._audio_player.delete()
    self._audio_player = None

It seems like this should be done whenever the last source is done playing, 
too. That way the PulseAudio stream that is created by PulseAudioPlayer will be 
closed. It won't fix the circular references, but it should allow the important 
resources at least to be cleaned up. I'm still wrapping my head around how 
these things interact so I'm not sure if this is correct, but perhaps it will 
work...

Original comment by larson.e...@gmail.com on 21 Apr 2014 at 11:02

[GoogleCodeExporter]

This is a more general problem, I can reproduce it with openal and directaudio 
drivers too.

Your weakref patch didn't work, I get errors in a loop source (eg, 
AttributeError: 'weakref' object has no attribute 'dispatch_event'). I tried 
with you _audio_player idea, but I'm still investigating the best option.

Original comment by useboxnet on 25 May 2014 at 8:03

[GoogleCodeExporter]

I've implemented a Player.delete() method as specified in the 
AbstractAudioPlayer in 
https://code.google.com/p/pyglet/source/detail?r=2a90aa630bb44bfc54e056a755447cf
94300867b

Can you give it a go? Basically call that method when you're over with a 
player. I'm not sure if I should include it in __del__ in Player class, but in 
my tests seems to clean all the sources properly (you may need a gc.collect() 
call though).

Original comment by useboxnet on 26 May 2014 at 10:36

[GoogleCodeExporter]

I should have time to try it tomorrow -- sorry, been a bit busy.

Regarding __del__ -- sometimes I create a player and set it playing some long 
sound, but don't keep a reference to it in my code anywhere. With your edits 
plus an addition in __del__, will Python's automatic garbage collection will 
cause it to be prematurely stopped/terminated in this circumstance? That would 
not be good, since I think it would be good to have this "fire and forget" sort 
of functionality -- and it would be a regression, since currently you can 
expect it to keep playing even without storing a reference to it.

Original comment by larson.e...@gmail.com on 27 May 2014 at 11:37

[GoogleCodeExporter]

I think you should keep that reference :)

Pyglet is used for games and apps that require performance. I'm not sure what 
to do here. I've found that the delete() method may also need gc.collect(); as 
my problem was that when the app was exiting it had opened files (avbin didn't 
close the files in a sources set to loop in an uncollected player!).

We can document the behaviour and advice users what to do. You can ignore 
delete() and allow the the GC to collect most of the instances, but for example 
call gc.collect() explicitly between stages or game episodes. For sounds set to 
loop, Player.delete() may be a good idea though (because of the open files).

I'm still thinking on this but I'm looking forward for the result of your tests.

(btw, I did some improvements in the clean up code of openal, pulse is next!)

Original comment by useboxnet on 28 May 2014 at 11:31

[GoogleCodeExporter]

Awesome! I can now run this just fine:

import pyglet
pyglet.options['audio'] = ('pulse',)
beep = pyglet.media.load('noise.wav', streaming=False)
for _ in range(100):
    print(_)
    x = pyglet.media.Player()
    x.queue(beep)
    x.play()
    x.delete()

No need for explicit collecting at my end. I can make this work for the use 
cases I have in mind, even if it's not ideal (e.g., use a Timer to call 
`player.delete()` after the sound is done). Thanks for looking into it, and let 
me know if you end up cleaning up the Pulse code -- I'm curious to see how you 
do it.

Original comment by larson.e...@gmail.com on 28 May 2014 at 5:22

[GoogleCodeExporter]

Cool!

Well, basically I want to be sure that all file descriptors and all the memory 
is released properly (something that wasn't happening with the openal driver).

I'm not closing this ticket just now because I have to write some docs 
regarding the delete method, but thanks a lot for your help and follow up of 
this issue!

Original comment by useboxnet on 28 May 2014 at 5:36

[GoogleCodeExporter]

This issue was closed by revision be595d421796.

Original comment by useboxnet on 31 May 2014 at 7:16

• Changed state: Fixed