Implement Timeout Feature for sounddevice.wait() Function

elias-jhsph commented 7 months ago

The purpose of this pull request is to introduce a new feature to the sounddevice module's wait() function. This update adds an optional timeout parameter, allowing users to specify a maximum waiting time for audio playback or recording to finish.

Key Highlights:

Introduces a timeout parameter to the wait() function, enabling users to set a maximum wait time.
Enhances the usability of the sounddevice module, particularly in scenarios where blocking the thread indefinitely is not viable.
Ensures backward compatibility by setting the default value of timeout to None, which retains the original behavior of the wait() function.
Includes updated documentation and docstrings explaining the new feature and how to use it.
The changes have been thoroughly tested to ensure reliability and consistency with the existing functionality of the module.

This resolves issue #507

Thank you for considering this contribution to the sounddevice module. I look forward to your feedback and the opportunity to improve this essential audio handling tool in Python.

mgeier commented 7 months ago

Thanks for this PR!

I'm not sure if it is good to have two falsey return values (None and False) for two different outcomes (success and timeout).

Could you please show a few hypothetical uses that illustrate how to deal with those return values?

elias-jhsph commented 7 months ago

I see your point.

I am not 100% on whether there are scenarios when finished can be True and self.status can be not None. If that is possible, by using False and True to return when using the timeout arg, False can differentiate from a claim that self.status is None and more clearly states that simply the timeout was reached but self.status could be None or not at this time. Similarly to True differentiate from a claim that self.status is None and more clearly states that simply the event was triggered and the stream was closed without exception. It also has the added benefit of matching return behavior of the standard threading library “wait” in this scenario.

On the other hand, if you think None won’t be overthought and especially if when Finished is True self.status is always None then differentiating between status returning and timeout behavior returning is less important and line #2659 could be

return True if finished else None

In general though, I think we both agree that if we are using a timeout we want to know if the timeout was reached or the event was set and the stream has been closed. But if you want some examples on that I can happily provide them.

mgeier commented 7 months ago

But if you want some examples on that I can happily provide them.

Yes, please.

Currently, I'm not interested in thinking about alternatives, I would first like to see how your current suggestion looks when it is used.

elias-jhsph commented 6 months ago

Old behavior is preserved but here is some examples of how timeout could be used

Interactive Audio Guide for a Museum Exhibit

Imagine an interactive audio guide system in a museum where visitors can trigger audio descriptions of various exhibits. Since the length of these audio clips can vary and visitors might interact with multiple exhibits simultaneously, the system needs to check for new inputs while playing the current audio clip.

import sounddevice as sd

def load_exhibit_audio(exhibit_id):
    # Load the audio file for the given exhibit
    return ...  # Replace with actual audio loading logic

def check_for_new_interaction():
    # Check if a visitor has interacted with a new exhibit
    return ... # Replace with actual interaction check logic

# Initial setup
current_exhibit_id = "exhibit_1"
audio_clip, samplerate = load_exhibit_audio(current_exhibit_id)
sd.play(audio_clip, samplerate)

poll_speed = 2  # Interval for each check (seconds)

# Main loop
while True:
    new_interaction = check_for_new_interaction()
    if new_interaction and new_interaction != current_exhibit_id:
        sd.stop()
        print(f"New interaction detected: {new_interaction}")
        current_exhibit_id = new_interaction
        audio_clip, samplerate = load_exhibit_audio(new_interaction)
        sd.play(audio_clip, samplerate)

    finished = sd.wait(timeout=poll_speed)
    if finished is not False:
        if finished is True:
            print("Restarting...")
            break
        else:
            print("Waiting for next interaction...")
            time.sleep(poll_speed)

print("Audio guide session ended.")

Environmental Interaction with Audio Playback and Monitoring

A research setup plays an audio clip (like a bird song) to study its influence on animal behavior. Simultaneously, a separate sensor system monitors the environment for any responses or changes.

import sounddevice as sd
import time

# Prepare an audio clip (e.g., bird song)
bird_song, samplerate = ...  # Load or generate bird song audio

def check_environmental_response():
    return # Logic to receive data from the sensor system

# Start playing the bird song
sd.play(bird_song, samplerate)

monitoring_duration = 30  # Total duration for monitoring (seconds)
timeout_duration = 5      # Interval for each check (seconds)
start_time = time.time()

finished = False
while not finished:  
    response_detected = check_environmental_response()
    if response_detected:
        print("Environmental response detected!")
    else:
        print("No response detected yet.")
    finished = sd.wait(timeout=timeout_duration)
    if time.time() - start_time > monitoring_duration:
        sd.stop()
        break

if finished is False:
    print("Bird song cut off because it was longer than monitor duration")
elif finished is not True:
    print("Bird song cut off due to issue during playback")
else:
    print("Bird song finished")

