CoreAudio add support for new HAL API for applications that target Mac OS X >= 10.5

[jmelas]

This pull request addresses item #218 by using the new CoreAudio APIs while keeping compatibility with
applications that need to use the old APIs!
This is done with the following check
#if (MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_5)
#define PA_NEW_HAL 1
#else
#define PA_NEW_HAL 0
#endif
which checks if the application targets Mac OSX >= 10.5, so applications targeting Mac OS X 10.4 will use the old API.

I tried to make the changes as minimal as possible by introducing wrapper functions and #ifdefs
for two reasons:
a) easier review
b) compatibility with old API

[philburk]

Thank you so much for this Mac OS update. This will make a lot of people happy.

I see that you also cleaned up a lot of trailing white space. That's great. But unfortunately they obscure the relevant changes. Would it be possible to resubmit this pull request without the white space commit? We can do those as a later pull request. Generally it is best to separate style or format changes from real code changes.

Then we can do the detailed code review.

[jmelas]

You are welcome Phil!
OK, I've removed the commit with the spaces cleanup!

[philburk]

I checked out this fork.
./configure && make clean && make

and got these errors:

libtool: compile: gcc -c -std=c99 -O2 -Wall -pedantic -pipe -fPIC -DNDEBUG -DPA_LITTLE_ENDIAN -I./include -I./src/common -I./src/os/unix -Wno-deprecated -Werror -arch x86_64 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk -mmacosx-version-min=10.4 -DPACKAGE_NAME="" -DPACKAGE_TARNAME="" -DPACKAGE_VERSION="" -DPACKAGE_STRING="" -DPACKAGE_BUGREPORT="" -DPACKAGE_URL="" -DSTDC_HEADERS=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1 -DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1 -DHAVE_STRINGS_H=1 -DHAVE_INTTYPES_H=1 -DHAVE_STDINT_H=1 -DHAVE_UNISTD_H=1 -DHAVE_DLFCN_H=1 -DLT_OBJDIR=".libs/" -DSIZEOF_SHORT=2 -DSIZEOF_INT=4 -DSIZEOF_LONG=8 -DHAVE_CLOCK_GETTIME=1 -DHAVE_NANOSLEEP=1 -DPA_USE_COREAUDIO=1 src/hostapi/coreaudio/pa_mac_core_utilities.c -fno-common -DPIC -o src/hostapi/coreaudio/.libs/pa_mac_core_utilities.o
src/hostapi/coreaudio/pa_mac_core_utilities.c:75:5: error: use of undeclared identifier 'macErr'
macErr = AudioHardwareGetProperty(inPropertyID, ioPropertyDataSize, outPropertyData);
^
src/hostapi/coreaudio/pa_mac_core_utilities.c:87:5: error: use of undeclared identifier 'macErr'
macErr = AudioHardwareGetPropertyInfo(inPropertyID, outSize, NULL);
^
src/hostapi/coreaudio/pa_mac_core_utilities.c:104:5: error: use of undeclared identifier 'macErr'
macErr = AudioDeviceGetProperty(inDevice, inChannel, isInput, inPropertyID, ioPropertyDataSize, outPropertyData);

[philburk]

Thanks John. With your changes I can now compile using "-mmacosx-version-min=10.4".
I changed that to "-mmacosx-version-min=10.5" and verified that is is calling your new code.
That also compiles OK.

I ran pa_devs, paex_sine, paex_record, paqa_devs, paqa_errs, paqa_latency and they passed OK.
Note that paqa_devs has some problems with numChannels > 2, but not because of this PR.

[RossBencina]

Thanks for this. I haven't finished reviewing it but I just want to write down some notes here while I remember:

• Change PA_NEW_HAL to mac-specific naming convention, same for pa_ prefix on new functions
• Don't cut and paste to duplicate almost identical code between new and old listener callbacks -- should share the same code.

I'll explain more later, and perhaps Phil and I can advise for how to improve things.

[jmelas]

About #ifdefs and duplication of code:
Do you think it makes sense to drop support for OS X 10.4? If yes then it's trivial to remove old code as it is now.
If not, indeed I will have to introduce new functions to hide the duplications!

