[portaudio:macos] avoid resampling when talking to audio hardware #4477

dyfer · 2019-07-07T20:22:21Z

Purpose and Motivation

In the process of working on #4009, I discovered CoreAudio-specific flags in PortAudio, that make it run on macOS with settings tuned for pro-audio applications. What it means in practice is that now PortAudio will ask the audio device to run at the requested sample rate, instead of allowing CoreAudio to resample. This matches scsynth's behavior with the native CoreAudio driver.

By default it will only benefit supernova on macOS (which uses PortAudio), however it is possible to build scsynth with PortAudio as well. ~~so the changes were applied for both scsynth and supernova.~~ The changes are applied to a common file that can be used by both scsynth and supernova.

~~EDIT: this will be re-worked after #4614 is merged.~~ This is ready for review.

Types of changes

New feature

To-do list

Code is tested
All tests are passing (audio is running)
This PR is ready for review

mossheim

IIUC lines 364-403 in the scsynth file are nearly equivalent to lines 152-191 in the supernova file; we should combine them. the only difference I see is supernova clips the requested input and output channels to the advertised max ins and outs of the device. it's probably better if both programs behave the same here; I would be in favor of removing that clipping behavior for supernova since it offers a good opportunity to catch user error -- this check can be performed earlier and be a hard failure with an appropriate log line now.

server/scsynth/SC_PortAudio.cpp

dyfer · 2019-07-13T00:38:23Z

IIUC lines 364-403 in the scsynth file are nearly equivalent to lines 152-191 in the supernova file; we should combine them. the only difference I see is supernova clips the requested input and output channels to the advertised max ins and outs of the device. it's probably better if both programs behave the same here; I would be in favor of removing that clipping behavior for supernova since it offers a good opportunity to catch user error -- this check can be performed earlier and be a hard failure with an appropriate log line now.

Scsynth seems to clip that as well in lines 347 and 357, right? For a moment I though that this was related to #4029 but alas, no.

Not sure what kind of user error you're referring to - user certainly can request larger number of channels than the hardware provides (on scsynth). The extraneous channels are just not routed to hardware. This is a very common use case AFAICT.

mossheim · 2019-07-15T18:43:58Z

Scsynth seems to clip that as well in lines 347 and 357, right? For a moment I though that this was related to #4029 but alas, no.

Yes! Thanks for pointing that out.

Not sure what kind of user error you're referring to - user certainly can request larger number of channels than the hardware provides (on scsynth). The extraneous channels are just not routed to hardware. This is a very common use case AFAICT.

Ah okay, never mind then :)

mossheim · 2019-07-15T20:24:58Z

Btw @dyfer I believe you can add labels to your PRs now, can you please make sure to do that going forward? Thanks :)

dyfer · 2019-07-16T00:24:27Z

Btw @dyfer I believe you can add labels to your PRs now, can you please make sure to do that going forward? Thanks :)

Sure! Sorry, I haven't noticed I can do that now.

dyfer · 2019-07-16T00:27:39Z

So to move forward with this, should I combine the section as outlined in the previous comment? Should that code go into an common header file?

mossheim · 2019-07-16T01:17:09Z

So to move forward with this, should I combine the section as outlined in the previous comment? Should that code go into an common header file?

Yes please; common/ is a good folder for now I think? If we started combining more code between the server implementations I would want to think harder about where files like that should go. (server/common?)

dyfer · 2019-07-16T05:47:16Z

Yes please; common/ is a good folder for now I think? If we started combining more code between the server implementations I would want to think harder about where files like that should go. (server/common?)

Hm, I see many things in common/ but some in include/server/... which one is better?

BTW isn't this a little messy ATM?

mossheim · 2019-07-16T15:39:59Z

include is for the public facing API, this is internal so should go in common.

what exactly is messy? i probably agree, just don't know what "this" refers to ;)

dyfer · 2019-07-16T16:43:29Z

include is for the public facing API, this is internal so should go in common.

Ah, makes sense. This makes it less messy :)

what exactly is messy? i probably agree, just don't know what "this" refers to ;)

It seems like common has things related mostly to sclang, but also some related to server, is that right? This is what I was mostly referring to.

mossheim · 2019-12-22T23:19:06Z

It seems like common has things related mostly to sclang, but also some related to server, is that right? This is what I was mostly referring to.

i think so?

what's the status of this PR now? is it depending on anything else or is it ready for review?

dyfer · 2019-12-24T18:44:34Z

@brianlheim this PR is relevant for PortAudio backend on macOS, which is by default used only by supernova, and not scsynth. However, supernova's PortAudio backend is not using this implementation yet, see #4476.
I would say that It is technically ready to review, but this code will only be used if one builds with PortAudio backend explicitly on macOS (which would make scsynth use PortAudio and thus test this PR). Whether this is used by scsynth or supernova should not make a difference for testing purposes, AFAIU.

mossheim · 2019-12-24T19:42:14Z

common/SC_PaUtils.cpp

