OpenJPEG: actually fold TPSOT_COMPLIANT open option in existing STRIC…

…T one
rouault · Nov 13, 2024 · 86a7c68 · 86a7c68
1 parent 76a238c
commit 86a7c68
Show file tree

Hide file tree

Showing 5 changed files with 12 additions and 28 deletions.
diff --git a/doc/source/drivers/raster/jp2openjpeg.rst b/doc/source/drivers/raster/jp2openjpeg.rst
@@ -94,7 +94,11 @@ The following open options are available:
       (OpenJPEG >= 2.5) Whether strict/pedantic
       decoding mode should be enabled.
       This can be set to NO to allow decoding (some) broken files, typically
-      truncated single-tiled files.
+      truncated single-tiled files. Starting with GDAL 3.11 and OpenJPEG 2.5.3,
+      the default STRICT=YES mode also disables the "TPSOT fixing logic" introduced
+      in https://github.com/uclouvain/openjpeg/pull/514 that is needed to read
+      some broken files, but may hurt performance when reading conformant remote
+      files.
 
 -  .. oo:: 1BIT_ALPHA_PROMOTION
       :choices: YES, NO
@@ -119,16 +123,6 @@ The following open options are available:
       the image is single-tiled. Note however that the tile size must not
       exceed 2 GB since that's the limit supported by GDAL.
 
--  .. oo:: TPSOT_COMPLIANT
-      :choices: YES, NO
-      :default: YES
-      :since: 3.11
-
-      (OpenJPEG >= 2.5.3) Assume that a multi-tilepart file is compliant with
-      respect to the TPSOT value. Turn that setting to NO only if you may deal
-      with non-compliant files such as the ones of https://github.com/uclouvain/openjpeg/pull/514 .
-      Default behavior prior to OpenJPEG 2.5.3 was equivalent to NO.
-
 Creation Options
 ----------------
 

diff --git a/frmts/openjpeg/openjpegdrivercore.cpp b/frmts/openjpeg/openjpegdrivercore.cpp
@@ -101,9 +101,6 @@ void OPENJPEGDriverSetCommonMetadata(GDALDriver *poDriver)
         "description='Whether to always use the JPEG-2000 block size as "
         "the "
         "GDAL block size' default='NO'/>"
-        "   <Option name='TPSOT_COMPLIANT' type='boolean' "
-        "description='Assume that a multi-tilepart file is compliant with "
-        "respect to the TPSOT value' default='YES'/>"
         "</OpenOptionList>");
 
     poDriver->SetMetadataItem(

diff --git a/frmts/openjpeg/opjdatasetbase.h b/frmts/openjpeg/opjdatasetbase.h
@@ -261,10 +261,9 @@ struct OPJCodecWrapper
         return osComment + opj_version();
     }
 
-    void updateStrict(CPL_UNUSED bool strict)
+    void updateStrict(bool strict)
     {
-        // prevent linter from treating this as potential static method
-        (void)this;
+        this->m_bStrict = strict;
 #if IS_OPENJPEG_OR_LATER(2, 5, 0)
         if (!strict)
             opj_decoder_set_strict_mode(pCodec, false);
@@ -313,8 +312,7 @@ struct OPJCodecWrapper
 
     bool setUpDecompress(CPL_UNUSED int numThreads,
                          vsi_l_offset nCodeStreamLength, uint32_t *nTileW,
-                         uint32_t *nTileH, int *numResolutions,
-                         [[maybe_unused]] bool bTPSOTCompliant)
+                         uint32_t *nTileH, int *numResolutions)
     {
 
         OPJCodecWrapper codec;
@@ -331,7 +329,7 @@ struct OPJCodecWrapper
         opj_set_default_decoder_parameters(&decompressParams);
 
 #ifdef OPJ_DPARAMETERS_DISABLE_TPSOT_FIX
-        if (bTPSOTCompliant)
+        if (m_bStrict)
         {
             decompressParams.flags |= OPJ_DPARAMETERS_DISABLE_TPSOT_FIX;
         }
@@ -750,6 +748,7 @@ struct OPJCodecWrapper
     jp2_image *psImage;
     jp2_image_comp_param *pasBandParams;
     JP2File *psJP2File;
+    bool m_bStrict = true;
 };
 
 /************************************************************************/
@@ -830,7 +829,7 @@ struct JP2OPJDatasetBase : public JP2DatasetBase
             opj_set_default_decoder_parameters(&parameters);
 
 #ifdef OPJ_DPARAMETERS_DISABLE_TPSOT_FIX
-            if (m_bTPSOTCompliant)
+            if (m_bStrict)
             {
                 parameters.flags |= OPJ_DPARAMETERS_DISABLE_TPSOT_FIX;
             }

diff --git a/frmts/opjlike/jp2opjlikedataset.cpp b/frmts/opjlike/jp2opjlikedataset.cpp
@@ -1302,15 +1302,12 @@ GDALDataset *JP2OPJLikeDataset<CODEC, BASE>::Open(GDALOpenInfo *poOpenInfo)
     auto eCodecFormat = (nCodeStreamStart == 0) ? CODEC::cvtenum(JP2_CODEC_J2K)
                                                 : CODEC::cvtenum(JP2_CODEC_JP2);
 
-    const bool bTPSOTCompliant =
-        CPLFetchBool(poOpenInfo->papszOpenOptions, "TPSOT_COMPLIANT", true);
-
     uint32_t nTileW = 0, nTileH = 0;
     int numResolutions = 0;
     CODEC localctx;
     localctx.open(poOpenInfo->fpL, nCodeStreamStart);
     if (!localctx.setUpDecompress(numThreads, nCodeStreamLength, &nTileW,
-                                  &nTileH, &numResolutions, bTPSOTCompliant))
+                                  &nTileH, &numResolutions))
         return nullptr;
 
     GDALDataType eDataType = GDT_Byte;
@@ -1384,7 +1381,6 @@ GDALDataset *JP2OPJLikeDataset<CODEC, BASE>::Open(GDALOpenInfo *poOpenInfo)
     poDS->bIs420 = bIs420;
     poDS->bSingleTiled = (poDS->nRasterXSize == (int)nTileW &&
                           poDS->nRasterYSize == (int)nTileH);
-    poDS->m_bTPSOTCompliant = bTPSOTCompliant;
     poDS->m_nX0 = localctx.psImage->x0;
     poDS->m_nY0 = localctx.psImage->y0;
     poDS->m_nTileWidth = nTileW;
@@ -1752,7 +1748,6 @@ GDALDataset *JP2OPJLikeDataset<CODEC, BASE>::Open(GDALOpenInfo *poOpenInfo)
         poODS->iLevel = poDS->nOverviewCount + 1;
         poODS->bSingleTiled = poDS->bSingleTiled;
         poODS->bUseSetDecodeArea = poDS->bUseSetDecodeArea;
-        poODS->m_bTPSOTCompliant = poDS->m_bTPSOTCompliant;
         poODS->nRedIndex = poDS->nRedIndex;
         poODS->nGreenIndex = poDS->nGreenIndex;
         poODS->nBlueIndex = poDS->nBlueIndex;

diff --git a/frmts/opjlike/jp2opjlikedataset.h b/frmts/opjlike/jp2opjlikedataset.h
@@ -97,7 +97,6 @@ struct JP2DatasetBase
     int nThreads = -1;
     bool bUseSetDecodeArea = false;
     bool bSingleTiled = false;
-    bool m_bTPSOTCompliant = true;
     int m_nBlocksToLoad = 0;
     int m_nX0 = 0;
     int m_nY0 = 0;