Pass cli CMAKE_CXX_FLAGS(_XXX) and improve SoundTouch linking

[Asd-g]

No description provided.

[qyot27]

I'm confused about the ClangCL comment. That section of CMakeLists.txt hasn't changed in 5 years and I just tested on W10, W11, VS2019, and VS2022, and -T ClangCL works on all of them, although the change does seems to greatly speed up the compilation process.

[Asd-g]

Are you using recent clang 19.1.x? IIRC the issue raised when I updated it. These patches are from ~2 weeks ago when I build avs+ - I have to check again what was the exact issue.

[qyot27]

The VS2019 check on Win10 was using Clang 12, and VS2022 is using Clang 18.1.8 (I ran the updater first just to make sure). The only 19.x version I have is the llvm-mingw installation on my main Linux setup, and it doesn't have the CL interface with it due to a lack of the MS SDK stuff that would require.

[Asd-g]

Yes, I don't use clang shipped with VS (same with cmake and other stuff). I use these scripts (with some changes) to implement the clang support in VS and the builds from llvm.

[pinterf]

Regarding the exception handling cl parameters, I found this conversation, which mentions something that is available only since v19:
llvm/llvm-project#62606

I see that there was some incompatibility between MSVC and clang-cl, /EHsc and /EHa was used for Clang-cl, but I do not really understand the difference and how it affects Avisynth code.

I haven't installed clang 19.1 yet, actually what was the problem?

[Asd-g]

Attached the logs from 19.1.0 and 19.1.6.

logs.zip

Edit: The issue is not related to the exceptions. If you pass -DCMAKE_CXX_FLAGS=xxx" in cli xxx is not passed and moreover some other flags are removed (like /EHa, you can see in the logs).
Changing these lines to add_compile_options(${CMAKE_CXX_FLAGS} /EHa -Wno-inconsistent-missing-override) makes possible passing CMAKE_CXX_FLAGS from cli and doesn't overwrite/delete other set flags.

[pinterf]

Anyway, add_compile_options is available at our minimum CMAKE_MINIMUM_REQUIRED( VERSION 3.6.2 ) so it can be safely used. Back to the /EHsc opcion, why was it needed to change it to /EHa?

[qyot27]

From what I've gleaned elseweb, the defaults for MSVC are (were?) different depending on if you used the GUI or the CLI, and - it was probably NMake - the CLI would just spam warnings about the exception handling model into the Prompt. This would have been years ago, so the behavior could very well have changed since the main compiler in use for the core was VS 2010 (or 2013).

I remember the warnings, and the comments where it does the override mention the warnings, but I'm not sure if the 'differing defaults' explanation actually was ever true, or if it was, if it still does that with VS 2019 or 2022. Maybe since it did definitely affect NMake, it would also affect using MSVC+Ninja, but I don't know. It was long before we ever supported Clang, and the rules likely were just copy/pasted to apply to that case when ClangCL was added to the detection.

[pinterf]

Looking good, but let's wait for qyot27.
(there is a typo in the description: misspelled SoundTorch instead of SoundTouch)

[Asd-g]

Thanks. There was also soundtorch in the commit message.

[pinterf]

