fix issue #1

[tgarc]

fix for #1

this commit makes sure that any streams started with play/rec/playrec
are closed before exiting to avoid segmentation faults from unreleased
PaStream objects

Added a similar try/finally to StreamBase.__exit__ to make sure with always closes the stream properly.

Don't quote me on this but I'm pretty sure it's possible that a stream that has had PaStopStream() called on it can be started again without creating a new stream object so it may make sense to add a global close function to deallocate the stream corresponding to _last_callback.

[mgeier]

Thanks!

I'm just trying to reproduce the original problem #1 on macOS and the error doesn't seem to happen there (but your changes don't seem to hurt, either).

I'll try it later on Linux.

On which OSs could you reproduce the original error?

it may make sense to add a global close function

I don't understand. A sound started with play() is not supposed to be started again.
I think stop() should be enough. It already calls stream.close().
After stop(), there is now way (and IMHO shouldn't be) to "open" the stream again.

Maybe I'm missing something ... can you provide an example where a global close() would be used?

I think it makes sense to call stop() in the exit handler, though.

[tgarc]

I'm just trying to reproduce the original problem #1 on macOS and the error doesn't seem to happen there

Did you try using python 2? I tested it using python 2 on Linux. Anyhow, it absolutely makes sense that the error would occur: the _last_callback stream never gets closed in the exit handler. The call to stop() I added there is the only line needed to fix this particular issue.

it may make sense to add a global close function

I don't understand. A sound started with play() is not supposed to be started again.
I think stop() should be enough. It already calls stream.close().
After stop(), there is now way (and IMHO shouldn't be) to "open" the stream again.

What I meant was, I think that you can start and stop a PaStream multiple times as long as it hasn't been closed. I'm starting to suspect that I'm wrong about this though since I haven't been able to find any examples or evidence of this. I just always figured it was the case since that's the only way I could reason having a separate PaStopStream and PaCloseStream. (If it's not restartable than why not just have PaAbortStream and PaStopStream close the stream?).

Anyhow, that's kind of a separate discussion.

One detail about the commit: Do you have any objection to the part of the commit which doesn't ignore errors on __exit__?

[mgeier]

OK, I think this was a bug in PortAudio and it was fixed in the newest version.
Sure, I admit that I didn't properly close the stream, but PortAudio could as well have taken care of this when shutting itself down. And it looks like now it does.

Which PortAudio version did you use?

I just downgraded to (1899, u'PortAudio V19-devel (built Feb 15 2014 23:28:00)'), and with that version I can reproduce the interpreter crash on Python 3. Surprisingly, it still doesn't crash with Python 2.

But anyway, since not everyone is using the latest and greatest version yet, we should apply your fix.

This PR indeed fixes the problem, but I think it does too much.

I assumed that closing a stream implies stopping it (or rather, to be exact, aborting it). Did you encounter a situation where this wasn't the case?

I'm not sure about the stop()'n'close() combo inside of __exit__() ...
I guess you wanted to ensure closing the stream if stop() fails, right?
Were you able to provoke such a situation?

Even if this can happen, the stream would already be in a broken state, so close() would very likely fail as well, so I don't know if this makes sense.

I think that you can start and stop a PaStream multiple times as long as it hasn't been closed.

Yes, you can indeed do that.

Try this in an interactive session:

>>> import sounddevice as sd
>>> def callback(indata, outdata, *stuff):
...     outdata[:] = indata
... 
>>> s = sd.Stream(channels=2, callback=callback)
>>> s.start()
>>> s.stop()
>>> s.start()

Works like a charm for me (BTW, multiple consecutive calls to stop() are fine). Once the stream is closed, it's over:

>>> s.close()
>>> s.start()
Traceback (most recent call last):
...
sounddevice.PortAudioError: Error starting stream: Invalid stream pointer
>>> 

If the stream is stopped by raising an exception in the callback, it cannot be re-start()ed, though.
This is obscurely hinted to in the documentation, which I took straight from the PortAudio docs:
http://python-sounddevice.readthedocs.io/en/latest/#sounddevice.Stream.stopped
http://portaudio.com/docs/v19-doxydocs/portaudio_8h.html#a52d778c985ae9d566de7e13529cc771f

[tgarc]

wait so you're saying that the only time that stopped will be true is if stop was called on it? This seems to be the case: if I let all my callbacks finish successfully (i.e. paComplete was returned on the last callback) then stopped is still false! The portaudio docs do seem to say this but man, you'd think they'd try to make that super clear since it's not what you'd intuitively expect.

[tgarc]

pro tip: If you use finished_callback, it will only be called if the stream status was stopped before starting the stream. e.g., if you call start() then abort() then start() again, the finished_callback will only be called the first time around. In order to get it to call your finished_callback each time you must call stop() before each subsequent call to start().

Phew. I'm using an older portaudio version so it's possible this was a bug that's been fixed. I'll let the portaudio people know otherwise.

[mgeier]

wait so you're saying that the only time that stopped will be true is if stop was called on it?

Yes. At least that's how I understand it.
It seems that "stopped" doesn't just mean "not doing anything" but more strictly "somebody has actively used the stop function". Confusingly, it could also mean that nobody as used the start function yet ...

The behavior of finished_callback you described indeed sounds like a bug.

On top of somewhat vaguely specified behavior, you never know if all host APIs at least behave the same way ...

[tgarc]

On top of somewhat vaguely specified behavior, you never know if all host APIs at least behave the same way ...

Well here's a case in point: when I abort() a stream on my redhat machine (using ALSA), IsStreamStopped returns False. (Which is not what I'd expect). However, I tested the same code on a windows machine with ASIO and IsStreamStopped returns True after abort() is called.

I need to go back and sync up my portaudio library versions to see if this difference in behavior is still true today..the one I'm using on linux is all the way from 2010.

EDIT: I tested this with a portaudio release from Oct 2016 and IsStreamStopped still returns False on Linux/ALSA after a call to AbortStream. I've reported this on the portaudio mailing list.

[mgeier]

@tgarc Would be nice to get this merged ... what are your thoughts on my comments above (#75 (comment))?

[tgarc]

This PR indeed fixes the problem, but I think it does too much

I think you're right. The thing is when I tried removing all the changes except for the adding of stop() in the exit_handler the program just hung inside of Pa_CloseStream. I just hadn't spent the time to figure out why this is happening yet. I have a hunch that somehow Stop/Abort/Close is being called during this call to Pa_CloseStream (or the other way around). In my experience this causes portaudio to hang indefinitely; this is not unexpected since the API is not thread safe.

[tgarc]

so oddly enough, aborting or closing the stream inside the exit handler causes portaudio to hang. however calling PaStopStream before closing seems to fix the issue altogether. Try it out but I think this is ready for a merge. Feel free to look into why it hangs when you call close() without calling stop() first in the exit handler, but it was starting to look like a rabbit hole to me. I suspect it has to do with the non-deterministic behavior of closing down the interpreter plus portaudio, but that's about as far as I got.

[tgarc]

Sure, I admit that I didn't properly close the stream, but PortAudio could as well have taken care of this when shutting itself down. And it looks like now it does.

True. I tried loading up a newer (2016) version of portaudio and I couldn't reproduce this issue.

[mgeier]

@tgarc Thanks for your investigations, I could reproduce the hanging behavior and it is indeed very strange.

I wanted to suggest that you use abort() instead of stop(), because this would convey the urgency of the situation a bit better.
But alas, with abort() it also hangs!

I don't have the stamina to investigate this further, and I wouldn't even know where to start.

But since you fixed it and current versions of PortAudio don't even have that bug anymore, I'm fine with not knowing all the details.

I'm merging this, thanks again for your help!