Merge pull request #25363 from fuzzard/cmake_exiv2_fix

Exiv2 support old API

5 files changed, 116 insertions, 57 deletions

diff --git a/cmake/modules/FindExiv2.cmake b/cmake/modules/FindExiv2.cmake
index 5e6c9e0fca..8bd5df2543 100644
--- a/cmake/modules/FindExiv2.cmake
+++ b/cmake/modules/FindExiv2.cmake
@@ -10,17 +10,29 @@
 if(NOT TARGET ${APP_NAME_LC}::${CMAKE_FIND_PACKAGE_NAME})
 
   macro(buildexiv2)
-
-    find_package(Patch MODULE REQUIRED)
     find_package(Iconv REQUIRED)
 
     # Note: Please drop once a release based on master is made. First 2 are already upstream.
     set(patches "${CMAKE_SOURCE_DIR}/tools/depends/target/${MODULE_LC}/0001-Add-EXIV2_ENABLE_FILESYSTEM_ACCESS-option.patch"
-    "${CMAKE_SOURCE_DIR}/tools/depends/target/${MODULE_LC}/0002-Set-conditional-HTTP-depending-on-EXIV2_ENABLE_WEBRE.patch"
-    "${CMAKE_SOURCE_DIR}/tools/depends/target/${MODULE_LC}/0003-UWP-Disable-getLoadedLibraries.patch")
+                "${CMAKE_SOURCE_DIR}/tools/depends/target/${MODULE_LC}/0002-Set-conditional-HTTP-depending-on-EXIV2_ENABLE_WEBRE.patch"
+                "${CMAKE_SOURCE_DIR}/tools/depends/target/${MODULE_LC}/0003-UWP-Disable-getLoadedLibraries.patch"
+                "${CMAKE_SOURCE_DIR}/tools/depends/target/${MODULE_LC}/0004-WIN-lib-postfix.patch")
 
     generate_patchcommand("${patches}")
 
+    if(WIN32 OR WINDOWS_STORE)
+      set(EXIV2_DEBUG_POSTFIX d)
+
+      # Exiv2 cant be built using /RTC1, so we alter and disable the auto addition of flags
+      # using WIN_DISABLE_PROJECT_FLAGS
+      string(REPLACE "/RTC1" "" EXIV2_CXX_FLAGS_DEBUG ${CMAKE_CXX_FLAGS_DEBUG} )
+
+      set(EXTRA_ARGS "-DCMAKE_CXX_FLAGS=${CMAKE_CXX_FLAGS}$<$<CONFIG:Debug>: ${EXIV2_CXX_FLAGS_DEBUG}>$<$<CONFIG:Release>: ${CMAKE_CXX_FLAGS_RELEASE}>"
+                     "-DCMAKE_EXE_LINKER_FLAGS=${CMAKE_EXE_LINKER_FLAGS}$<$<CONFIG:Debug>: ${CMAKE_EXE_LINKER_FLAGS_DEBUG}>$<$<CONFIG:Release>: ${CMAKE_EXE_LINKER_FLAGS_RELEASE}>")
+
+      set(WIN_DISABLE_PROJECT_FLAGS ON)
+    endif()
+
     set(CMAKE_ARGS -DBUILD_SHARED_LIBS=OFF
                    -DEXIV2_ENABLE_WEBREADY=OFF
                    -DEXIV2_ENABLE_XMP=OFF
@@ -33,7 +45,8 @@ if(NOT TARGET ${APP_NAME_LC}::${CMAKE_FIND_PACKAGE_NAME})
                    -DEXIV2_ENABLE_BROTLI=OFF
                    -DEXIV2_ENABLE_INIH=OFF
                    -DEXIV2_ENABLE_FILESYSTEM_ACCESS=OFF
-                   -DEXIV2_BUILD_EXIV2_COMMAND=OFF)
+                   -DEXIV2_BUILD_EXIV2_COMMAND=OFF
+                   ${EXTRA_ARGS})
 
     if(NOT CMAKE_CXX_COMPILER_LAUNCHER STREQUAL "")
       list(APPEND CMAKE_ARGS -DBUILD_WITH_CCACHE=ON)