@@ -156,6 +161,11 @@ PaStreamParameters MakePaStreamParameters(int device, int channelCount, double s
    streamParams.channelCount = channelCount;
    streamParams.sampleFormat = fmt;
    streamParams.suggestedLatency = suggestedLatency;
+#ifdef __APPLE__
+    PaMacCore_SetupStreamInfo(&macInfo, paMacCorePro);


do we really want this called on every invocation of this function?

Maybe not, but it doesn't seem to hurt it. Should this be changed?

just wondering. i'm a fan of keeping things as close to where they're used as possible. can you move macInfo inside this function as a static variable? then a reader doesn't need to jump up to the top of the file to find out what macInfo is.

I think it needs to be the same macInfo instance (for both times this function is called for input and output device), otherwise I was getting crashes. But I can investigate again...

static would make it the same variable across calls and persistent for the life of the application. if it were a non-static local variable, it would be deallocated after the function call and you would probably get a segmentation fault later from the dangling pointer you stored in streamParams

see https://stackoverflow.com/questions/5033627/static-variable-inside-of-a-function-in-c for a quick explanation

perfect! Thank you for that clarification, this is very helpful. I'll try this shortly.

dyfer · 2019-12-27T19:52:37Z

@brianlheim how does it look now?

mossheim · 2019-12-27T20:33:12Z

good, thanks!

mossheim suggested changes Jul 12, 2019

View reviewed changes

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

mossheim added os: macOS comp: scsynth labels Jul 15, 2019

dyfer mentioned this pull request Jul 25, 2019

[scsynth portaudio] allow independent input output devices #4475

Merged

5 tasks

dyfer mentioned this pull request Oct 9, 2019

[scsynth:portaudio] move helper functions #4614

Merged

3 tasks

dyfer force-pushed the topic/portaudio-macos-avoid-resampling branch from 7676022 to ef2d618 Compare November 27, 2019 20:22

dyfer added the comp: supernova label Nov 27, 2019

mossheim reviewed Dec 24, 2019

View reviewed changes

portaudio-macos: avoid resampling by default

7638b9e

dyfer force-pushed the topic/portaudio-macos-avoid-resampling branch from ef2d618 to 7638b9e Compare December 27, 2019 19:52

mossheim approved these changes Dec 27, 2019

View reviewed changes

mossheim merged commit 1fc363d into supercollider:develop Dec 27, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[portaudio:macos] avoid resampling when talking to audio hardware #4477

[portaudio:macos] avoid resampling when talking to audio hardware #4477

dyfer commented Jul 7, 2019 •

edited

Loading

mossheim left a comment

dyfer commented Jul 13, 2019

mossheim commented Jul 15, 2019

mossheim commented Jul 15, 2019

dyfer commented Jul 16, 2019 •

edited

Loading

dyfer commented Jul 16, 2019

mossheim commented Jul 16, 2019

dyfer commented Jul 16, 2019 •

edited

Loading

mossheim commented Jul 16, 2019

dyfer commented Jul 16, 2019

mossheim commented Dec 22, 2019

dyfer commented Dec 24, 2019 •

edited

Loading

mossheim Dec 24, 2019

dyfer Dec 24, 2019

mossheim Dec 24, 2019

dyfer Dec 25, 2019 •

edited

Loading

mossheim Dec 25, 2019

dyfer Dec 25, 2019

dyfer commented Dec 27, 2019

mossheim commented Dec 27, 2019

[portaudio:macos] avoid resampling when talking to audio hardware #4477

[portaudio:macos] avoid resampling when talking to audio hardware #4477

Conversation

dyfer commented Jul 7, 2019 • edited Loading

Purpose and Motivation

Types of changes

To-do list

mossheim left a comment

Choose a reason for hiding this comment

dyfer commented Jul 13, 2019

mossheim commented Jul 15, 2019

mossheim commented Jul 15, 2019

dyfer commented Jul 16, 2019 • edited Loading

dyfer commented Jul 16, 2019

mossheim commented Jul 16, 2019

dyfer commented Jul 16, 2019 • edited Loading

mossheim commented Jul 16, 2019

dyfer commented Jul 16, 2019

mossheim commented Dec 22, 2019

dyfer commented Dec 24, 2019 • edited Loading

mossheim Dec 24, 2019

Choose a reason for hiding this comment

dyfer Dec 24, 2019

Choose a reason for hiding this comment

mossheim Dec 24, 2019

Choose a reason for hiding this comment

dyfer Dec 25, 2019 • edited Loading

Choose a reason for hiding this comment

mossheim Dec 25, 2019

Choose a reason for hiding this comment

dyfer Dec 25, 2019

Choose a reason for hiding this comment

dyfer commented Dec 27, 2019

mossheim commented Dec 27, 2019

dyfer commented Jul 7, 2019 •

edited

Loading

dyfer commented Jul 16, 2019 •

edited

Loading

dyfer commented Jul 16, 2019 •

edited

Loading

dyfer commented Dec 24, 2019 •

edited

Loading

dyfer Dec 25, 2019 •

edited

Loading