[scsynth portaudio] allow independent input output devices #4475

dyfer · 2019-07-07T20:09:27Z

Purpose and Motivation

This is a cleaned up version of #4009 focused on scsynth and documentation. See #4476 for supernova-related PR. Fixes #3990.

On Windows, scsynth was not able to select separate input and output devices (it accepted only single device name and tried to use it as both input and output). This essentially prevented manual device selection for anything other than soundcards with ASIO drivers.

This PR allows independent device selection, provides documentation and improves automatic device selection with PortAudio.

Thanks to @jamshark70 @sonoro1234 and others who helped with the original PR. This is the summary of the discussion so far:

the documentation was reviewed and seemed to be received favorably
there has been a lot of back and forth w/r/t which API to use for selecting the default device
- ~~there seemed to be a consensus that ASIO should be selected by default~~
- ~~if ASIO device is not available,~~
- MME device is selected by default
  - this is the same behavior as before - more testing showed that sometimes non-working ASIO drivers are present on Windows system by default preventing server to boot
there have been multiple tests done and booting the server is now much more reliable than before
an issue of device names with unicode characters has been raised; this PR does not address that (see below)

Types of changes

Bug fix: allow scsynth with PortAudio backend to select separate input and output devices
~~New feature~~ Bug fix: improve default device selection
- if one device is specified, select another one from the same API
- if no devices are specified, try selecting an ASIO device first
- if no devices are specified, select a MME device (as before)
- provide more verbose output for all choices made
- provide better error reporting, including e.g. catching mismatched sample rate between input and output devices
Bug fix: treat empty strings as "invalid device" (triggering default device selection); previously empty string was causing scsynth to choose the last device on the list.
Bug fix: remove hardcoded default sample rate 44100 on scsynth with PortAudio on Windows (it will boot to hardware's sample rate by default)
- this resolves booting when the API doesn't allow changing SR and SR was not specified on the command line
- ASIO still boots to 44100 by default, which seems to be "by design" in PortAudio
Documentation: Provide new document describing audio device selection, including overview of different APIs on Windows

To-do list

Code is tested
All tests are passing
Updated documentation
This PR is ready for review
commit history cleanup (to be done after the review and before merging)

Unresolved issues

This PR does not solve setting device names for devices with unicode characters. Character encoding seems to be a larger issue. My work so far on it was moved to #4479. Partial names however are matched as before.

sonoro1234 · 2019-07-08T16:40:16Z

Default selection working for ASIO and MME
-o 0 and -i 0 works
Not matching samplerates works but there is a crash after could not initialize audio
(I think not related to this PR as also happens when hitting Ctrl+C)

Requested devices:
  In (matching device found):
  - Windows WASAPI : Mic interno (IDT High Definition Audio CODEC)
  Out (matching device found):
  - Windows WASAPI : Line 1/2 (4- M-Audio Fast Track Pro)


Requested devices Mic interno (IDT High Definition Audio CODEC) and Line 1/2 (4- M-Audio Fast Track Pro) use different sample
 rates. Please set matching sample rates in the Windows Sound Control Panel and try again.
SC_PortAudioDriver: PortAudio failed at Pa_OpenStream with error: 'Invalid sample rate'
could not initialize audio.

Now debugging crash

dyfer · 2019-07-08T16:46:18Z

@sonoro1234 thanks, this also had the bug you found #4476 (comment) - fixed now.

server/scsynth/SC_PortAudio.cpp

sonoro1234 · 2019-07-08T17:23:40Z

Cant debug this because only happens with my WASAPI devices but I can reach them compiling with mingw

dyfer · 2019-07-08T17:32:05Z

Could you give a full scsynth output for the WASAPI issue?

sonoro1234 · 2019-07-08T17:35:01Z

It is not a WASAPI issue. The issue is a crash when not possible to boot.
I got one with ASIO and samplerate 11200

Requested sample rate 11200.000000 for devices Fast Track Pro ASIO and Fast Track Pro ASIO is not supported.
SC_PortAudioDriver: PortAudio failed at Pa_OpenStream with error: 'Invalid sample rate'
could not initialize audio.
[Thread 4388.0xa80 exited with code 1]
[Thread 4388.0x14ec exited with code 1]
[Thread 4388.0x16f0 exited with code 1]
[Thread 4388.0x17d0 exited with code 1]
[Thread 4388.0x13a4 exited with code 1]
[Thread 4388.0x14cc exited with code 1]
[Thread 4388.0xa58 exited with code 1]
terminate called without an active exception

This application has requested the Runtime to terminate it in an unusual way.
Please contact the application's support team for more information.
[Inferior 1 (process 4388) exited with code 01]
(gdb) thread apply all bt
(gdb)

There is not information in gdb. I think that it is related to exception handling in scsynth.
May be in
https://github.com/supercollider/supercollider/blob/develop/server/scsynth/SC_World.cpp#L426
instead of return 0 an error should be thrown? @brianlheim

dyfer · 2019-07-08T17:39:27Z

Are you requesting sample rate 11200? I've seen issues booting to SR < 44100 (like 8000) and I don't know what's the problem. Does this happen with the regular build as well?

sonoro1234 · 2019-07-08T17:42:28Z

Are you requesting sample rate 11200? I've seen issues booting to SR < 44100 (like 8000) and I don't know what's the problem. Does this happen with the regular build as well?

This is not a problem, my ASIO card wont boot with 11200, I just asked for this impossible samplerate to debug the crash after could not initialize audio.
This crash has nothing related to your PR and everything is working as expected.

dyfer · 2019-07-12T00:22:23Z

Note on selecting ASIO soundcard by default: I had an opportunity to do more tests on other systems. I found at least one other Windows laptop that had Realtek ASIO driver present (for the internal soundcard), and booting to this did not work.

Therefore, I removed selecting ASIO device by default.

This PR still adds more checks that help with device selection, but the default behavior (selecting default MME device) is the same as before.

mossheim

thanks @dyfer!!

server/scsynth/SC_PortAudio.cpp

include/server/SC_WorldOptions.h

HelpSource/Reference/AudioDeviceSelection.schelp

server/scsynth/scsynth_main.cpp

dyfer · 2019-07-12T23:47:54Z

Thanks for the review @brianlheim ! I'll work on implementing changes.

Also, Travis CI is failing on an arithmetic unit test... not sure what's that about...

mossheim · 2019-07-12T23:50:12Z

Thanks for the review @brianlheim ! I'll work on implementing changes.

Thanks! Let me know if you have any more questions or if any comments were unclear

Also, Travis CI is failing on an arithmetic unit test... not sure what's that about...

Not sure either, I just restarted the job. Don't worry about it.

dyfer · 2019-07-12T23:56:12Z

Thanks! Let me know if you have any more questions or if any comments were unclear

Thanks! Things are rather clear, I'll just need a bit of time to work on this and will ask you if I have questions along the way.

dyfer · 2019-07-25T00:43:22Z

Hi @brianlheim, I tried to address your comments. Please let me know how this looks now.

Also, I created a function to get stream parameters to simplify some of the new code. Should I also use it in the DriverSetup section (which would be a work towards implementing your other suggestion) or should that be left for #4477?

mossheim

thanks marcin! this is not as comprehensive a review as i wanted to give right now but i think there should still be plenty to work with. i will try to respond to your other comments asap.

server/scsynth/SC_PortAudio.cpp

dyfer · 2019-08-01T20:49:26Z

Thank you for your (partial?) review Brian!

I also have a question regarding IOType here.

Which is better:

bool isInput; //if matchingDeviceType is hypothetically neither Input nor Output, this stays unititialized
if (matchingDeviceType == IOType::Input)
    isInput = true;
else if (matchingDeviceType == IOType::Output)
    isInput = false;

if (isInput) {
    // do on input
} else {
    // do on output
}
fprintf(stdout, "this is %s\n", (isInput ? "input" : "output"));

vs

bool isInput = false; //we don't check whether it's IOType::Output or something else
if (matchingDeviceType == IOType::Input)
    isInput = true; 

if (isInput) {
    // do on input
} else {
    // do on output
}
fprintf(stdout, "this is %s\n", (isInput ? "input" : "output"));

vs

if (matchingDeviceType == IOType::Input) {
    // do on input
} else if (matchingDeviceType == IOType::Output) {
    // do on output
}
fprintf(stdout, "this is %s\n", ((matchingDeviceType == IOType::Input) ? "input" : "output")); //we don't check whether it's IOType::Output or something else

mossheim

OK, I took some more time to digest what was going on and I think this should be my last round of comments. Thanks again for your patience. When a comment is more a light recommendation than a request for changes, I've marked it with "(suggestion)" at the start.

Two general suggestions:

add documentation for functions (e.g., CheckPaDevices) in the header, not the cpp file.
I don't know if you need "Pa" in the name of every function since it's already obvious these functions operate on PortAudio streams and objects. I realize you may have been trying to follow precedent with GetPaDeviceFromName but for me it has the effect of making it harder to understand when a function call is to something in PortAudio's API vs part of the class interface.

To answer your earlier question:

Also, I created a function to get stream parameters to simplify some of the new code. Should I also use it in the DriverSetup section (which would be a work towards implementing your other suggestion) or should that be left for #4477?

Leave it for #4477 :)

I also have a question regarding IOType here.
Which is better:

I answered your question in an inline comment here

server/scsynth/SC_PortAudio.cpp

jrsurge · 2019-08-04T13:32:39Z

Server crash with ASIO:

Thanks for spotting this!

If the user specifies a correct ASIO device, the server has to choose that device for both in and out, regardless of the default.

I tried correcting it. If only one device is specified, and it's a full duplex ASIO device, it will be used for both input and output. Could you test on your setup if the fix works correctly?

Thanks for the fix! This now works as expected

dyfer · 2019-08-04T19:26:04Z

I found many minor grammar/style changes for the new Audio Device Selection helpfile. It was easier for me to change them myself, rather than adding dozens of review comments.

Thank you for the patches @jamshark70!

Question: you replaced "macOS" with "MacOS" in your patches. Are you sure this is best? It seems that Apple stylizes it "macOS" everywhere, including beginning of the sentence, and I was trying to follow that. Example:

I don't have a very strong opinion, but I wanted to clarify my conscious choice here.

edit: after the dev meeting the consensus was that we should keep it as "macOS" in all cases. Thanks again for all the corrections!

geoffroymontel · 2019-08-04T21:01:34Z

I've just tried the build with both my Steinberg MR816 ASIO card and Reaper's ReaRoute ASIO and both work well, no regression ! thanks !

dyfer · 2019-08-04T21:45:45Z

Thanks for testing @geoffroymontel
If I could ask you to test one more thing - what happens if you don't specify sample rate in server options and boot to ASIO? The server should boot to the device's SR, is that the case? Is it different from the previous behavior?

jamshark70 · 2019-08-05T01:56:13Z

Question: you replaced "macOS" with "MacOS" in your patches. Are you sure this is best? It seems that Apple stylizes it "macOS" everywhere, including beginning of the sentence, and I was trying to follow that.

No objection to changing it back to Apple/apple(?) style.

geoffroymontel · 2019-08-05T06:28:14Z

If I could ask you to test one more thing - what happens if you don't specify sample rate in server options and boot to ASIO? The server should boot to the device's SR, is that the case? Is it different from the previous behavior?

I've tried the following

setting the MR816 to 96 khz
starting SC with s.options.device = "ASIO : Yamaha Steinberg FW ASIO";
SC would change the MR816 back to 44khz

Booting with:
  In: ASIO : Yamaha Steinberg FW ASIO
  Out: ASIO : Yamaha Steinberg FW ASIO
SCDoc: Indexed 1991 documents in 1.33 seconds
  Sample rate: 44100.000
  Latency (in/out): 0.004 / 0.012 sec
SC_AudioDriver: sample rate = 44100.000000, driver's block size = 64

The behaviour is the same on 3.10.2 on my Windows box

dyfer · 2019-08-05T07:21:29Z

@geoffroymontel thanks for testing. I'll check if there's a reason. Otherwise at least it's not a regression.

dyfer · 2019-08-05T16:34:26Z

update on ASIO default sample rate:

It seems this is a PortAudio bug. PortAudio obtains defaultSampleRate by checking whether the device supports a given SR agains a list of known SRs and takes the first match. I have tried to replace that code with

ASIOError asioError = ASIOGetSampleRate(&deviceInfo->defaultSampleRate);

and that seemed to work (server booted to device's sample rate), but I don't know enough to propose a full fix in PA for that. I have posted to their list so maybe it will be fixed upstream at some point.

edit:
It seems that PortAudio has a different understanding of what the "default sample rate" is than what I assumed. It seems that the current behavior is not likely to change:
https://lists.columbia.edu/pipermail/portaudio/2019-August/001893.html

server/scsynth/SC_PortAudio.cpp

dyfer · 2019-08-09T18:10:31Z

@brianlheim please let me know when you think this is ready for me to rebase.

@jamshark70 could you take a look whether your comments were properly addressed?

mossheim

Thanks for your work on this and patience @dyfer .

@jamshark70 please review when you have time

Windows/Linux: list/use APIs in device names Improve device selection logic Windows: use device's sample rate by default

create new reference: AudioDeviceSelection.schelp update and move Aggregate Device creation (macOS) to the new document link the new document from other help files

dyfer · 2019-08-10T22:35:45Z

This PR's commit history is now cleaned up (I rebased and squashed relevant commits).

Pending @jamshark70 's approval w/r/t #4475 (comment) it is ready to merge.

mossheim · 2019-08-15T04:34:35Z

@jamshark70 no pressure but FYI this PR (and your approval of it) is what's currently blocking the 3.10.3 release.

jamshark70 · 2019-08-15T06:57:58Z

Sorry for the delay -- I hadn't realized that I was holding up a release.

I took the liberty of merging -- I hope that wasn't out of turn, but I recall this PR has already been extensively, exhaustively reviewed and if my approval was the last thing needed, seemed appropriate.

Much-needed feature!

nhthn · 2019-08-16T02:52:52Z

thanks @jamshark70 !!!

sonoro1234 · 2019-08-18T09:56:39Z

@dyfer Congratulations for such a long work!!
supernova version should be done to have compatible command line and behaviour!!!

This was referenced Jul 7, 2019

supernova: portaudio improvements #4476

Merged

[scsynth] [supernova] [PortAudio] [Windows] separate input and output devices, better default selection #4009

Closed

dyfer requested review from mossheim and jamshark70 July 7, 2019 21:30

jamshark70 mentioned this pull request Jul 8, 2019

[scsynth:windows] support unicode in argv #4479

Merged

2 tasks

sonoro1234 reviewed Jul 8, 2019

View reviewed changes

server/scsynth/SC_PortAudio.cpp Outdated Show resolved Hide resolved

sonoro1234 reviewed Jul 8, 2019

View reviewed changes

server/scsynth/SC_PortAudio.cpp Outdated Show resolved Hide resolved

mossheim added the comp: scsynth label Jul 11, 2019

sonoro1234 mentioned this pull request Jul 12, 2019

scsynth crash when unable to boot audio #4486

Open

mossheim suggested changes Jul 12, 2019

View reviewed changes

jrsurge self-requested a review July 14, 2019 19:14

mossheim reviewed Aug 1, 2019

View reviewed changes

dyfer force-pushed the topic/scsynth-portaudio-input-output-devices branch from 1fee251 to fb991c5 Compare August 1, 2019 20:36

mossheim suggested changes Aug 2, 2019

View reviewed changes

nhthn added this to the 3.10.3 milestone Aug 4, 2019

sonoro1234 mentioned this pull request Aug 5, 2019

[supernova] sound input fails if number of requested output channels exceeds soundcard capabilities #4029

Closed

mossheim reviewed Aug 9, 2019

View reviewed changes

server/scsynth/SC_PortAudio.cpp Outdated Show resolved Hide resolved

jrsurge approved these changes Aug 9, 2019

View reviewed changes

mossheim approved these changes Aug 10, 2019

View reviewed changes

dyfer force-pushed the topic/scsynth-portaudio-input-output-devices branch from 8ee9913 to b315166 Compare August 10, 2019 22:28

dyfer added 2 commits August 10, 2019 15:30

[scsynth:portaudio] Allow independent input/output device settings

2e3c84e

Windows/Linux: list/use APIs in device names Improve device selection logic Windows: use device's sample rate by default

[classlib:help] provide a reference for selecting audio device

0316193

create new reference: AudioDeviceSelection.schelp update and move Aggregate Device creation (macOS) to the new document link the new document from other help files

dyfer force-pushed the topic/scsynth-portaudio-input-output-devices branch from b315166 to 0316193 Compare August 10, 2019 22:30

jamshark70 approved these changes Aug 15, 2019

View reviewed changes

jamshark70 merged commit 906b219 into supercollider:3.10 Aug 15, 2019

nhthn mentioned this pull request Aug 17, 2019

[enhancement] Support ServerOptions inDevice and outDevice in Windows #3990

Closed

ethancrawford mentioned this pull request Oct 26, 2019

Could not boot Sonic Pi Server sonic-pi-net/sonic-pi#2153

Closed

jamshark70 mentioned this pull request Jan 2, 2020

Server fails to boot (Windows 10) #4718

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[scsynth portaudio] allow independent input output devices #4475

[scsynth portaudio] allow independent input output devices #4475

dyfer commented Jul 7, 2019 •

edited

Loading

sonoro1234 commented Jul 8, 2019

dyfer commented Jul 8, 2019

sonoro1234 commented Jul 8, 2019

dyfer commented Jul 8, 2019

sonoro1234 commented Jul 8, 2019 •

edited

Loading

dyfer commented Jul 8, 2019

sonoro1234 commented Jul 8, 2019 •

edited

Loading

dyfer commented Jul 12, 2019 •

edited

Loading

mossheim left a comment

dyfer commented Jul 12, 2019

mossheim commented Jul 12, 2019

dyfer commented Jul 12, 2019

dyfer commented Jul 25, 2019

mossheim left a comment

dyfer commented Aug 1, 2019

mossheim left a comment •

edited

Loading

jrsurge commented Aug 4, 2019

dyfer commented Aug 4, 2019 •

edited

Loading

geoffroymontel commented Aug 4, 2019

dyfer commented Aug 4, 2019

jamshark70 commented Aug 5, 2019

geoffroymontel commented Aug 5, 2019

dyfer commented Aug 5, 2019

dyfer commented Aug 5, 2019 •

edited

Loading

dyfer commented Aug 9, 2019 •

edited

Loading

mossheim left a comment

dyfer commented Aug 10, 2019 •

edited

Loading

mossheim commented Aug 15, 2019

jamshark70 commented Aug 15, 2019

nhthn commented Aug 16, 2019

sonoro1234 commented Aug 18, 2019

[scsynth portaudio] allow independent input output devices #4475

[scsynth portaudio] allow independent input output devices #4475

Conversation

dyfer commented Jul 7, 2019 • edited Loading

Purpose and Motivation

Types of changes

To-do list

Unresolved issues

sonoro1234 commented Jul 8, 2019

dyfer commented Jul 8, 2019

sonoro1234 commented Jul 8, 2019

dyfer commented Jul 8, 2019

sonoro1234 commented Jul 8, 2019 • edited Loading

dyfer commented Jul 8, 2019

sonoro1234 commented Jul 8, 2019 • edited Loading

dyfer commented Jul 12, 2019 • edited Loading

mossheim left a comment

Choose a reason for hiding this comment

dyfer commented Jul 12, 2019

mossheim commented Jul 12, 2019

dyfer commented Jul 12, 2019

dyfer commented Jul 25, 2019

mossheim left a comment

Choose a reason for hiding this comment

dyfer commented Aug 1, 2019

mossheim left a comment • edited Loading

Choose a reason for hiding this comment

jrsurge commented Aug 4, 2019

dyfer commented Aug 4, 2019 • edited Loading

geoffroymontel commented Aug 4, 2019

dyfer commented Aug 4, 2019

jamshark70 commented Aug 5, 2019

geoffroymontel commented Aug 5, 2019

dyfer commented Aug 5, 2019

dyfer commented Aug 5, 2019 • edited Loading

dyfer commented Aug 9, 2019 • edited Loading

mossheim left a comment

Choose a reason for hiding this comment

dyfer commented Aug 10, 2019 • edited Loading

mossheim commented Aug 15, 2019

jamshark70 commented Aug 15, 2019

nhthn commented Aug 16, 2019

sonoro1234 commented Aug 18, 2019

dyfer commented Jul 7, 2019 •

edited

Loading

sonoro1234 commented Jul 8, 2019 •

edited

Loading

sonoro1234 commented Jul 8, 2019 •

edited

Loading

dyfer commented Jul 12, 2019 •

edited

Loading

mossheim left a comment •

edited

Loading

dyfer commented Aug 4, 2019 •

edited

Loading

dyfer commented Aug 5, 2019 •

edited

Loading

dyfer commented Aug 9, 2019 •

edited

Loading

dyfer commented Aug 10, 2019 •

edited

Loading