@@ -58,7 +71,36 @@ if(NOT TARGET ${APP_NAME_LC}::${CMAKE_FIND_PACKAGE_NAME})
 
     buildexiv2()
   else()
-    if(NOT TARGET Exiv2::exiv2lib)
+    if(TARGET Exiv2::exiv2lib OR TARGET exiv2lib)
+      # We create variables based off TARGET data for use with FPHSA
+      # exiv2 < 0.28 uses a non namespaced target, but also has an alias. Prioritise
+      # namespaced target, and fallback to old target for < 0.28
+      if(TARGET Exiv2::exiv2lib)
+        set(_exiv_target_name Exiv2::exiv2lib)
+      else()
+        set(_exiv_target_name exiv2lib)
+      endif()
+
+      # This is for the case where a distro provides a non standard (Debug/Release) config type
+      # eg Debian's config file is exiv2Config-none.cmake
+      # convert this back to either DEBUG/RELEASE or just RELEASE
+      # we only do this because we use find_package_handle_standard_args for config time output
+      # and it isnt capable of handling TARGETS, so we have to extract the info
+      get_target_property(_EXIV2_CONFIGURATIONS ${_exiv_target_name} IMPORTED_CONFIGURATIONS)
+      foreach(_exiv2_config IN LISTS _EXIV2_CONFIGURATIONS)
+        # Some non standard config (eg None on Debian)
+        # Just set to RELEASE var so select_library_configurations can continue to work its magic
+        string(TOUPPER ${_exiv2_config} _exiv2_config_UPPER)
+        if((NOT ${_exiv2_config_UPPER} STREQUAL "RELEASE") AND
+          (NOT ${_exiv2_config_UPPER} STREQUAL "DEBUG"))
+          get_target_property(EXIV2_LIBRARY_RELEASE ${_exiv_target_name} IMPORTED_LOCATION_${_exiv2_config_UPPER})
+        else()
+          get_target_property(EXIV2_LIBRARY_${_exiv2_config_UPPER} ${_exiv_target_name} IMPORTED_LOCATION_${_exiv2_config_UPPER})
+        endif()
+      endforeach()
+
+      get_target_property(EXIV2_INCLUDE_DIR ${_exiv_target_name} INTERFACE_INCLUDE_DIRECTORIES)
+    else()
       find_package(PkgConfig)
       # Fallback to pkg-config and individual lib/include file search
       if(PKG_CONFIG_FOUND)
@@ -67,35 +109,12 @@ if(NOT TARGET ${APP_NAME_LC}::${CMAKE_FIND_PACKAGE_NAME})
       endif()
 
       find_path(EXIV2_INCLUDE_DIR NAMES exiv2/exiv2.hpp
-                                  HINTS ${PC_EXIV2_INCLUDEDIR} NO_CACHE)
-      find_library(EXIV2_LIBRARY NAMES exiv2
-                                 HINTS ${PC_EXIV2_LIBDIR} NO_CACHE)
+                                  HINTS ${PC_EXIV2_INCLUDEDIR})
+      find_library(EXIV2_LIBRARY_RELEASE NAMES exiv2
+                                        HINTS ${PC_EXIV2_LIBDIR})
     endif()
   endif()
 
