fix: support for grayscale jpegs (#410)

dasch-swiss · Feb 15, 2024 · fd63dd0 · fd63dd0
1 parent 4800bd2
commit fd63dd0
Show file tree

Hide file tree

Showing 16 changed files with 103 additions and 67 deletions.
diff --git a/Dockerfile b/Dockerfile
@@ -1,7 +1,7 @@
 # syntax=docker/dockerfile:1.3
 
 # Expose (global) variables (ARGs before FROM can only be used on FROM lines and not afterwards)
-ARG SIPI_BASE=daschswiss/sipi-base:2.19.1
+ARG SIPI_BASE=daschswiss/sipi-base:2.20.0
 ARG UBUNTU_BASE=ubuntu:22.04
 
 # STAGE 1: Build

diff --git a/Dockerfile.sipi-dev-env b/Dockerfile.sipi-dev-env
@@ -1,4 +1,4 @@
-FROM daschswiss/sipi-base:2.19.1
+FROM daschswiss/sipi-base:2.20.0
 
 ARG DEBIAN_FRONTEND=noninteractive
 ENV TZ=Europe/Zurich

diff --git a/Makefile b/Makefile
@@ -7,7 +7,7 @@ CURRENT_DIR := $(shell dirname $(realpath $(firstword $(MAKEFILE_LIST))))
 include vars.mk
 
 # Version of the base Docker image
-SIPI_BASE := daschswiss/sipi-base:2.19.1
+SIPI_BASE := daschswiss/sipi-base:2.20.0
 UBUNTU_BASE := ubuntu:22.04
 
 .PHONY: docs-build

diff --git a/ext/kakadu/CMakeLists.txt b/ext/kakadu/CMakeLists.txt
@@ -11,17 +11,18 @@ message(STATUS "KDU_MAKE=${KDU_MAKE}")
 
 set(timestamp_policy DOWNLOAD_EXTRACT_TIMESTAMP OLD)
 
+set(KakaduVersion "v8_4_1-01382N")
 
 ExternalProject_Add(
         project_kakadu
         INSTALL_DIR ${COMMON_LOCAL}
-        DOWNLOAD_COMMAND unzip -o -d ${COMMON_SRCS} ${COMMON_VENDOR}/v8_3-01727L.zip
+        DOWNLOAD_COMMAND unzip -o -d ${COMMON_SRCS} ${COMMON_VENDOR}/${KakaduVersion}.zip
         ${timestamp_policy}
         CONFIGURE_COMMAND ""
-        BUILD_COMMAND make --directory=${COMMON_SRCS}/v8_3-01727L/make/ -f ${KDU_MAKE} EXEC_PLATFORM=${KDU_EXEC_PLATFORM} all_but_jni
-        INSTALL_COMMAND ${CMAKE_COMMAND} -E copy_directory ${COMMON_SRCS}/v8_3-01727L/lib/${KDU_ARCH} ${CONFIGURE_LIBDIR}
-        COMMAND ${CMAKE_COMMAND} -E copy_directory ${COMMON_SRCS}/v8_3-01727L/bin/${KDU_ARCH} ${COMMON_LOCAL}/bin
-        COMMAND ${CMAKE_COMMAND} -E copy_directory ${COMMON_SRCS}/v8_3-01727L/managed/all_includes ${COMMON_LOCAL}/include
+        BUILD_COMMAND make --directory=${COMMON_SRCS}/${KakaduVersion}/make/ -f ${KDU_MAKE} EXEC_PLATFORM=${KDU_EXEC_PLATFORM} all_but_jni
+        INSTALL_COMMAND ${CMAKE_COMMAND} -E copy_directory ${COMMON_SRCS}/${KakaduVersion}/lib/${KDU_ARCH} ${CONFIGURE_LIBDIR}
+        COMMAND ${CMAKE_COMMAND} -E copy_directory ${COMMON_SRCS}/${KakaduVersion}/bin/${KDU_ARCH} ${COMMON_LOCAL}/bin
+        COMMAND ${CMAKE_COMMAND} -E copy_directory ${COMMON_SRCS}/${KakaduVersion}/managed/all_includes ${COMMON_LOCAL}/include
 )
 
 ExternalProject_Get_Property(project_kakadu install_dir)

diff --git a/ext/openssl/CMakeLists.txt b/ext/openssl/CMakeLists.txt
@@ -13,10 +13,11 @@ endif()
 
 set(timestamp_policy DOWNLOAD_EXTRACT_TIMESTAMP OLD)
 
+set(OpenSSLVersion "3.2.1")
 
 ExternalProject_Add(project_ssl
         INSTALL_DIR ${COMMON_LOCAL}
-        URL https://www.openssl.org/source/openssl-3.0.1.tar.gz
+        URL https://www.openssl.org/source/openssl-${OpenSSLVersion}.tar.gz
         ${timestamp_policy}
         DOWNLOAD_DIR ${COMMON_SRCS}/downloads
         #DOWNLOAD_EXTRACT_TIMESTAMP NEW

diff --git a/include/metadata/SipiIcc.h b/include/metadata/SipiIcc.h
@@ -50,7 +50,7 @@ namespace Sipi {
         icc_sRGB,           //!< Standard sRGB profile
         icc_AdobeRGB,       //!< Standard AdobeRGB profile
         icc_RGB,            //!< A RGB profile that's given with parameters such as white point, primary colors etc. (see TIFF specs)
-        icc_CYMK_standard,  //!< A "standard" CMYK profile. We currently use the "USWebCoatedSWOP" profile
+        icc_CMYK_standard,  //!< A "standard" CMYK profile. We currently use the "USWebCoatedSWOP" profile
         icc_GRAY_D50,       //!< A standard profile for gray value images using a D50 light source and a gamma of 2.2
         icc_LUM_D65,        //!< A standard profile for gray value images as used be JPEG2000 JP2_sLUM_SPACE
         icc_ROMM_GRAY,       //!< A profile used by the JPEG2000 ISO suite....
@@ -107,8 +107,7 @@ namespace Sipi {
          * \param[in] tfunc Transfer function tables as retrieved by libtiff with either (1 << bitspersample) or 3*(1 << bitspersample) entries
          * \param[in] Length of tranfer function table
          */
-        SipiIcc(float white_point_p[], float primaries_p[], const unsigned short *tfunc = nullptr,
-                int tfunc_len = 0);
+        SipiIcc(float white_point_p[], float primaries_p[], const unsigned short *tfunc = nullptr, int tfunc_len = 0);
 
         /**
          * Destructor

diff --git a/shttps/Parsing.cpp b/shttps/Parsing.cpp
@@ -202,7 +202,7 @@ namespace shttps {
                 // wrong!)
                 //
                 bool found = false;
-                for (const auto mt: mimes_from_extension) {
+                for (const auto &mt: mimes_from_extension) {
                     if (mt == mimetype) { found = true; break; }
                 }
                 if (found) {
@@ -249,7 +249,7 @@ namespace shttps {
                 // now we test if the mimetype given given by the magic number corresponds to a valid mimetype for this extension
                 //
                 bool got1 = false;
-                for (const auto mt: mime_from_extension) {
+                for (const auto &mt: mime_from_extension) {
                     if (mt == actual_mimetype.first) { got1 = true; break; }
                 }
                 if (!got1) return false;
@@ -259,7 +259,7 @@ namespace shttps {
                     // now we test if the expected mimetype corresponds to a valid mimetype for this extension
                     //
                     bool got2 = false;
-                    for (const auto mt: mime_from_extension) {
+                    for (const auto &mt: mime_from_extension) {
                         if (mt == given_mimetype) { got2 = true; break; }
                     }
                     if (!got2) return false;

diff --git a/src/SipiImage.cpp b/src/SipiImage.cpp
@@ -438,7 +438,7 @@ namespace Sipi {
                 }
 
                 case 4: {
-                    icc = std::make_shared<SipiIcc>(icc_CYMK_standard); // assume CYMK
+                    icc = std::make_shared<SipiIcc>(icc_CMYK_standard); // assume CYMK
                     break;
                 }
 
@@ -489,7 +489,7 @@ namespace Sipi {
                 break;
             }
 
-            case icc_CYMK_standard: {
+            case icc_CMYK_standard: {
                 photo = SEPARATED;
                 break;
             }

diff --git a/src/formats/SipiIOJ2k.cpp b/src/formats/SipiIOJ2k.cpp
@@ -238,8 +238,7 @@ namespace Sipi {
 
         jp2_ultimate_src.open(filepath.c_str());
 
-        if (jpx_in.open(&jp2_ultimate_src, true)
-            < 0) { // if < 0, not compatible with JP2 or JPX.  Try opening as a raw code-stream.
+        if (jpx_in.open(&jp2_ultimate_src, true) < 0) { // if < 0, not compatible with JP2 or JPX.  Try opening as a raw code-stream.
             jp2_ultimate_src.close();
             file_in.open(filepath.c_str());
             input = &file_in;
@@ -436,7 +435,7 @@ namespace Sipi {
                     }
                     case kdu_supp::JP2_CMYK_SPACE: {
                         img->photo = SEPARATED;
-                        img->icc = std::make_shared<SipiIcc>(icc_CYMK_standard);
+                        img->icc = std::make_shared<SipiIcc>(icc_CMYK_standard);
                         break;
                     }
                     case kdu_supp::JP2_YCbCr1_SPACE: {
@@ -673,8 +672,7 @@ namespace Sipi {
 
         jp2_ultimate_src.open(filepath.c_str());
 
-        if (jpx_in.open(&jp2_ultimate_src, true) <
-            0) { // if < 0, not compatible with JP2 or JPX.  Try opening as a raw code-stream.
+        if (jpx_in.open(&jp2_ultimate_src, true) < 0) { // if < 0, not compatible with JP2 or JPX.  Try opening as a raw code-stream.
             jp2_ultimate_src.close();
             file_in.open(filepath.c_str());
             input = &file_in;
@@ -1013,7 +1011,7 @@ namespace Sipi {
             }
 
             //
-            // we need the essenation metaqdata in order to preserve unsupported ICC profiles
+            // we need the essential metadata in order to preserve unsupported ICC profiles
             //
             SipiEssentials es = img->essential_metadata();
 
@@ -1035,7 +1033,12 @@ namespace Sipi {
                             break;
                         }
                         case icc_sRGB: {
-                            jp2_family_colour.init(JP2_sRGB_SPACE);
+                            // TODO: this fixes the problem with grayscale JPEGs, but is it breaking anything else?
+                            if (img->nc - img->es.size() == 1) {
+                                jp2_family_colour.init(JP2_sLUM_SPACE);
+                            } else {
+                                jp2_family_colour.init(JP2_sRGB_SPACE);
+                            }
                             break;
                         }
                         case icc_AdobeRGB: {
@@ -1050,7 +1053,7 @@ namespace Sipi {
                             jp2_family_colour.init(icc_bytes);
                             break;
                         }
-                        case icc_CYMK_standard: {
+                        case icc_CMYK_standard: {
                             jp2_family_colour.init(JP2_CMYK_SPACE);
                             break;
                         }

diff --git a/src/formats/SipiIOJpeg.cpp b/src/formats/SipiIOJpeg.cpp
@@ -235,6 +235,9 @@ namespace Sipi {
     }
     //=============================================================================
 
+    /*!
+     * Load the JPEG file
+     */
     static void jpeg_file_src(struct jpeg_decompress_struct *cinfo, int file_id) {
         struct jpeg_source_mgr *srcmgr;
         FileBuffer *file_buffer;
@@ -568,7 +571,7 @@ namespace Sipi {
         int icc_buffer_len = 0;
         while (marker) {
             if (marker->marker == JPEG_COM) {
-                std::string emdatastr((char *) marker->data, marker->data_length);
+                std::string emdatastr(reinterpret_cast<char *>(marker->data), marker->data_length);
                 if (emdatastr.compare(0, 5, "SIPI:", 5) == 0) {
                     SipiEssentials se(emdatastr);
                     img->essential_metadata(se);
@@ -577,12 +580,12 @@ namespace Sipi {
                 //
                 // first we try to find the exif part
                 //
-                auto *pos = (unsigned char *) memmem(marker->data, marker->data_length, "Exif\000\000", 6);
+                auto *pos = static_cast<unsigned char *>(memmem(marker->data, marker->data_length, "Exif\000\000", 6));
                 if (pos != nullptr) {
                     img->exif = std::make_shared<SipiExif>(pos + 6, marker->data_length - (pos - marker->data) - 6);
                     uint16_t ori;
                     if (img->exif->getValByKey("Exif.Image.Orientation", ori)) {
-                        img->orientation = Orientation(ori);
+                        img->orientation = static_cast<Orientation>(ori);
                     }
                 }
 
@@ -655,10 +658,10 @@ namespace Sipi {
                 //
                 // first we try to find the exif part
                 //
-                auto *pos = (unsigned char *) memmem(marker->data, marker->data_length, "ICC_PROFILE\0", 12);
+                auto *pos = static_cast<unsigned char *>(memmem(marker->data, marker->data_length, "ICC_PROFILE\0", 12));
                 if (pos != nullptr) {
                     auto len = marker->data_length - (pos - (unsigned char *) marker->data) - 14;
-                    icc_buffer = (unsigned char *) realloc(icc_buffer, icc_buffer_len + len);
+                    icc_buffer = static_cast<unsigned char*>(realloc(icc_buffer, icc_buffer_len + len));
                     Sipi::memcpy(icc_buffer + icc_buffer_len, pos + 14, (size_t) len);
                     icc_buffer_len += len;
                 }

diff --git a/src/formats/SipiIOTiff.cpp b/src/formats/SipiIOTiff.cpp
@@ -468,20 +468,21 @@ namespace Sipi {
                 throw Sipi::SipiImageError(__file__, __LINE__, msg);
             }
 
-            auto sll = (unsigned int) TIFFScanlineSize(tif);
+            auto sll = static_cast<unsigned int>(TIFFScanlineSize(tif));
+
             TIFF_GET_FIELD (tif, TIFFTAG_SAMPLESPERPIXEL, &stmp, 1);
-            img->nc = (int) stmp;
+            img->nc = static_cast<size_t>(stmp);
 
             TIFF_GET_FIELD (tif, TIFFTAG_BITSPERSAMPLE, &stmp, 1);
-            img->bps = stmp;
+            img->bps = static_cast<size_t>(stmp);
 
             TIFF_GET_FIELD (tif, TIFFTAG_ORIENTATION, &ori, ORIENTATION_TOPLEFT);
             img->orientation = static_cast<Orientation>(ori);
 
             if (1 != TIFFGetField(tif, TIFFTAG_PHOTOMETRIC, &stmp)) {
                 img->photo = MINISBLACK;
             } else {
-                img->photo = (PhotometricInterpretation) stmp;
+                img->photo = static_cast<PhotometricInterpretation>(stmp);
             }
 
             //
@@ -886,7 +887,7 @@ namespace Sipi {
                     }
 
                     case SEPARATED: {
-                        img->icc = std::make_shared<SipiIcc>(icc_CYMK_standard);
+                        img->icc = std::make_shared<SipiIcc>(icc_CMYK_standard);
                         break;
                     }
 

diff --git a/src/metadata/SipiIcc.cpp b/src/metadata/SipiIcc.cpp
@@ -118,9 +118,9 @@ namespace Sipi {
             case icc_RGB: {
                 throw SipiError(__file__, __LINE__, "Profile type \"icc_RGB\" uses other constructor");
             }
-            case icc_CYMK_standard: {
+            case icc_CMYK_standard: {
                 icc_profile = cmsOpenProfileFromMem(USWebCoatedSWOP_icc, USWebCoatedSWOP_icc_len);
-                profile_type = icc_CYMK_standard;
+                profile_type = icc_CMYK_standard;
                 break;
             }
             case icc_GRAY_D50: {

diff --git a/src/metadata/SipiXmp.cpp b/src/metadata/SipiXmp.cpp
@@ -26,8 +26,6 @@
 #include "SipiError.h"
 #include "SipiXmp.h"
 
-static const char __file__[] = __FILE__;
-
 /*!
  * ToDo: remove provisional code as soon as Exiv2::Xmp is thread safe (expected v.26)
  * ATTENTION!!!!!!!!!
@@ -40,9 +38,8 @@ namespace Sipi {
 
     XmpMutex xmp_mutex;
 
-
     void xmplock_func(void *pLockData, bool lockUnlock) {
-        XmpMutex *m = (XmpMutex *) pLockData;
+        auto *m = static_cast<XmpMutex *>(pLockData);
         if (lockUnlock) {
             std::cerr << "XMP-LOCK!" << std::endl;
             m->lock.lock();

diff --git a/test/_test_data/images/unit/.gitignore b/test/_test_data/images/unit/.gitignore
@@ -2,7 +2,6 @@
 *
 # Except these files
 !.gitignore
-!image_with_rotation.jpg
 !8pdET49BfoJ-EeRcIbgcLch.info
 !8pdET49BfoJ-EeRcIbgcLch.mp4
 !8pdET49BfoJ-EeRcIbgcLch.mp4.orig
@@ -12,6 +11,7 @@
 !cmyk.tif
 !cmyk_lossy.jp2
 !cmyk_with_alpha.tif
+!gray_with_icc_another.jpg
 !gray_with_icc.jp2
 !has-missing-sidecar-file.mp4
 !has-missing-sidecar-file.mp4.orig

diff --git a/test/_test_data/images/unit/gray_with_icc_another.jpg b/test/_test_data/images/unit/gray_with_icc_another.jpg