[Be-ing]

I am writing this comment on a 9 year old Macbook Air stuck on macOS 10.13 because Apple won't let me update it to a newer OS. macOS 10.04 was released in 2005. So I say, yes, drop the legacy API. Probably very few machines are running it anymore and probably lots of other libraries used in any recent application wouldn't be able to run on them.

[Be-ing]

@philburk @RossBencina would you be okay with removing the old API now?

[philburk]

I tried to find statistics for the current share of machines running 10.4 and it is off the bottom of the chart for everything I could find. I imagine that people running 10.4 would not expect newly built apps to still run on their machine.
I think most people with old machine have ways to find old versions of apps that will run.

For a new apps built today, even if PortAudio supported 10.4, the app would probably use other APIs that do not run on on 10.4.

10.4 came out in 2005 and machines built in 2005 probably upgraded to later versions.

So I think it is reasonable to drop support for 10.4 and to just note that if people want to support 10.4 then they can use an old stable release or a snapshot.

[RossBencina]

I agree to dropping support for 10.4. If we can support 10.6 and later that is good enough. That said, the newer API is kinda sucky compared to the old one, so perhaps keeping the wrapper functions in this patch still makes sense?

[RossBencina]

Further to my earlier comments: This may no longer be relevant, but our naming conventions are documented here:

https://github.com/PortAudio/portaudio/wiki/ImplementationStyleGuidelines

With respect to the added functions prefixed pa_: static functions should have no special prefix/suffix and module-specific globally visible functions should, in this case, be prefixed with PaMacCore_.

The preprocessor symbol naming convention does not appear to be clearly documented but in this case I'd say PA_NEW_HAL should instead be PA_MAC_CORE_NEW_HAL (since this is a symbol specific to pa mac core, not to PA as a whole.

[jmelas]

Thank you all for commenting. I've just dropped support for Tiger and also renamed all helper functions to PaMacCore_.

[philburk]

The code looks great. Thanks for making all these changes. But bad news...

I followed the instructions above to try the new code:

git checkout -b jmelas-bigsur master
git pull https://github.com/jmelas/portaudio.git bigsur
./configure && make clean && make

bin/paex_sine works OK
bin/paex_write_sine works OK
bin/paex_read_write_wire fails with this message and then hangs:

Wire on. Will run 10 seconds.
||PaMacCore (AUHAL)|| Error on line 2409: err=''insz'', msg=Unknown Error

bin/patest_wire has a similar failure

If I switch back to the master branch then they work fine.
So there seems to be a problem with INPUT.

[sharonlev]

+1 awaiting for this merge. (REF: spatialaudio/python-sounddevice#299)

[Be-ing]

I propose making a new release as soon as this is merged.

[Be-ing]

@RossBencina is there anything left to do here?

[mechanical-girl]

Similarly waiting for this merge and release!

[Be-ing]

ping @RossBencina this critical bug fix has been sitting for over two weeks

[xasopheno]

Awesome this was merged. Does this being merged mean that it's available via something like brew install portaudio or do I need to wait for a release. Thanks! D

[Be-ing]

I will update vcpkg as soon as the new release is made.

[xasopheno]

I will update vcpkg as soon as the new release is made.

Is there somewhere I track progress towards new release? I'm grateful that folks are working on this. I'm blocked until a new release a cut.

[meme]

Awesome this was merged. Does this being merged mean that it's available via something like brew install portaudio or do I need to wait for a release. Thanks! D

You can brew install portaudio --HEAD

[philburk]

I found vcpkg at https://vcpkg.readthedocs.io/en/latest/And PortAudio at:    https://github.com/microsoft/vcpkg/tree/master/ports/portaudio
I was not aware of this. We should document this on the Wiki.

You can brew install portaudio --HEAD

Does that grab the current HEAD from GitHub? Or the old head from Assembla?

[meme]

Does that grab the current HEAD from GitHub? Or the old head from Assembla?

HEAD from GitHub. I used that command to get this fix on my host.