fix(asio): paused streams, draining timeouts and in-use drivers - #1374
Conversation
5b7f60c to
f4ad1f9
Compare
f4ad1f9 to
3af43da
Compare
Will do |
LastExceed
left a comment
LastExceed
left a comment
There was a problem hiding this comment.
Just 1 semantic technicality, rest LGTM
| - **ASIO**: Fix loading a driver while a previous driver was still unloading. | ||
| - **ASIO**: `Stream` no longer risks blocking or panicking in the driver callback while another stream is being created or destroyed. | ||
| - **ASIO**: A paused output stream now goes silent instead of looping the last buffered audio. | ||
| - **ASIO**: Building a stream while another driver is loaded now reports `DeviceBusy` instead of `DeviceNotAvailable`. |
There was a problem hiding this comment.
Not sure about this one. If I understand correctly, then the situation is basically:
this device is not available
because
another device is busy
As a user, I'd intuitively assume the error to be about the device that I am accessing, not about another which happens to be in the way. In that mindset, DeviceBusy is misleading. Knowing the inside details, I understand that DeviceBusy better expresses the cause of the problem, but from an outside perspective, DeviceNotAvailable is more representative of the resulting situation.
There was a problem hiding this comment.
That is an interesting point, you could say that DeviceBusy is a specialisation of DeviceNotAvailable. But, from src/error.rs:
/// The device is temporarily busy. This can happen when another application or stream
/// is using the device. Retrying after a short delay may succeed.
DeviceBusy,
/// The requested audio device is not available.
///
/// This can happen if the device has been disconnected while the program is running, or if
/// the device identifier refers to a device that does not exist on this system.
DeviceNotAvailable,DeviceBusy seems to closer match the definition, wouldn't you agree?
There was a problem hiding this comment.
No. The doc comment says "The device" twice. But which device? As a user I would naturally expect it to be the one I am trying to use. I might not even be aware that there is an alternative at all.
There was a problem hiding this comment.
Hmm, you're right. I guess doing that still addresses #1356 sufficiently by having clarified the asio-sys error message.
45434d6 to
b12a340
Compare
2 participants
@LastExceed could you review?
From a code review I am particular taken by the
pause()mechanism. Did that never work before or what am I missing?Fixes: