PortAudio / portaudio

PortAudio is a cross-platform, open-source C language library for real-time audio input and output.
Other
1.44k stars 300 forks source link

CMake expected ASIOSDK location issues [regression] #518

Open EvanBalster opened 3 years ago

EvanBalster commented 3 years ago

When using CMake to generate portaudio with ASIO support, the ASIO SDK directory must be a sibling of the portaudio repository. This is not documented in the ASIO readme: https://github.com/PortAudio/portaudio/blob/master/src/hostapi/asio/ASIO-README.txt.

containing_folder/portaudio/src/hostapi/asio/ASIOSDK -- Required location for non-CMake builds containing_folder/portaudio -- CMake required location (undocumented)

I had to investigate this CMake file to suss it out: https://github.com/portaudio/portaudio/blob/master/cmake_support/FindASIOSDK.cmake

To Reproduce

Expected behavior

CMake build instructions should be documented at least briefly in https://github.com/PortAudio/portaudio/blob/master/src/hostapi/asio/ASIO-README.txt.

Recommend CMake should support one or more of the following:

Actual behavior

CMake reports missing values of ASIOSDK_ROOT_DIR and ASIOSDK_INCLUDE_DIR. Defining these as CMake environment variables has no effect. The CMake configuration does not provide a a way to provide these values.

Desktop:

Be-ing commented 3 years ago

461 searches the portaudio repository root and the directory above it for the SDK ZIP file. Alternatively the path can be explicitly specified with the CMake option ASIO_SDK_ZIP_PATH.

EvanBalster commented 3 years ago

Historical methods for building Portaudio with ASIO support involved placing the ASIO SDK in src/hostapi/asio. Although it's a strange location, it is still indicated by the documentation and so I would recommend adding it to the search directories.

RossBencina commented 2 years ago

I agree with your "Expected behavior" @EvanBalster, the supported location(s) should be consistent between the docs, the legacy MSVC build file and CMake. On the other hand, I agree with @Be-ing about the logical locations for the SDK.

It's not clear from @Be-ing's comment whether the SDK is expected to be zipped or unzipped. But certainly an unzipped option should be supported since it is common to patch bugs in the SDK.

Be-ing commented 2 years ago

The issue is that the documentation has not been updated yet. I don't think it's worth working on the docs until the new build system is made for the C++ bindings, but that can't be done until someone makes the new repo (which I have asked for several times).

RossBencina commented 2 years ago

Related #662

RossBencina commented 2 years ago

This particular issue is unrelated to C++ bindings and can be resolved independently of C++ bindings by updating the CMake script to automatically check both preferred and legacy locations for the ASIO SDK.

RossBencina commented 2 years ago

From what I've gleaned from this thread so far, the following are required to resolve this issue (@EvanBalster could you confirm that this list is sufficient, or did I miss something?)

EvanBalster commented 2 years ago

If I recall correctly, the issue I previously encountered was CMake looking out-of-tree for path ../ASIOSDK relative to the portaudio root. Presumably on the assumption that portaudio is checked out to a folder full of libraries...

So to answer @RossBencina... That list looks sufficient, so long as the priority order makes sense. I would say:

  1. User-supplied CMake Configuration Variable
  2. Environment Variable
  3. Autodetect (src/hostapi/asio/ASIOSDK or ../ASIOSDK)
RossBencina commented 2 years ago

The path forward seems clear. We're keen to accept a patch from whoever wants to fix this.

dmitrykos commented 8 months ago

This issue is not relevant anymore as ASIO dependency is downloaded automatically into CMake build folder and used from there by the build process. Tested with MinGW an MSVC, see closed ticket #837 and my last comment.

In case of custom ASIO dependency location it is a task of the user to modify own CMake configuration scripts and not really an issue of PA CMake build system. This issue needs to be closed.

RossBencina commented 8 months ago

This issue is not relevant anymore as ASIO dependency is downloaded automatically

I disagree. For a few reasons:

  1. The CMake code does still allow the use of a local zip file, it just does not search the old well-known location (hence it is a regression that needs to be fixed: the old location should be searched!).

  2. Manually managing the exact SDK version (and hence the ASIO SDK download) is essential for producing reproducible builds. In most cases you do not want to just blindly pull the latest SDK version from the Steinberg site. Even our CI code downloads a specific version and caches it.

  3. The ASIO SDK has known bugs, and it is typical to manage a local patched version.

dechamps commented 8 months ago

Manually managing the exact SDK version (and hence the ASIO SDK download) is essential for producing reproducible builds. In most cases you do not want to just blindly pull the latest SDK version from the Steinberg site

I agree, the ASIO SDK URL should be a specific version and the CMakeLists.txt should include a hash check to guarantee the zipfile is the one that's expected. This is what I did for one of my projects: https://github.com/dechamps/ASIOUtil/blob/781482bdd272abfaed92f0002d6dc9320cc1a1db/CMakeLists.txt#L9

dmitrykos commented 8 months ago
  1. The CMake code does still allow the use of a local zip file, it just does not search the old well-known location (hence it is a regression that needs to be fixed: the old location should be searched!).

@RossBencina could you please specify what old well-known location actually is? According to ASIO-README.txt it can be any, if am not mistaken. Currently PA CMake will look for existing asiosdk folder in the parent folder of PA source folder:

For example if I place asiosdk in the parent folder of PA: Found ASIO SDK: E:/SVN/audio/portaudio_git/../asiosdk

So SDK is actually found successfully for me taking into account that predefined location of ASIO SDK folder.

In most cases you do not want to just blindly pull the latest SDK version from the Steinberg site.

Is there any reason for this assumption? Unless some future SDK release breaks PA ASIO completely and renders it unusable I do not see any reason to stick to some old SDK version which may have bugs which were likely fixed in the new. Moreover it seems Steinberg is not super active with this SDK with last release dated 2019th (latest pulled asiosdk_2.3.3_2019-06-14).

If developer needs to use some specific version of SDK then it could be hosted in the folder or can be provided as ZIP via ASIO_SDK_ZIP_PATH:

cmake -DASIO_SDK_ZIP_PATH="E:/SVN/audio/portaudio_git/my_specific_asiosdk.zip"

dechamps commented 8 months ago

Is there any reason for this assumption? Unless some future SDK release breaks PA ASIO completely and renders it unusable I do not see any reason to stick to some old SDK version which may have bugs which were likely fixed in the new.

Because of the basic principle of reproducible software builds: you want builds to be hermetic and a given source tree should always compile to the same thing, regardless of external factors such as some third party updating some URL somewhere. This makes things easier to reason about (this PortAudio commit = this version of the ASIO SDK, period), ensures an update to the ASIO SDK cannot suddenly break PortAudio, and improves security against software supply chain attacks (if the download is hash-checked for integrity, which it should be).

That doesn't mean the ASIO SDK cannot be updated, it just means it will only be updated on PortAudio's own terms with an explicit PortAudio commit. Of course PortAudio users can always do the update with a local patch if they really want.

This is just software engineering best practice.

dmitrykos commented 8 months ago

@dechamps PA is not dealing with rocket science therefore freezing some SDK version does not make sense as progress and development move forward (see how Apple enforces this progress). I have already mentioned that if developer wants to freeze SDK at some version then there is possibility to do it by providing ZIP via ASIO_SDK_ZIP_PATH or by keeping offline folder. Freezing SDK at library level (PA) does not make sense unless SDK's developers do some insane things and break clients all the time, then I would agree with your point.