xiph / opus

Modern audio compression for the internet.
https://opus-codec.org/
Other
2.27k stars 604 forks source link

Compiling with CMake for Windows/MSVC only supports DLL runtime libraries #333

Open Malcolmnixon opened 6 months ago

Malcolmnixon commented 6 months ago

I'm attempting to consume opus as a git submodule from an official tag, and build a Godot extension cross-compiling for Windows, Linux, and Android.

The current CMakeLists.txt targets cmake 3.1 which hard-codes the C runtime library to either MultiThreadedDebugDLL or MultiThreadedDLL. The resulting code therefore has to ship the Microsoft runtime library DLL.

Upgrading CMakeLists.txt to cmake_minimum_required(VERSION 3.15) unlocks the options for specifying the runtime library options on the command-line such as cmake -DCMAKE_MSVC_RUNTIME_LIBRARY=MultiThreaded .

It would be preferable to support an OPUS_STATIC_RUNTIME option, which for MSVC would use CMAKE_MSVC_RUNTIME_LIBRARY Generator Expressions to correctly specify the Debug or Release Static or DLL runtimes.

Possibly something like:

if(MSVC)
  if(OPUS_STATIC_RUNTIME)
    set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>")
  else()
    set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>DLL")
  endif()
endif()
jmvalin commented 6 months ago

@xnorpx Any opinion here? unfortunately I don't know anything on the topic.

xnorpx commented 6 months ago

Last time we tried to upgrade min version people on ancient Linux distros got "upset" and we reverted back.

I don't know on top of my head if it's possible to use different min version per platform? But if possible then we could apply a patch.

jmvalin commented 6 months ago

Do you remember when you had to revert and what ancient distro had the problem?

xnorpx commented 6 months ago

Found it in an old pr. Ancient might been exaggeration.

Requirement was set to 3.12 and the user wanted 3.5 or less as that was the default in Ubuntu 16.04 (which was at that time only 2 years old)

3.15 is ~5 years old: https://github.com/Kitware/CMake/releases/tag/v3.15.0

Is there is an easy way to find what versions the common distros is on?

xnorpx commented 6 months ago

https://launchpad.net/ubuntu/+source/cmake/

Looks like 3.16 is fine for 20.04 LTS so seems resonable for Ubuntu.

xnorpx commented 6 months ago

For debian:

https://tracker.debian.org/pkg/cmake

Not sure how "old" old old stable is and how many users.

Malcolmnixon commented 6 months ago

Assuming 3.15 (or 3.16) is considered acceptable I could construct a PR for a new OPUS_STATIC_RUNTIME option, or is this something preferably done by the build team to keep consistency across cmake/meson/autoconf/etc?

xnorpx commented 6 months ago

Consistency is best effort :) not sure if Meson have that Opion.

Given it's 5 years i think it's fine. So pr would be good.

Add it's so the option is visible only on Windows also please test the different combinations.

xnorpx commented 6 months ago

I would say update to 3.16 since that is 20.04 LTS Ubuntu and Debian seems to support it (except the oldest)

Also update this test to 3.16: https://github.com/xiph/opus/blob/0e30966b198ad28943799eaf5b3b08100b6f70c3/.github/workflows/cmake.yml#L6C3-L9C11