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

How to handle stream disconnect #32

Open
SupernaviX opened this issue Jul 10, 2021 · 4 comments
Open

How to handle stream disconnect #32

SupernaviX opened this issue Jul 10, 2021 · 4 comments

Comments

@SupernaviX
Copy link

SupernaviX commented Jul 10, 2021

Hi!

I've been using oboe-rs in my program, and for phones > 8.0 (i.e. anything that uses AAudio) it seems to be a frequent source of crashes. I finally tested the aaudio paths on my own phone (8.0, the first buggy version that can use AAudio) and it looks like

  • When I plug or unplug headphones, the stream disconnects (throws a DISCONNECTED error)
  • After this happens, audio stops playing AND the next time I call request_stop I get a CLOSED error.
    (If it makes a difference, my source code is here).

Reading the AAudio docs, it looks like I'm supposed to cope with this case by (opening a new thread and) creating a brand new audio stream. My audio callback owns a buffer which another thread writes to, so I can't easily construct a new callback; I think I need to create a new stream which reuses the callback from the disconnected one. My problem is, these rust bindings are getting in the way of this.

  • I can't directly pass the callback into a new AudioStreamBuilder, because the builder takes ownership of it.
  • I can't reuse the AudioStreamBuilder to make a new stream, because AudioStreamBuilderAsync.open_stream takes self instead of &self and consumes the builder.

I think that both of these are issues with the rust typings.

The documentation for set_callback says that the API intentionally doesn't take a smart pointer, but is not meant to own the callback:

The caller should retain ownership of the object streamCallback points to. At first glance weak_ptr may seem like a good candidate for streamCallback as this implies temporary ownership. However, a weak_ptr can only be created from a shared_ptr. A shared_ptr incurs some performance overhead. The callback object is likely to be accessed every few milliseconds when the stream requires new data so this overhead is something we want to avoid.

This leaves a raw pointer as the logical type choice. The only caveat being that the caller must not destroy the callback before the stream has been closed.

The AAudio docs say that

You can save the builder and reuse it in the future to make more streams.

Do you have any guidance for how to handle this case? I can't find any sample usages of oboe-rs that deal with disconnected streams or non-trivial callbacks, and I can't follow the C++ examples because of these typings. I'm an amateur in programming audio, rust, and Android, so I suspect I'm doing something wrong, but can't find any good examples to copy from.

@katyo
Copy link
Owner

katyo commented Jul 16, 2021

@SupernaviX Course, this library still far from perfection.

The stream builder takes ownership of callback but current API lacks any ways to get it back when stream drops.
I already thinking about such scenarios. In my opinion the tradeoff solution is adding function to convert stream to builder back. Additionally we may add function to decouple builder and callback.

Have you any better ideas?

@SupernaviX
Copy link
Author

@katyo I think that I found a way to accomplish this with the current interface in my project. Sample usage here.

I wrote a ManagedAudioOutputCallback trait, which is like the regular AudioOutputCallback but also knows how to build new streams for itself. A wrapper ReconnectingAudioOutputCallback holds onto an Option<ManagedAudioOutputCallback>, forwards calls to it on the happy path, and creates a new stream from it on disconnect. The system also keeps an Arc<Mutex<the current stream>> up-to-date, so that I can control the stream from the main thread.

I think that "turn a stream back into a builder" (or maybe just "clone a stream") could be helpful, but also hard to do. I'm realizing now that the stream really does need to own its callback, because otherwise you have to either deal with mutexes in a critical path or multiple streams possibly messing with the callback at once. And you need a thread-safe "global" reference to the "current" stream somewhere, which gets updated inside of on_error_after_close , or else you can't control the stream from the outside anymore. So I can't think of a nicer way to handle all that than what I've got so far.

@katyo
Copy link
Owner

katyo commented Jul 26, 2021

@SupernaviX looks interesting as as compromise solution.

@chriscoomber
Copy link

chriscoomber commented Sep 22, 2021

I'd like to add that I'd love a way to get the callback back when closing the stream.

At risk of giving too much detail:

I'm writing a MIDI file player, so my callback calculates the next chunk of sound data, stepping through the MIDI file (stored in memory) as it does so. So, the callback needs a mutable reference (or ownership) to:

  • my sequencer (the thing that manages the position in the MIDI file), and
  • my synthesizer (the thing that keeps track of which notes are playing, and generates sound data)

All this happens on the stream's callback thread.

However, users can also pause playback at any time (closing the stream), and also skip ahead in the file while the stream is running. These actions come from a different thread. I'm fine with a small delay on these actions, so my plan was to just send messages to the callback thread which it can act on them when it gets a chance. This way there's only one thread mutating my sequencer/synthesizer, and I don't have to worry about locks or blocking. For pausing playback, it depends on whether the callback had a mutable reference to the sequencer/synthesizer or ownership: in the former case, this doesn't really work without hacking (e.g. Box::leak) because the lifetime of that reference won't be 'static; in the latter case I'd need the stream to return the callback to me upon closing it.

Sorry if that wasn't particularly clear. Basically for performance reasons I want the callback thread to directly access some resources, but for usability reasons I want those resources to outlive the thread (e.g. in the case of pause/play) and be mutable (albeit with small delay) from outside the thread.

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

No branches or pull requests

3 participants