From 0f50bf95c9822b513bb900c0a9299581889724c7 Mon Sep 17 00:00:00 2001 From: Larry Gritz Date: Mon, 22 Jul 2024 10:39:44 -0700 Subject: [PATCH] build(deps): Raise giflib minimum to 5.0 (#4349) Even that is quite old -- released in 2012! But it's the first version that guaranteed thread safety for error reporting. So we're finally making that our minimum and taking out the locking we did on our side. Signed-off-by: Larry Gritz --- INSTALL.md | 3 +-- src/cmake/externalpackages.cmake | 6 ++---- src/gif.imageio/gifinput.cpp | 33 +++----------------------------- 3 files changed, 6 insertions(+), 36 deletions(-) diff --git a/INSTALL.md b/INSTALL.md index 4c2114db85..40270955af 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -56,8 +56,7 @@ NEW or CHANGED MINIMUM dependencies since the last major release are **bold**. or for capturing images from a camera: * **OpenCV 4.x** (tested through 4.10) * If you want support for GIF images: - * giflib >= 4.1 (tested through 5.2; 5.0+ is strongly recommended for - stability and thread safety) + * **giflib >= 5.0** (tested through 5.2) * If you want support for HEIF/HEIC or AVIF images: * libheif >= 1.3 (1.7 required for AVIF support, 1.16 required for correct orientation support, tested through 1.17.6) diff --git a/src/cmake/externalpackages.cmake b/src/cmake/externalpackages.cmake index dbfd17e898..71439639c1 100644 --- a/src/cmake/externalpackages.cmake +++ b/src/cmake/externalpackages.cmake @@ -162,10 +162,8 @@ checked_find_package (TBB 2017 checked_find_package (DCMTK CONFIG VERSION_MIN 3.6.1) checked_find_package (FFmpeg VERSION_MIN 4.0) -checked_find_package (GIF - VERSION_MIN 4 - RECOMMEND_MIN 5.0 - RECOMMEND_MIN_REASON "for stability and thread safety") + +checked_find_package (GIF VERSION_MIN 5.0) # For HEIF/HEIC/AVIF formats checked_find_package (Libheif VERSION_MIN 1.3 diff --git a/src/gif.imageio/gifinput.cpp b/src/gif.imageio/gifinput.cpp index e95f7096d9..45cd33cc9e 100644 --- a/src/gif.imageio/gifinput.cpp +++ b/src/gif.imageio/gifinput.cpp @@ -19,7 +19,7 @@ // for older giflib versions #ifndef GIFLIB_MAJOR -# define GIFLIB_MAJOR 4 +# error "GIFLIB 5.0 minimum required" #endif #ifndef DISPOSAL_UNSPECIFIED @@ -132,12 +132,8 @@ OIIO_EXPORT const char* gif_input_extensions[] = { "gif", NULL }; OIIO_EXPORT const char* gif_imageio_library_version() { -#if defined(GIFLIB_MAJOR) && defined(GIFLIB_MINOR) && defined(GIFLIB_RELEASE) return "gif_lib " OIIO_STRINGIZE(GIFLIB_MAJOR) "." OIIO_STRINGIZE( GIFLIB_MINOR) "." OIIO_STRINGIZE(GIFLIB_RELEASE); -#else - return "gif_lib unknown version"; -#endif } OIIO_PLUGIN_EXPORTS_END @@ -408,20 +404,12 @@ GIFInput::seek_subimage(int subimage, int miplevel) if (!m_gif_file) { if (!ioproxy_use_or_open(m_filename)) return false; -#if GIFLIB_MAJOR >= 5 int giflib_error; m_gif_file = DGifOpen(this, readFunc, &giflib_error); if (!m_gif_file) { errorfmt("{}", GifErrorString(giflib_error)); return false; } -#else - m_gif_file = DGifOpen(this, readFunc); - if (!m_gif_file) { - errorfmt("Error opening GIF"); - return false; - } -#endif m_subimage = -1; m_canvas.resize(m_gif_file->SWidth * m_gif_file->SHeight * 4); } @@ -459,27 +447,12 @@ GIFInput::seek_subimage(int subimage, int miplevel) -static spin_mutex gif_error_mutex; - - void GIFInput::report_last_error(void) { - // N.B. Only GIFLIB_MAJOR >= 5 looks properly thread-safe, in that the - // error is guaranteed to be specific to this open file. We use a spin - // mutex to prevent a thread clash for older versions, but it still - // appears to be a global call, so we can't be absolutely sure that the - // error was for *this* file. So if you're using giflib prior to - // version 5, beware. -#if GIFLIB_MAJOR >= 5 + // GIFLIB_MAJOR >= 5 looks properly thread-safe, in that the error is + // guaranteed to be specific to this open file. errorfmt("{}", GifErrorString(m_gif_file->Error)); -#elif GIFLIB_MAJOR == 4 && GIFLIB_MINOR >= 2 - spin_lock lock(gif_error_mutex); - errorfmt("{}", GifErrorString()); -#else - spin_lock lock(gif_error_mutex); - errorfmt("GIF error {}", GifLastError()); -#endif }