CMAKE_CXX_FLAGS_RELEASE is empty if -DAVIF_CODEC_AOM=LOCAL -DAVIF_LIBYUV=OFF is specified

[wantehchang]

@fdintino @vrabaud

To reproduce this bug, run the following command on Linux. Make sure the ext/aom/ directory doesn't exist.

rm -rf build
mkdir build
cd build
cmake .. -G Ninja -DCMAKE_BUILD_TYPE=Release -DAVIF_CODEC_AOM=LOCAL -DAVIF_BUILD_APPS=ON -DAVIF_BUILD_TESTS=ON -DAVIF_GTEST=LOCAL -DAVIF_LIBYUV=OFF
ninja -v

Note that C++ files such as aviftest_helpers.cc and avifincrtest.cc are compiled without the -O3 -DNDEBUG flags commonly used in the Release build configuration.

Note that the problem is gone if I change -DAVIF_LIBYUV=OFF to -DAVIF_LIBYUV=LOCAL.

My experimets showed that this bug only occurs when we use FetchContent to build libaom locally. I think this bug is caused by the code in cmake/Modules/LocalAom.cmake. It touches variables like CMAKE_C_FLAGS_RELEASE. Seeing how complicated that code is, I think it would be better to build libaom using ExternalProject.

You can apply this patch and your will see that the CMAKE_CXX_FLAGS_RELEASE variable is empty.

diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index ec972dff..3e22775d 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -42,6 +42,8 @@ endforeach()
 
 if(AVIF_ENABLE_FUZZTEST OR AVIF_ENABLE_GTEST OR AVIF_BUILD_APPS)
     add_library(aviftest_helpers OBJECT gtest/aviftest_helpers.cc)
+    message(STATUS "WTC WTC WTC: CMAKE_BUILD_TYPE: ${CMAKE_BUILD_TYPE}")
+    message(STATUS "WTC WTC WTC: CMAKE_CXX_FLAGS_RELEASE: ${CMAKE_CXX_FLAGS_RELEASE}")
     target_link_libraries(aviftest_helpers PUBLIC avif_apps avif)
     target_link_libraries(aviftest_helpers PRIVATE avif_enable_warnings)
     add_library(aviftest_helpers_internal OBJECT gtest/aviftest_helpers.cc)

[wantehchang]

On Windows this results in a linker error, because we lose not only the compiler optimizaiton flag and -DNDEBUG but also the /MD flag, so the static CRT library libucrt.lib is used when linking the C++ test executables.

A cmake command line to reproduce the bug on Windows is:

cmake .. -G Ninja -DCMAKE_BUILD_TYPE=Release -DAVIF_JPEG=LOCAL -DAVIF_ZLIBPNG=LOCAL -DAVIF_BUILD_TESTS=ON -DAVIF_GTEST=LOCAL -DAVIF_CODEC_AOM=LOCAL -DAVIF_LIBYUV=OFF

[wantehchang]

A simple workaround is to add CXX to our project() call in the top-level CMakeLists.txt file. I.e., if we enable the C++ language before we call FetchContent on libaom.

[fdintino]

I think this also fixes the issue:

diff --git a/cmake/Modules/LocalAom.cmake b/cmake/Modules/LocalAom.cmake
index d7f83f56..5fc59fe1 100644
--- a/cmake/Modules/LocalAom.cmake
+++ b/cmake/Modules/LocalAom.cmake
@@ -145,7 +145,8 @@ else()

         foreach(_config_setting CMAKE_C_FLAGS CMAKE_CXX_FLAGS CMAKE_EXE_LINKER_FLAGS)
             foreach(_config_type DEBUG RELEASE MINSIZEREL RELWITHDEBINFO)
-                set(${_config_setting}_${_config_type} ${${_config_setting}_${_config_type}_ORIG} CACHE STRING "" FORCE)
+                unset(${_config_setting}_${_config_type} CACHE)
+                set(${_config_setting}_${_config_type} ${${_config_setting}_${_config_type}_ORIG})
                 unset(${_config_setting}_${_config_type}_ORIG)
             endforeach()
         endforeach()

The problem is that aom sets CMAKE_CXX_FLAGS_RELEASE (and other flags) as CACHE variables. In order to undo the changes AOM makes, I was setting them back to their original values as cache variables. But that prevented enable_language(CXX) from setting the flags. What I ought to have done (and what the above patch does) is to unset the variables as cache variables so that they can be reset as normal variables.

[wantehchang]

Frankie: Thank you! I am testing your patch now.

Is it possible to get the type of the MAKE_CXX_FLAGS_RELEASE variables before calling FetchContent on libaom, and set them back to the same type afterwards?

[wantehchang]

Frankie: Your patch fixed the bug on both Linux and Windows. Thanks!

[wantehchang]

Frankie: I converted your patch in #2365 (comment) to a pull request.

[wantehchang]

I filed a libaom bug report on this issue: https://aomedia.g-issues.chromium.org/issues/357715839

I will file a libavif issue on building libaom locally with ExternalProject_Add().

[fdintino]

I think it would be preferable if it could be addressed in libaom. It's nice to have the ability to pass in whatever aom-specific cmake configs you might need, and conversely it's a pain to properly copy over cmake flags to an external project (which would at the very least need to support cross-compiling).