-  # We create variables based off TARGET data for use with FPHSA
-  if(TARGET Exiv2::exiv2lib AND NOT TARGET exiv2)
-    # This is for the case where a distro provides a non standard (Debug/Release) config type
-    # eg Debian's config file is exiv2Config-none.cmake
-    # convert this back to either DEBUG/RELEASE or just RELEASE
-    # we only do this because we use find_package_handle_standard_args for config time output
-    # and it isnt capable of handling TARGETS, so we have to extract the info
-    get_target_property(_FMT_CONFIGURATIONS Exiv2::exiv2lib IMPORTED_CONFIGURATIONS)
-    foreach(_exiv2_config IN LISTS _FMT_CONFIGURATIONS)
-      # Some non standard config (eg None on Debian)
-      # Just set to RELEASE var so select_library_configurations can continue to work its magic
-      string(TOUPPER ${_exiv2_config} _exiv2_config_UPPER)
-      if((NOT ${_exiv2_config_UPPER} STREQUAL "RELEASE") AND
-         (NOT ${_exiv2_config_UPPER} STREQUAL "DEBUG"))
-        get_target_property(EXIV2_LIBRARY_RELEASE Exiv2::exiv2lib IMPORTED_LOCATION_${_exiv2_config_UPPER})
-      else()
-        get_target_property(EXIV2_LIBRARY_${_exiv2_config_UPPER} Exiv2::exiv2lib IMPORTED_LOCATION_${_exiv2_config_UPPER})
-      endif()
-    endforeach()
-
-    get_target_property(EXIV2_INCLUDE_DIR Exiv2::exiv2lib INTERFACE_INCLUDE_DIRECTORIES)
-  endif()
-
   include(SelectLibraryConfigurations)
   select_library_configurations(EXIV2)
   unset(EXIV2_LIBRARIES)
@@ -106,14 +125,31 @@ if(NOT TARGET ${APP_NAME_LC}::${CMAKE_FIND_PACKAGE_NAME})
                                     VERSION_VAR EXIV2_VER)
 
   if(EXIV2_FOUND)
-    if(TARGET Exiv2::exiv2lib AND NOT TARGET exiv2)
-      # Exiv2 config found. Use it
-      add_library(${APP_NAME_LC}::${CMAKE_FIND_PACKAGE_NAME} ALIAS Exiv2::exiv2lib)
+    if((TARGET Exiv2::exiv2lib OR TARGET exiv2lib) AND NOT TARGET exiv2)
+      # Exiv alias exiv2lib in their latest cmake config. We test for the alias
+      # to workout what we need to point OUR alias at.
+      get_target_property(_EXIV2_ALIASTARGET exiv2lib ALIASED_TARGET)
+      if(_EXIV2_ALIASTARGET)
+        set(_exiv_target_name ${_EXIV2_ALIASTARGET})
+      endif()
+      add_library(${APP_NAME_LC}::${CMAKE_FIND_PACKAGE_NAME} ALIAS ${_exiv_target_name})
     else()
       add_library(${APP_NAME_LC}::${CMAKE_FIND_PACKAGE_NAME} UNKNOWN IMPORTED)
       set_target_properties(${APP_NAME_LC}::${CMAKE_FIND_PACKAGE_NAME} PROPERTIES
-                                                                       IMPORTED_LOCATION "${EXIV2_LIBRARY}"
                                                                        INTERFACE_INCLUDE_DIRECTORIES "${EXIV2_INCLUDE_DIR}")
+
+      if(EXIV2_LIBRARY_RELEASE)
+        set_target_properties(${APP_NAME_LC}::${CMAKE_FIND_PACKAGE_NAME} PROPERTIES
+                                                                         IMPORTED_CONFIGURATIONS RELEASE
+                                                                         IMPORTED_LOCATION_RELEASE "${EXIV2_LIBRARY_RELEASE}")
+      endif()
+      if(EXIV2_LIBRARY_DEBUG)
+        set_target_properties(${APP_NAME_LC}::${CMAKE_FIND_PACKAGE_NAME} PROPERTIES
+                                                                         IMPORTED_LOCATION_DEBUG "${EXIV2_LIBRARY_DEBUG}")
+        set_property(TARGET ${APP_NAME_LC}::${CMAKE_FIND_PACKAGE_NAME} APPEND PROPERTY
+                                                                              IMPORTED_CONFIGURATIONS DEBUG)
+      endif()
+
       if(CORE_SYSTEM_NAME STREQUAL "freebsd")
         set_property(TARGET ${APP_NAME_LC}::${CMAKE_FIND_PACKAGE_NAME} APPEND PROPERTY
                                                                               INTERFACE_LINK_LIBRARIES procstat)
