Add support for ASIO messages

[EvanBalster]

This PR addresses PortAudio's bad habit of ignoring certain highly actionable messages from ASIO drivers by providing a callback mechanism to the application. Applications may receive the following events:

• Reset Request
• Resync Request
• Sample Rate Changed
• Buffer Size Changed
• Latencies Changed

In particular, Reset Request is commonly used to notify the ASIO client when the stream should be reset in order to put configuration changes into effect or resume the interrupted flow of audio. Implementing automatic reset allows the user to interactively re-configure the ASIO stream by way of a driver-specific UI without superfluous manual steps in the client application.

The patch works by extending the ASIO configuration structure and, therefore, does not change PortAudio's ABI. I have been using this functionality in production for over four years in an application with an integrated crash forensics system and haven't identified any faults.

[EvanBalster]

Additional context: This PR is a recreation of an old merge request on PortAudio's previous Assembla-hosted repository. readers can find that Merge Request here...

https://app.assembla.com/spaces/portaudio/git/merge_requests/3939463

[npostavs]

I have a PR that overlaps somewhat with this, so we should probably coordinate that (#473). The difference is that I only cover kAsioResetRequest and kAsioBufferSizeChange, and the callback isn't tied to a stream. What happens with our code if the user changes the buffer size before the application is playing/recording sound? I think I mixed up between opening vs starting the stream here. I believe this PR can simply replace mine.

[RossBencina]

I still need to review this properly, but two obvious things:

1. This extension should be selected using a flag (same as other host API extensions) not (only) by inspecting the struct version
2. it needs to be documented is that it is the client's problem to deal with synchronising notification callbacks that occur during Pa_CloseStream() to avoid race conditions.

[EvanBalster]

I notice this was tagged for inclusion in the next release of PortAudio.

I'm currently working on ASIO functionality in my company's application and expect I will have an opportunity to revisit this Pull Request in the coming weeks.

[dyfer]

If I understand correctly, this PR would prevent PortAudio apps from using JackRouter (routing audio through JACK) on Windows, is this correct? If so, such change would affect SuperCollider Windows users, if they use JACK. I understand that one can't make everyone happy, but I think this would be an unfortunate regression.

To clarify, the issue only occurs with older versions of JACK (pre-2021); it does not occur with the current version.

EDIT: maybe blocking JackRouter could be enabled/disabled at compile time with a CMake option? That way those building PortAudio themselves could make the appropriate decision as they see fit.

[EvanBalster]

If I understand correctly, this PR would prevent PortAudio apps from using JackRouter (routing audio through JACK) on Windows, is this correct? If so, such change would affect SuperCollider Windows users, if they use JACK. I understand that one can't make everyone happy, but I think this would be an unfortunate regression.

To clarify, the issue only occurs with older versions of JACK (pre-2021); it does not occur with the current version.

EDIT: maybe blocking JackRouter could be enabled/disabled at compile time with a CMake option? That way those building PortAudio themselves could make the appropriate decision as they see fit.

This PR blocks interfacing with JACK through its ASIO adaptor, "JackRouter", in order to avoid a problem that historically rendered the JACK ASIO adaptor nonfunctional for 64-bit apps. PortAudio also supports interfacing directly with the JACK API on Windows (ie, natively rather than through ASIO/JackRouter) which will understandably be the better approach to making Windows applications with Jack support.

If the bug in JackRouter has been fixed, and that fix has propagated widely, the guard could be removed -- perhaps with a CMake option as you suggest. But be advised that the JackRouter problem causes applications to crash when initializing PortAudio on any system with the faulty ASIO driver; this usually means an app without this guard will be unopenable on affected systems, regardless of whether the user intends to interface with JACK. The nature of the crash also makes inspection of the debug trace rather difficult.

[dyfer]

I'm aware of the bug, and that it prevents the application from starting.

Is building PortAudio with the JACK backend on Windows actually operational? I don't think that was the case in the past. Also, would the application be then linked against the Jack library? IMO it's quite important to allow the same binary to run both on systems that do and don't have JACK installed.

If PA's JACK backend works on Windows, AND if the JACK library would be dynamically loaded, then indeed it is a viable workaround.

As I wrote, the fixed JackRouter has been around for over two years now. I don't have the data on how well it is propagated. Since SuperCollider uses PortAudio on Windows, this has come up in SuperCollider's issue tracker, so it is likely that there are users who use PortAudio + Jack (with fixed JackRouter) on Windows.

[EvanBalster]

Pardon my two year (minus one day) delay.

• Removed the rule blacklisting the JackRouter virtual ASIO driver.
• Added PaAsioUseMessageCallback flag as requested by @RossBencina, and added validation code.

Note that the latter change will break compatibility with any existing applications that use my branch (the message callback will not be used because the flag is not set).

I considered checking for this in ValidateAsioSpecificStreamInfo but that would represent a departure from the way channelSelectors is validated (ie, non-null channelSelectors is simply ignored if the flag is not set). This gives me the impression the library is meant to tolerate the case where unused fields in the structure are uninitialized rather than zero-initialized.

[RossBencina]

Phil and I discussed the struct version check. Phil raised the point that the check will fail if a newer version of the struct is passed even if it does not use new features. Something to think about.

Given that I don't think we have a clear and stable global policy on struct version management I think the way that the version check is being handled in this PR is fine to merge as-is even if it's not perfect.

[RossBencina]

Please fix the trailing whitespace issues. Maybe consider installing an editor plugin to strip trailing whitespace on save.

Run python ./pa_whitelint.py
error: src/hostapi/asio/pa_asio.cpp(1884) trailing whitespace:
b'            '
error: src/hostapi/asio/pa_asio.cpp(1889) trailing whitespace:
b'            '
error: src/hostapi/asio/pa_asio.cpp(3219) trailing whitespace:
b'        '
error: src/hostapi/asio/pa_asio.cpp(3221) trailing whitespace:
b'    '
error: src/hostapi/asio/pa_asio.cpp(3231) trailing whitespace:
b'    '
error: src/hostapi/asio/pa_asio.cpp(3249) trailing whitespace:
b'    '
error: src/hostapi/asio/pa_asio.cpp(3256) trailing whitespace:
b'    '
error: src/hostapi/asio/pa_asio.cpp(3358) trailing whitespace:
b'    '
error: src/hostapi/asio/pa_asio.cpp(3361) trailing whitespace:
b'    '
error: include/pa_asio.h(92) trailing whitespace:
b'    '
error: include/pa_asio.h(102) trailing whitespace:
b'            '
error: include/pa_asio.h(107) trailing whitespace:
b'    '
error: include/pa_asio.h([11](https://github.com/PortAudio/portaudio/actions/runs/8013040990/job/21926914230?pr=519#step:4:12)0) trailing whitespace:
b'        '
error: include/pa_asio.h(115) trailing whitespace:
b'    '
error: include/pa_asio.h(118) trailing whitespace:
b'        '
error: include/pa_asio.h([12](https://github.com/PortAudio/portaudio/actions/runs/8013040990/job/21926914230?pr=519#step:4:13)3) trailing whitespace:
b'    '
error: include/pa_asio.h(126) trailing whitespace:
b'        '
error: include/pa_asio.h([13](https://github.com/PortAudio/portaudio/actions/runs/8013040990/job/21926914230?pr=519#step:4:14)0) trailing whitespace:
b'    paAsioResyncRequest     = 4, '
error: include/pa_asio.h(131) trailing whitespace:
b'    '
error: include/pa_asio.h(135) trailing whitespace:
b'        '
error: include/pa_asio.h([14](https://github.com/PortAudio/portaudio/actions/runs/8013040990/job/21926914230?pr=519#step:4:15)0) trailing whitespace:
b'    '
error: include/pa_asio.h(144) trailing whitespace:
b' '
error: include/pa_asio.h(147) trailing whitespace:
b' '
error: include/pa_asio.h([15](https://github.com/PortAudio/portaudio/actions/runs/8013040990/job/21926914230?pr=519#step:4:16)0) trailing whitespace:
b' '
error: include/pa_asio.h(1[53](https://github.com/PortAudio/portaudio/actions/runs/8013040990/job/21926914230?pr=519#step:4:54)) trailing whitespace:
b' '
error: include/pa_asio.h(1[56](https://github.com/PortAudio/portaudio/actions/runs/8013040990/job/21926914230?pr=519#step:4:57)) trailing whitespace:
b'    '

[EvanBalster]

Phil and I discussed the struct version check. Phil raised the point that the check will fail if a newer version of the struct is passed even if it does not use new features. Something to think about.

Given that I don't think we have a clear and stable global policy on struct version management I think the way that the version check is being handled in this PR is fine to merge as-is even if it's not perfect.

I noticed this. I was copying the behavior of the previous code (the current library will fail any version other than 1). At your discretion I'd be happy to modify the validation check to facilitate forward compatibility.

[EvanBalster]

I went ahead and implemented the forward-compatibility as it seems unlikely to cause problems compared to the alternative. Implementors may need a fallback to deal with the possibility of an older portaudio DLL that rejects non-1 versions though. 0308b1e

[philburk]

The CI is still finding some trailing white space.

[EvanBalster]

At Ross' suggestion I've split off #885 which fixes a bug in stream info validation. 885 should be merged before this PR; otherwise it won't be possible to use ASIO messages without also specifying a channel mapping.

[RossBencina]

#885 is merged now

[RossBencina]

This seems like incomplete code (same in the sampleRateChanged handler). Is there a rationale for dropping these messages?

To me, it seems that if we're going to pass through driver messages, we should pass through all of them. All it would take would be to stash the required callback data for use here even before the stream is fully created.

[EvanBalster]

It has been quite a while since I visited this code. My faint recollection is that some ASIO drivers were generating spurious ResetRequest messages while opening the stream, but I'd need to look into it.