Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Timeout Feature for sounddevice.wait() Function #509

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

elias-jhsph
Copy link

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.

This commit introduces a new `timeout` parameter to the `wait()` function in the `sounddevice` module.
The addition of the `timeout` argument enhances the flexibility of the `wait()` function,
allowing users to specify a maximum duration to wait for audio playback/recording to finish.
This feature is particularly useful in scenarios where blocking the thread indefinitely is not desirable or practical.

Changes:
- Modified the `wait()` function definition to include an optional `timeout` parameter.
- Updated the `wait()` function's docstring to describe the new parameter and its behavior.
- Adjusted the internal `_CallbackContext.wait()` method to handle the timeout functionality, ensuring that the function returns a boolean indicating whether the operation completed before the timeout or if the timeout was reached.
- Ensured backward compatibility by setting the default value of `timeout` to `None`, preserving the existing behavior of waiting indefinitely until the operation completes.

This update provides users with greater control over their audio operations without working directly with the stream, making the `sounddevice` module more versatile for a variety of applications.
@mgeier
Copy link
Member

mgeier commented Dec 3, 2023

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
Copy link
Author

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
Copy link
Member

mgeier commented Dec 3, 2023

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
Copy link
Author

elias-jhsph commented Dec 5, 2023

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")

fixing bullet list for sphinx
@pep8speaks
Copy link

pep8speaks commented Dec 5, 2023

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! 🍻

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

@mgeier
Copy link
Member

mgeier commented Dec 6, 2023

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
Copy link
Author

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
Copy link
Member

mgeier commented Dec 6, 2023

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
Copy link
Author

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
Copy link
Member

mgeier commented Dec 6, 2023

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
Copy link
Member

mgeier commented Dec 6, 2023

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
Copy link
Member

mgeier commented Jan 21, 2024

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

@elias-jhsph
Copy link
Author

elias-jhsph commented Jan 21, 2024 via email

@mgeier
Copy link
Member

mgeier commented Mar 30, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants