From 7368f2d43a7dcb3e84bf2f0b1f82626562d1c5a9 Mon Sep 17 00:00:00 2001
From: Even Rouault <even.rouault@spatialys.com>
Date: Fri, 4 Oct 2024 17:56:09 +0200
Subject: [PATCH] NITF: properly take into account comma-separated list of
 values for JPEG2000 QUALITY

when JPEG2000_DRIVER=JP2OpenJPEG

Fixes #10927
---
 autotest/gdrivers/nitf.py          | 28 +++++++++++++++++++
 doc/source/drivers/raster/nitf.rst |  6 +++--
 frmts/nitf/nitfdataset.cpp         | 43 +++++++++++++++++++++++-------
 3 files changed, 66 insertions(+), 11 deletions(-)

diff --git a/autotest/gdrivers/nitf.py b/autotest/gdrivers/nitf.py
index 582b8622d094..702cf95d5494 100755
--- a/autotest/gdrivers/nitf.py
+++ b/autotest/gdrivers/nitf.py
@@ -986,9 +986,36 @@ def test_nitf_28_jp2openjpeg_bis(tmp_path):
             createcopy=True,
         )
         ds = gdal.Open(filename)
+        size = os.stat(filename).st_size
         assert ds.GetRasterBand(1).Checksum() in (31604, 31741)
         ds = None
 
+        nitf_create(
+            filename,
+            ["ICORDS=G", "IC=C8", "QUALITY=1,25"],
+            set_inverted_color_interp=False,
+            createcopy=True,
+        )
+        ds = gdal.Open(filename)
+        size2 = os.stat(filename).st_size
+        assert ds.GetRasterBand(1).Checksum() in (31604, 31741)
+        ds = None
+
+        assert size2 > size
+
+        nitf_create(
+            filename,
+            ["ICORDS=G", "IC=C8", "QUALITY=1.9,25"],
+            set_inverted_color_interp=False,
+            createcopy=True,
+        )
+        ds = gdal.Open(filename)
+        size3 = os.stat(filename).st_size
+        assert ds.GetRasterBand(1).Checksum() in (31604, 31741)
+        ds = None
+
+        assert size3 > size2
+
         tmpfilename = "/vsimem/nitf_28_jp2openjpeg_bis.ntf"
         src_ds = gdal.GetDriverByName("MEM").Create("", 1025, 1025)
         gdal.GetDriverByName("NITF").CreateCopy(tmpfilename, src_ds, options=["IC=C8"])
@@ -1100,6 +1127,7 @@ def test_nitf_jp2openjpeg_npje_numerically_lossless(tmp_vsimem):
                 "IC=C8",
                 "JPEG2000_DRIVER=JP2OpenJPEG",
                 "PROFILE=NPJE_NUMERICALLY_LOSSLESS",
+                "QUALITY=10,100",
             ],
         )
 
diff --git a/doc/source/drivers/raster/nitf.rst b/doc/source/drivers/raster/nitf.rst
index 6db8c38ede1e..4d95ded1e8ba 100644
--- a/doc/source/drivers/raster/nitf.rst
+++ b/doc/source/drivers/raster/nitf.rst
@@ -131,10 +131,12 @@ The following creation options are available:
          CreateCopy() and/or Create() methods. See below paragraph for specificities.
 
 -  .. co:: QUALITY
-      :choices: 10-100
       :default: 75
 
-      JPEG quality 10-100
+      For JPEG, quality as integer values in the 10-100 range
+      For JPEG2000, quality as a floating-point value in >0 - 100 range.
+      When JPEG2000_DRIVER=JP2OpenJPEG and PROFILE is not one of the NPJE ones,
+      several quality layers can be specified as a comma-separated list of values.
 
 -  .. co:: PROGRESSIVE
       :choices: YES, NO
diff --git a/frmts/nitf/nitfdataset.cpp b/frmts/nitf/nitfdataset.cpp
index c546c57d4d2a..812431b83f85 100644
--- a/frmts/nitf/nitfdataset.cpp
+++ b/frmts/nitf/nitfdataset.cpp
@@ -4018,8 +4018,15 @@ static char **NITFJP2OPENJPEGOptions(GDALDriver *poJ2KDriver,
 {
     char **papszJP2Options = CSLAddString(nullptr, "CODEC=J2K");
 
-    double dfQuality =
-        CPLAtof(CSLFetchNameValueDef(papszOptions, "QUALITY", "0"));
+    const char *pszQuality = CSLFetchNameValue(papszOptions, "QUALITY");
+    double dfQuality = 0;
+    if (pszQuality)
+    {
+        for (const char *pszVal :
+             CPLStringList(CSLTokenizeString2(pszQuality, ",", 0)))
+            dfQuality = std::max(dfQuality, CPLAtof(pszVal));
+    }
+
     double dfTarget =
         CPLAtof(CSLFetchNameValueDef(papszOptions, "TARGET", "0"));
 
@@ -4036,10 +4043,10 @@ static char **NITFJP2OPENJPEGOptions(GDALDriver *poJ2KDriver,
     }
 
     // Set it now before the NPJE profiles have a chance to override it
-    if (dfQuality > 0)
+    if (pszQuality)
     {
-        papszJP2Options = CSLSetNameValue(papszJP2Options, "QUALITY",
-                                          CPLSPrintf("%f", dfQuality));
+        papszJP2Options =
+            CSLSetNameValue(papszJP2Options, "QUALITY", pszQuality);
     }
 
     const char *pszProfile = CSLFetchNameValueDef(papszOptions, "PROFILE", "");
@@ -4050,6 +4057,14 @@ static char **NITFJP2OPENJPEGOptions(GDALDriver *poJ2KDriver,
         // (https://nsgreg.nga.mil/doc/view?i=2031&month=3&day=22&year=2021),
         // for NPJE (Appendix D ) profile
 
+        if (pszQuality && strchr(pszQuality, ','))
+        {
+            CPLError(CE_Warning, CPLE_AppDefined,
+                     "Only largest value of QUALITY used when PROFILE=%s "
+                     "is specified",
+                     pszProfile);
+        }
+
         papszJP2Options =
             CSLAddString(papszJP2Options, "@BLOCKSIZE_STRICT=YES");
 
@@ -7051,11 +7066,21 @@ void NITFDriver::InitCreationOptionList()
     if (bHasJPEG2000Drivers)
         osCreationOptions += "       <Value>C8</Value>";
 
-    osCreationOptions +=
-        "   </Option>"
+    osCreationOptions += "   </Option>";
+
+#if !defined(JPEG_SUPPORTED)
+    if (bHasJPEG2000Drivers)
+#endif
+    {
+        osCreationOptions +=
+            "   <Option name='QUALITY' type='string' "
+            "description='JPEG (10-100) or JPEG2000 quality, possibly as a"
+            "separated list of values for JPEG2000_DRIVER=JP2OPENJPEG' "
+            "default='75'/>";
+    }
+
 #ifdef JPEG_SUPPORTED
-        "   <Option name='QUALITY' type='int' description='JPEG quality "
-        "10-100' default='75'/>"
+    osCreationOptions +=
         "   <Option name='PROGRESSIVE' type='boolean' description='JPEG "
         "progressive mode'/>"
         "   <Option name='RESTART_INTERVAL' type='int' description='Restart "