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

ASIO: Handle buffer size changes #473

Closed
wants to merge 2 commits into from

Conversation

npostavs
Copy link

This fixes #472. I don't know if it's the best approach.

When kAsioBufferSizeChange or kAsioResetRequest is received, remember
it and update the buffer sizes the next time
PaAsio_GetAvailableBufferSizes() is called.  Add callbacks for those
messages so that the library user can be aware of them.

@RossBencina RossBencina added the src-asio Steinberg ASIO Host API /src/hostapi/asio label Jan 27, 2021
@RossBencina
Copy link
Collaborator

I'm not sure whether it is the best approach but it is not bad. bufferSizeOutdated needs to be atomic.

I wonder whether a single generic callback would be better than individual callbacks for each event type.

@npostavs
Copy link
Author

bufferSizeOutdated needs to be atomic.

Ah, it's been a while since I've had to think about threads. Do you mean std::atomic?

I wonder whether a single generic callback would be better than individual callbacks for each event type.

Hmm, probably yes. I started out thinking that just kAsioBufferSizeChange would give me the all the info I wanted. But then it turns out my device sends kAsioResetRequest when I change the buffer size (I assume this is typical). And I end up ignoring the newBufferSize parameter in the code anyway.

Copy link
Collaborator

@philburk philburk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for contributing.

@npostavs
Copy link
Author

npostavs commented Feb 6, 2021

bufferSizeOutdated needs to be atomic.

Ah, it's been a while since I've had to think about threads. Do you mean std::atomic?

According to https://github.com/PortAudio/portaudio/wiki/ImplementationStyleGuidelines, C++ code should be C++98 so std::atomic wouldn't be available. But perhaps the guidelines are out of date?

I'm also unsure about the thread safety of ASIOGetBufferSize.

@npostavs
Copy link
Author

I'm thinking perhaps it would be simpler to just drop all the attempts at updating the buffer size, leaving just the callbacks. The library user would be responsible for calling Pa_Terminate() and Pa_Initialize() to get updated info.

This would push all the threading trouble onto the caller. And there can be changes other than buffer size anyway.

When kAsioBufferSizeChange or kAsioResetRequest is received, remember
it and update the buffer sizes the next time
PaAsio_GetAvailableBufferSizes() is called.  Add callbacks for those
messages so that the library user can be aware of them.
It raises threading synchronization issues, and not all
kAsioResetRequest events will be about that anyway.  For example,
ASIO4ALL can have changes in input channels; Portaudio's current API
can't have chanel names change without reinitializing the library.
@philburk philburk modified the milestone: V19.8 Mar 16, 2021
@RossBencina
Copy link
Collaborator

@npostavs do you think #519 would supersede this?

@npostavs
Copy link
Author

As I mentioned there, #519 doesn't seem to cover the case of messages sent before a stream is opened.

@RossBencina
Copy link
Collaborator

@npostavs I don't think it is possible to receive callbacks before the stream is opened. Your code does not handle that either: the callbacks asioCallbacks_ are set during OpenStream.

@npostavs
Copy link
Author

npostavs commented May 6, 2021

I don't think it is possible to receive callbacks before the stream is opened.

Oh, you're right. I managed to confuse myself into thinking it could work (possibly I got a bit mixed up between opening vs starting the stream). I believe #519 supersedes this one then.

@RossBencina
Copy link
Collaborator

Closing as superseded by #519.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
src-asio Steinberg ASIO Host API /src/hostapi/asio
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handling ASIO buffer size changes
3 participants