diff --git a/tools/depends/target/exiv2/0004-WIN-lib-postfix.patch b/tools/depends/target/exiv2/0004-WIN-lib-postfix.patch
new file mode 100644
index 0000000000..a78dacec9f
--- /dev/null
+++ b/tools/depends/target/exiv2/0004-WIN-lib-postfix.patch
@@ -0,0 +1,10 @@
+--- a/src/CMakeLists.txt
++++ b/src/CMakeLists.txt
+@@ -215,6 +215,7 @@
+     target_compile_definitions(exiv2lib PRIVATE PSAPI_VERSION=1)    # to be compatible with <= WinVista (#905)
+     # Since windows.h is included in some headers, we need to propagate this definition
+     target_compile_definitions(exiv2lib PUBLIC WIN32_LEAN_AND_MEAN)
++    set_target_properties(exiv2lib PROPERTIES DEBUG_POSTFIX d)
+ endif()
+ 
+ if (NOT MSVC)
diff --git a/xbmc/pictures/metadata/ImageMetadataParser.cpp b/xbmc/pictures/metadata/ImageMetadataParser.cpp
index 485fd8d974..aad8ade1b5 100644
--- a/xbmc/pictures/metadata/ImageMetadataParser.cpp
+++ b/xbmc/pictures/metadata/ImageMetadataParser.cpp
@@ -16,6 +16,14 @@
 #include <limits>
 #include <vector>
 
+#if (EXIV2_MAJOR_VERSION == 0) && (EXIV2_MINOR_VERSION < 28)
+#define EXIV_toUint32 toLong
+#define EXIV_toInt64 toLong
+#else
+#define EXIV_toUint32 toUint32
+#define EXIV_toInt64 toInt64
+#endif
+
 using namespace XFILE;
 
 namespace
