From 057cd06c19875bcf8b5d34d41d92a8abdb856b7c Mon Sep 17 00:00:00 2001 From: Kleis Auke Wolthuizen Date: Fri, 11 Mar 2022 21:11:24 +0100 Subject: [PATCH] Remove LCMS mutex (#112) * Remove LCMS mutex Requires mm2/Little-CMS@a35bacd, which is released in LCMS v2.11. * Use a threadsafe alternative of gmtime in LCMS Requires mm2/Little-CMS@68ee2ff, which is released in LCMS v2.13. LCMS submodule was updated to version 2.13.1 instead. --- lib/jxl/enc_color_management.cc | 15 +++++++++++++++ third_party/CMakeLists.txt | 2 +- third_party/lcms2.cmake | 14 -------------- 3 files changed, 16 insertions(+), 15 deletions(-) Index: libjxl-0.8.1/lib/jxl/enc_color_management.cc =================================================================== --- libjxl-0.8.1.orig/lib/jxl/enc_color_management.cc +++ libjxl-0.8.1/lib/jxl/enc_color_management.cc @@ -18,6 +18,7 @@ #include #include #include +#include #include #include @@ -308,6 +309,14 @@ int DoColorSpaceTransform(void* t, size_ // Define to 1 on OS X as a workaround for older LCMS lacking MD5. #define JXL_CMS_OLD_VERSION 0 +// cms functions (even *THR) are not thread-safe, except cmsDoTransform. +// To ensure all functions are covered without frequent lock-taking nor risk of +// recursive lock, we lock in the top-level APIs. +static std::mutex& LcmsMutex() { + static std::mutex m; + return m; +} + #if JPEGXL_ENABLE_SKCMS JXL_MUST_USE_RESULT CIExy CIExyFromXYZ(const float XYZ[3]) { @@ -871,6 +880,9 @@ bool ApplyCICP(const uint8_t color_prima } // namespace +// All functions that call lcms directly (except ColorSpaceTransform::Run) must +// lock LcmsMutex(). + Status ColorEncoding::SetFieldsFromICC() { // In case parsing fails, mark the ColorEncoding as invalid. SetColorSpace(ColorSpace::kUnknown); @@ -917,6 +929,7 @@ Status ColorEncoding::SetFieldsFromICC() DetectTransferFunction(profile, this); #else // JPEGXL_ENABLE_SKCMS + std::lock_guard guard(LcmsMutex()); const cmsContext context = GetContext(); Profile profile; @@ -984,6 +997,7 @@ void JxlCmsDestroy(void* cms_data) { if (cms_data == nullptr) return; JxlCms* t = reinterpret_cast(cms_data); #if !JPEGXL_ENABLE_SKCMS + std::lock_guard guard(LcmsMutex()); TransformDeleter()(t->lcms_transform); #endif delete t; @@ -1020,6 +1034,7 @@ void* JxlCmsInit(void* init_data, size_t return nullptr; } #else // JPEGXL_ENABLE_SKCMS + std::lock_guard guard(LcmsMutex()); const cmsContext context = GetContext(); Profile profile_src, profile_dst; if (!DecodeProfile(context, c_src.ICC(), &profile_src)) { Index: libjxl-0.8.1/third_party/CMakeLists.txt =================================================================== --- libjxl-0.8.1.orig/third_party/CMakeLists.txt +++ libjxl-0.8.1/third_party/CMakeLists.txt @@ -111,7 +111,7 @@ if (JPEGXL_ENABLE_SKCMS OR JPEGXL_ENABLE endif () if (JPEGXL_ENABLE_VIEWERS OR NOT JPEGXL_ENABLE_SKCMS) if( NOT EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/lcms/.git" OR JPEGXL_FORCE_SYSTEM_LCMS2 ) - find_package(LCMS2 2.13) + find_package(LCMS2 2.10) if ( NOT LCMS2_FOUND ) message(FATAL_ERROR "Please install lcms2 or run git submodule update --init") endif () Index: libjxl-0.8.1/third_party/lcms2.cmake =================================================================== --- libjxl-0.8.1.orig/third_party/lcms2.cmake +++ libjxl-0.8.1/third_party/lcms2.cmake @@ -60,18 +60,4 @@ target_compile_definitions(lcms2 target_compile_definitions(lcms2 PUBLIC "-DCMS_NO_REGISTER_KEYWORD=1") -# Ensure that a thread safe alternative of gmtime is used in LCMS -include(CheckSymbolExists) -check_symbol_exists(gmtime_r "time.h" HAVE_GMTIME_R) -if (HAVE_GMTIME_R) - target_compile_definitions(lcms2 - PUBLIC "-DHAVE_GMTIME_R=1") -else() - check_symbol_exists(gmtime_s "time.h" HAVE_GMTIME_S) - if (HAVE_GMTIME_S) - target_compile_definitions(lcms2 - PUBLIC "-DHAVE_GMTIME_S=1") - endif() -endif() - set_property(TARGET lcms2 PROPERTY POSITION_INDEPENDENT_CODE ON)