pep8speaks commented 6 months ago

Hello @elias-jhsph! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:

Comment last updated at 2023-12-05 22:27:32 UTC

mgeier commented 6 months ago

Thanks for these examples! I think they will be very useful for evaluating the API.

There is a little bug, but it isn't really relevant for us: I think it should be while not finished.

And there are a few instances where pycodestyle complains:

E712 comparison to False should be 'if cond is not False:' or 'if cond:'

Using == and != with True and False is un-Pythonic.

Apart from that, I would be interested how the CallbackFlags-part of the result could be handled. IMHO it is a cardinal sin to ignore buffer over-/underruns. And in case there is no timeout, I think it would be appropriate to check for that.

elias-jhsph commented 6 months ago

How embarrassing - I didn't run them and I am sure if I had made them runable I would have caught that...

Do you need anything more from me at this time or are you just going to take some time to reflect on it?

mgeier commented 6 months ago

Don't worry, it doesn't really have to be runnable. It's only about the line with the sd.wait(timeout=...) call and the following handling of its return value.

Do you need anything more from me

Yes, sorry that I wasn't clear ... it would be great if you could provide variations of your examples that handle the CallbackFlags thing. Because this will make the handling of the return value even more complicated.

Also, it would be nice if you could fix the thing that pycodestyle complains about, because things like finished == False look really wonky.

And then I would be interested in your opinion whether that usage looks nice, or if you see any potential for improvement.

elias-jhsph commented 6 months ago

ok I was worried about the falsey-ness of is False but I made sure and then edited the examples comment

>> test = None
>> if test is False:
      print("bad")
   else:
      print("ok good")

ok good

I don't have experience handling CallbackFlags but I can figure it out.

for reference below from the PR

        exception_raised = True
        finished = False
        try:
            finished = self.event.wait(timeout=timeout)
            exception_raised = False
        finally:
            if finished or exception_raised:
                self.stream.close(ignore_errors)
        if timeout and not exception_raised:
            return finished
        return self.status if self.status else None

The questions I will have to answer before I can answer you:

After self.stream.close are callback flags reset?
What action make sense if some callback flags are set for this level of the api?
Will sounddevice raise exceptions while the wait is running

if you have an advice feel free to let me know, but otherwise I will try and figure these out and post what I found, then the next questions I am working on, until I am in a better position to extend my examples to include callback flags handling

mgeier commented 6 months ago

After self.stream.close are callback flags reset?

No, they should stay available to be queried by the user until the next stream is created.

What action make sense if some callback flags are set for this level of the api?

I think the most important action is somehow reporting or logging the problem. I guess the easiest to do is something like this:

if flags:
    print(flags)

But that's the problem with returning True, which complicates this check.

For this PR it doesn't really matter what the user does with this information, as long as they can find out whether some flags are set or not and react accordingly.

Will sounddevice raise exceptions while the wait is running

No, I don't think so.

mgeier commented 6 months ago

I also wanted to comment on something you said earlier:

It also has the added benefit of matching return behavior of the standard threading library “wait” in this scenario.

The behavior of different timeout parameters in the standard library is surprisingly inconsistent. Sometimes True or False are returned, but there is also https://docs.python.org/3/library/asyncio-task.html#asyncio.wait_for and https://docs.python.org/3/library/queue.html#queue.Queue.get, which raise different exceptions on timeout.

Maybe this would also make sense here?

It might look something like this:

while True:
    if ...:
        sd.play(...)

    try:
        status = sd.wait(timeout=poll_speed)
    except sd.TimeoutExpired:
        print("Waiting for next interaction...")
    else:
        if status:
            print(status)
        print("Restarting...")
        break

mgeier commented 5 months ago

@elias-jhsph Any news on this? What do you think about my comments?

elias-jhsph commented 5 months ago

I was busy with work for my job but I am picking this back up this week

From: Matthias Geier @.> Sent: Sunday, January 21, 2024 12:53:42 PM To: spatialaudio/python-sounddevice @.> Cc: Elias Weston-Farber @.>; Mention @.> Subject: Re: [spatialaudio/python-sounddevice] Implement Timeout Feature for sounddevice.wait() Function (PR #509)

  External Email - Use Caution

@elias-jhsphhttps://github.com/elias-jhsph Any news on this? What do you think about my comments?

— Reply to this email directly, view it on GitHubhttps://github.com/spatialaudio/python-sounddevice/pull/509#issuecomment-1902709517, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ANSMEBB56D64SCG63O6T7WLYPVI2NAVCNFSM6AAAAABAENLZQKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBSG4YDSNJRG4. You are receiving this because you were mentioned.Message ID: @.***>

mgeier commented 3 months ago

@elias-jhsph Did you have a chance to look at this in the meantime?

spatialaudio / python-sounddevice