@@ -75,7 +83,7 @@ std::unique_ptr<ImageMetadata> CImageMetadataParser::ExtractMetadata(const std::
   CImageMetadataParser parser;
 
   // extract metadata
-  parser.ExtractCommonMetadata(image);
+  parser.ExtractCommonMetadata(*image);
 
   parser.ExtractExif(image->exifData());
   parser.ExtractIPTC(image->iptcData());
@@ -83,19 +91,20 @@ std::unique_ptr<ImageMetadata> CImageMetadataParser::ExtractMetadata(const std::
   return std::move(parser.m_imageMetadata);
 }
 
-void CImageMetadataParser::ExtractCommonMetadata(std::unique_ptr<Exiv2::Image>& image)
+void CImageMetadataParser::ExtractCommonMetadata(Exiv2::Image& image)
 {
   //! TODO: all these elements are generic should be moved out of the exif struct
-  m_imageMetadata->height = image->pixelHeight();
-  m_imageMetadata->width = image->pixelWidth();
-  m_imageMetadata->fileComment = image->comment();
-
-  if (image->imageType() == Exiv2::ImageType::jpeg)
+  m_imageMetadata->height = image.pixelHeight();
+  m_imageMetadata->width = image.pixelWidth();
+  m_imageMetadata->fileComment = image.comment();
+#if (EXIV2_MAJOR_VERSION >= 0) && (EXIV2_MINOR_VERSION >= 28) && (EXIV2_PATCH_VERSION >= 2)
+  if (image.imageType() == Exiv2::ImageType::jpeg)
   {
-    auto jpegImage = dynamic_cast<Exiv2::JpegImage*>(image.get());
+    auto jpegImage = dynamic_cast<Exiv2::JpegImage*>(&image);
     m_imageMetadata->isColor = jpegImage->numColorComponents() == 3;
     m_imageMetadata->encodingProcess = jpegImage->encodingProcess();
   }
+#endif
 }
 
 void CImageMetadataParser::ExtractExif(Exiv2::ExifData& exifData)
@@ -121,7 +130,7 @@ void CImageMetadataParser::ExtractExif(Exiv2::ExifData& exifData)
     }
     else if (exifKey == "Exif.Image.Orientation")
     {
-      const int orientationValue = it->value().toUint32();
+      const int orientationValue = it->value().EXIV_toUint32();
       if (orientationValue < 0 || orientationValue > 8)
       {
         CLog::LogF(LOGWARNING, "Exif: Undefined rotation value {}",
@@ -145,11 +154,11 @@ void CImageMetadataParser::ExtractExif(Exiv2::ExifData& exifData)
     }
     else if (exifKey == "Exif.Photo.ExposureProgram")
     {
-      m_imageMetadata->exifInfo.ExposureProgram = it->value().toUint32();
+      m_imageMetadata->exifInfo.ExposureProgram = it->value().EXIV_toUint32();
     }
     else if (exifKey == "Exif.Photo.ISOSpeedRatings")
     {
-      m_imageMetadata->exifInfo.ISOequivalent = it->value().toUint32();
+      m_imageMetadata->exifInfo.ISOequivalent = it->value().EXIV_toUint32();
     }
     else if (exifKey == "Exif.Photo.DateTimeOriginal")
     {
@@ -184,11 +193,11 @@ void CImageMetadataParser::ExtractExif(Exiv2::ExifData& exifData)
     }
     else if (exifKey == "Exif.Photo.MeteringMode")
     {
-      m_imageMetadata->exifInfo.MeteringMode = it->value().toUint32();
+      m_imageMetadata->exifInfo.MeteringMode = it->value().EXIV_toUint32();
     }
     else if (exifKey == "Exif.Photo.Flash")
     {
-      m_imageMetadata->exifInfo.FlashUsed = it->value().toUint32();
+      m_imageMetadata->exifInfo.FlashUsed = it->value().EXIV_toUint32();
     }
     else if (exifKey == "Exif.Photo.FocalLength")
     {
@@ -211,7 +220,7 @@ void CImageMetadataParser::ExtractExif(Exiv2::ExifData& exifData)
       // Use largest of height and width to deal with images that have been
       // rotated to portrait format.
       {
-        const int value = static_cast<int>(it->value().toInt64());
+        const int value = static_cast<int>(it->value().EXIV_toInt64());
         if (m_imageWidth < value)
         {
           m_imageWidth = value;
@@ -224,7 +233,7 @@ void CImageMetadataParser::ExtractExif(Exiv2::ExifData& exifData)
     }
     else if (exifKey == "Exif.Photo.FocalPlaneResolutionUnit")
     {
-      const uint32_t value = it->value().toUint32();
+      const uint32_t value = it->value().EXIV_toUint32();
       // see: https://exiftool.org/TagNames/EXIF.html
       switch (value)
       {
@@ -247,15 +256,15 @@ void CImageMetadataParser::ExtractExif(Exiv2::ExifData& exifData)
     }
     else if (exifKey == "Exif.Photo.ExposureMode")
     {
-      m_imageMetadata->exifInfo.ExposureMode = it->value().toUint32();
+      m_imageMetadata->exifInfo.ExposureMode = it->value().EXIV_toUint32();
     }
     else if (exifKey == "Exif.Photo.WhiteBalance")
     {
-      m_imageMetadata->exifInfo.Whitebalance = it->value().toUint32();
+      m_imageMetadata->exifInfo.Whitebalance = it->value().EXIV_toUint32();
     }
     else if (exifKey == "Exif.Photo.LightSource")
     {
-      m_imageMetadata->exifInfo.LightSource = it->value().toUint32();
+      m_imageMetadata->exifInfo.LightSource = it->value().EXIV_toUint32();
     }
     else if (exifKey == "Exif.Photo.DigitalZoomRatio")
     {
@@ -266,7 +275,7 @@ void CImageMetadataParser::ExtractExif(Exiv2::ExifData& exifData)
       // The focal length equivalent 35 mm is a 2.2 tag (defined as of April 2002)
       // if its present, use it to compute equivalent focal length instead of
       // computing it from sensor geometry and actual focal length.
-      m_imageMetadata->exifInfo.FocalLength35mmEquiv = it->value().toUint32();
+      m_imageMetadata->exifInfo.FocalLength35mmEquiv = it->value().EXIV_toUint32();
     }
     else if (exifKey == "Exif.GPSInfo.GPSLatitudeRef")
     {
@@ -308,7 +317,7 @@ void CImageMetadataParser::ExtractExif(Exiv2::ExifData& exifData)
     }
     else if (exifKey == "Exif.GPSInfo.GPSAltitudeRef")
     {
-      auto value = it->value().toUint32();
+      auto value = it->value().EXIV_toUint32();
       if (value == 1) // below sea level
       {
         m_imageMetadata->exifInfo.GpsAlt = "-" + m_imageMetadata->exifInfo.GpsAlt;
diff --git a/xbmc/pictures/metadata/ImageMetadataParser.h b/xbmc/pictures/metadata/ImageMetadataParser.h
index 513355cbb8..f7bdd5115f 100644
--- a/xbmc/pictures/metadata/ImageMetadataParser.h
+++ b/xbmc/pictures/metadata/ImageMetadataParser.h
@@ -24,7 +24,7 @@ public:
 
 private:
   CImageMetadataParser();
-  void ExtractCommonMetadata(std::unique_ptr<Exiv2::Image>& image);
+  void ExtractCommonMetadata(Exiv2::Image& image);
   void ExtractExif(Exiv2::ExifData& exifData);
   void ExtractIPTC(Exiv2::IptcData& iptcData);
 
diff --git a/xbmc/pictures/metadata/test/TestMetadataExtraction.cpp b/xbmc/pictures/metadata/test/TestMetadataExtraction.cpp
index 7b08118dfd..4a0edcfd7c 100644
--- a/xbmc/pictures/metadata/test/TestMetadataExtraction.cpp
+++ b/xbmc/pictures/metadata/test/TestMetadataExtraction.cpp
@@ -46,9 +46,11 @@ TEST_F(TestMetadataExtraction, TestGPSImage)
   EXPECT_EQ(metadata->width, 1);
   EXPECT_EQ(metadata->height, 1);
   EXPECT_TRUE(metadata->fileComment.empty());
+#if (EXIV2_MAJOR_VERSION >= 0) && (EXIV2_MINOR_VERSION >= 28) && (EXIV2_PATCH_VERSION >= 2)
   // format specific (but common) metadata
   EXPECT_TRUE(metadata->isColor);
   EXPECT_EQ(metadata->encodingProcess, "Baseline DCT, Huffman coding");
+#endif
 }
 
 TEST_F(TestMetadataExtraction, TestIPTC)
@@ -74,7 +76,9 @@ TEST_F(TestMetadataExtraction, TestIPTC)
   EXPECT_EQ(metadata->width, 1);
   EXPECT_EQ(metadata->height, 1);
   EXPECT_TRUE(metadata->fileComment.empty());
+#if (EXIV2_MAJOR_VERSION >= 0) && (EXIV2_MINOR_VERSION >= 28) && (EXIV2_PATCH_VERSION >= 2)
   // format specific (but common) metadata
   EXPECT_TRUE(metadata->isColor);
   EXPECT_EQ(metadata->encodingProcess, "Baseline DCT, Huffman coding");
+#endif
 }