Sorry, I'm just putting this ito the discussion, I'm using cmake gui, and CMAKE_PREFIX_PATH is not set, must be added manually after in each platform change and cache deletion. I was trying this addition in the main CMakeLists.txt.
Does it make sense? For me it helped to make the build process easier. Is Windows and MSVC limitations enough?

    # Check if the environment variable AVS_DEPS_BUILD_HOME is set
  if(DEFINED ENV{AVS_DEPS_BUILD_HOME})
      # Set the CMake variable to the value of the environment variable
      set(AVS_DEPS_BUILD_HOME $ENV{AVS_DEPS_BUILD_HOME})
      
      # Check if we are on Windows using MSVC
      if(MSVC AND WIN32)
          # Determine the architecture based on the generator platform
          if(CMAKE_GENERATOR_PLATFORM STREQUAL "ARM64")
              # ARM64 architecture
              set(ARCH_DIR "arm64")
          elseif(CMAKE_GENERATOR_PLATFORM STREQUAL "x64")
              # 64-bit architecture
              set(ARCH_DIR "x86-64")
          elseif(CMAKE_GENERATOR_PLATFORM STREQUAL "Win32")
              # 32-bit architecture
              set(ARCH_DIR "x86-32")
          elseif(NOT CMAKE_GENERATOR_PLATFORM)
              # Use the default platform if unspecified
              if(CMAKE_VS_PLATFORM_NAME_DEFAULT STREQUAL "ARM64")
                  set(ARCH_DIR "arm64")
              elseif(CMAKE_VS_PLATFORM_NAME_DEFAULT STREQUAL "x64")
                  set(ARCH_DIR "x86-64")
              elseif(CMAKE_VS_PLATFORM_NAME_DEFAULT STREQUAL "Win32")
                  set(ARCH_DIR "x86-32")
              else()
                  message(WARNING "Unsupported default platform. Leaving CMAKE_PREFIX_PATH as is.")
                  set(ARCH_DIR "")
              endif()
          else()
              message(WARNING "Unsupported generator platform. Leaving CMAKE_PREFIX_PATH as is.")
              set(ARCH_DIR "")
          endif()

          # Check if ARCH_DIR is set and if CMAKE_PREFIX_PATH is empty or initialized to default
          if(ARCH_DIR AND (NOT CMAKE_PREFIX_PATH OR CMAKE_PREFIX_PATH_INITIALIZED_TO_DEFAULT))
              # Set and cache the CMAKE_PREFIX_PATH using the environment variable and architecture
              set(CMAKE_PREFIX_PATH "${AVS_DEPS_BUILD_HOME}/avsplus_build_deps/${ARCH_DIR}" CACHE PATH "Prefix path for dependencies")
          endif()
      else()
          # message(WARNING "Not on Windows MSVC. Using default prefix path.")
          # Optionally set and cache a default prefix path if not on Windows MSVC
          if(NOT CMAKE_PREFIX_PATH OR CMAKE_PREFIX_PATH_INITIALIZED_TO_DEFAULT)
              set(CMAKE_PREFIX_PATH "" CACHE PATH "Prefix path for dependencies")
          endif()
      endif()
  else()
      # message(WARNING "Environment variable AVS_DEPS_BUILD_HOME is not set. Using default prefix path.")
      # Optionally set and cache a default prefix path if the environment variable is not set
      if(NOT CMAKE_PREFIX_PATH OR CMAKE_PREFIX_PATH_INITIALIZED_TO_DEFAULT)
          set(CMAKE_PREFIX_PATH "" CACHE PATH "Prefix path for dependencies")
      endif()
  endif()

[qyot27]

IMO, adhering a CMakeLists.txt option too closely to a build guide is fairly risky. But since we currently do similar enough assumptions for, example, the baseclasses for DirectShowSource, guarding it to detect whether someone is following the guide is probably fine.

In the short term, one of those reasons I consider it risky is because I've been weighing the Windows on ARM stuff and unless there's a really good reason not to, putting in a hard¹ restriction that MSVC and MSVC-like compilers can only be used on x86(-64). Which in turn means the ARM64 instructions in the guide will need to be split out and rewritten/adapted for MinGW under [WSL2|MSYS2], at least if we want to be consistent.

¹meaning that both CMakeLists.txt prevents that combination and that the config parameters and macros do as well, to try and actively stop it from being built that way. There are like, three different options for how to proceed on Windows-on-ARM, and none of them are optimal (two are merely decent, with one being riskier than the other depending on the point of view, and one is horrible and just continues perpetuating the problems that currently exist).

A future solution with regard to having the external dependencies build guide being followed or expected in CMakeLists.txt would be to have a second repository with a CMake project that fetches and builds/organizes SoundTouch and DevIL in the way we expect them to, or even go as far as having it build the core at the end too. Similar in spirit to mpv-winbuild-cmake, except significantly smaller in scope. One could argue: "at that point, why not still have it inside the AviSynthPlus repo as submodules?", but that would require us continuing to have that logic inside the core's build system, not to mention that submodules are tied to specific commits and we'd be right back to having to try to keep the dependencies up-to-date in the core's settings instead of just pointing at the ones already installed.

[pinterf]

You are right, this is a very specific modification. Not elegant. Anyway, CMAKE_PREFIX_PATH would be nice to appear as an editable entry in in option list.

[Asd-g]

Probably you know but you can do in cli cmake -B .... -DCMAKE_PREFIX_PATH=xxxx and then open cmake-gui.

IMO this should be changed to "IL/il.h" and this should be changed to "soundtouch/SoundTouch.h". These are the proper include locations and will make CMAKE_PREFIX_PATH to find also the headers not only the libs (avoiding CMAKE_CXX_STANDARD_INCLUDE_DIRECTORIES or manual put the include dirs). I use prebuilt DevIL libs so didn't tested it but with CMAKE_PREFIX_PATH the -DIL... commands could be omitted.