Fix code and improve unit tests to ensure nvimgcodec based decoder is tested alone (#573)

* Added saving decoded pixels for in deepth review if needed

Signed-off-by: M Q <mingmelvinq@nvidia.com>

* Fixed linting complaints

Signed-off-by: M Q <mingmelvinq@nvidia.com>

* Fixed the code and improve the tests with failed tests to be addressed.

Signed-off-by: M Q <mingmelvinq@nvidia.com>

* Force YBR for JEPG baseline, and test nvimgcodec without any decault decoders

Signed-off-by: M Q <mingmelvinq@nvidia.com>

* Critical changes make uncompressed images matching pydicom default decoders.

Signed-off-by: M Q <mingmelvinq@nvidia.com>

* Removed support for 12bit "JPEG Extended, Process 2+4"

Signed-off-by: M Q <mingmelvinq@nvidia.com>

* Address review comments including from AI agent

Signed-off-by: M Q <mingmelvinq@nvidia.com>

* Added reason for ignoring dcm files known to fail to uncompress

Signed-off-by: M Q <mingmelvinq@nvidia.com>

* Updated the notes on perf test results

Signed-off-by: M Q <mingmelvinq@nvidia.com>

* Explicitly minimized lazy loading impact and added comments on it.

Signed-off-by: M Q <mingmelvinq@nvidia.com>

* Updated doc sentences

Signed-off-by: M Q <mingmelvinq@nvidia.com>

* Editorial changes made to comments

Signed-off-by: M Q <mingmelvinq@nvidia.com>

---------

Signed-off-by: M Q <mingmelvinq@nvidia.com>

Author: MMelQin

monai/deploy/operators/decoder_nvimgcodec.py

@@ -1,4 +1,4 @@
-# Copyright 2025 MONAI Consortium
+# Copyright 2025-2026 MONAI Consortium
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
 # You may obtain a copy of the License at
@@ -13,7 +13,6 @@
 This decoder plugin for nvimgcodec <https://github.com/NVIDIA/nvImageCodec> decompresses
 encoded Pixel Data for the following transfer syntaxes:
     JPEGBaseline8Bit, 1.2.840.10008.1.2.4.50, JPEG Baseline (Process 1)
-    JPEGExtended12Bit, 1.2.840.10008.1.2.4.51, JPEG Extended (Process 2 & 4)
     JPEGLossless, 1.2.840.10008.1.2.4.57, JPEG Lossless, Non-Hierarchical (Process 14)
     JPEGLosslessSV1, 1.2.840.10008.1.2.4.70, JPEG Lossless, Non-Hierarchical, First-Order Prediction
     JPEG2000Lossless, 1.2.840.10008.1.2.4.90, JPEG 2000 Image Compression (Lossless Only)
@@ -75,7 +74,6 @@ def _decode_frame(src: bytes, runner: DecodeRunner) -> bytearray | bytes
     JPEG2000Decoder,
     JPEG2000LosslessDecoder,
     JPEGBaseline8BitDecoder,
-    JPEGExtended12BitDecoder,
     JPEGLosslessDecoder,
     JPEGLosslessSV1Decoder,
 )
@@ -118,7 +116,6 @@ def _decode_frame(src: bytes, runner: DecodeRunner) -> bytearray | bytes
 # Supported decoder classes of the corresponding transfer syntaxes by this decoder plugin.
 SUPPORTED_DECODER_CLASSES = [
     JPEGBaseline8BitDecoder,  # 1.2.840.10008.1.2.4.50, JPEG Baseline (Process 1)
-    JPEGExtended12BitDecoder,  # 1.2.840.10008.1.2.4.51, JPEG Extended (Process 2 & 4)
     JPEGLosslessDecoder,  # 1.2.840.10008.1.2.4.57, JPEG Lossless, Non-Hierarchical (Process 14)
     JPEGLosslessSV1Decoder,  # 1.2.840.10008.1.2.4.70, JPEG Lossless, Non-Hierarchical, First-Order Prediction
     JPEG2000LosslessDecoder,  # 1.2.840.10008.1.2.4.90, JPEG 2000 Image Compression (Lossless Only)
@@ -146,6 +143,8 @@ def _decode_frame(src: bytes, runner: DecodeRunner) -> bytearray | bytes
     for x in SUPPORTED_TRANSFER_SYNTAXES
 }
 
+DEFAULT_PI_NAME = "nvimgcodec_default_photometric_interpretation"
+
 
 # Required for decoder plugin
 def is_available(uid: UID) -> bool:
@@ -180,16 +179,18 @@ def _decode_frame(src: bytes, runner: DecodeRunner) -> bytearray | bytes:
     if not is_available(tsyntax):
         raise ValueError(f"Transfer syntax {tsyntax} not supported; see details in the debug log.")
 
-    runner.set_frame_option(runner.index, "decoding_plugin", "nvimgcodec")  # type: ignore[attr-defined]
-
+    # runner.set_frame_option(runner.index, "decoding_plugin", NVIMGCODEC_PLUGIN_LABEL)  # type: ignore[attr-defined]
+    # in pydicom v3.1.0 can use the above call, but do we want to limit to this plugin?
     is_jpeg2k = tsyntax in JPEG2000TransferSyntaxes
     samples_per_pixel = runner.samples_per_pixel
     photometric_interpretation = runner.photometric_interpretation
 
     # --- JPEG 2000: Precision/Bit depth ---
     if is_jpeg2k:
         precision, bits_allocated = _jpeg2k_precision_bits(runner)
-        runner.set_frame_option(runner.index, "bits_allocated", bits_allocated)  # type: ignore[attr-defined]
+        # runner.set_frame_option(runner.index, "bits_allocated", bits_allocated)  # type: ignore[attr-defined]
+        # in pydicom v3.1.0 can use the above call
+        runner.set_option("bits_allocated", bits_allocated)
         _logger.debug(f"Set bits_allocated to {bits_allocated} for J2K precision {precision}")
 
     # Check if RGB conversion requested (following Pillow decoder logic)
@@ -199,16 +200,18 @@ def _decode_frame(src: bytes, runner: DecodeRunner) -> bytearray | bytes:
 
     decoder = _get_decoder_resources()
     params = _get_decode_params(runner)
-    decoded_surface = decoder.decode(src, params=params).cpu()
-    np_surface = np.ascontiguousarray(np.asarray(decoded_surface))
-
-    # Handle JPEG2000-specific postprocessing separately
-    if is_jpeg2k:
-        np_surface = _jpeg2k_postprocess(np_surface, runner)
+    decoded_data = decoder.decode(src, params=params)
+    if decoded_data:
+        decoded_data = decoded_data.cpu()
+    else:
+        raise RuntimeError(f"Decoding failed: decoder.decode() returned a falsy value of type {type(decoded_data)}")
+    np_surface = np.ascontiguousarray(np.asarray(decoded_data))
 
     # Update photometric interpretation if we converted to RGB, or JPEG 2000 YBR*
     if convert_to_rgb or photometric_interpretation in (PI.YBR_ICT, PI.YBR_RCT):
-        runner.set_frame_option(runner.index, "photometric_interpretation", PI.RGB)  # type: ignore[attr-defined]
+        # runner.set_frame_option(runner.index, "photometric_interpretation", PI.RGB)  # type: ignore[attr-defined]
+        # in pydicom v3.1.0 can use the above call
+        runner.set_option("photometric_interpretation", PI.RGB)
         _logger.debug(
             "Set photometric_interpretation to RGB after conversion"
             if convert_to_rgb
@@ -227,7 +230,7 @@ def _get_decoder_resources() -> Any:
     global _NVIMGCODEC_DECODER
 
     if _NVIMGCODEC_DECODER is None:
-        _NVIMGCODEC_DECODER = nvimgcodec.Decoder()
+        _NVIMGCODEC_DECODER = nvimgcodec.Decoder(options=":fancy_upsampling=1")
 
     return _NVIMGCODEC_DECODER
 
@@ -252,7 +255,25 @@ def _get_decode_params(runner: RunnerBase) -> Any:
 
     # Access DICOM metadata from the runner
     samples_per_pixel = runner.samples_per_pixel
-    photometric_interpretation = runner.photometric_interpretation
+    photometric_interpretation = runner.get_option(DEFAULT_PI_NAME, runner.photometric_interpretation)
+
+    # we will change the PI at the end of the function if we convert to rgb
+    # but we need to have original PI to decide if we need to apply color transform for JPEG
+    if runner.get_option(DEFAULT_PI_NAME, None) is None:
+        runner.set_option(DEFAULT_PI_NAME, photometric_interpretation)
+
+    transfer_syntax = runner.transfer_syntax
+    as_rgb = runner.get_option("as_rgb", False)
+    force_rgb = runner.get_option("force_rgb", False)
+    force_ybr = runner.get_option("force_ybr", False)
+
+    _logger.debug("DecodeRunner options:")
+    _logger.debug(f"transfer_syntax: {transfer_syntax}")
+    _logger.debug(f"photometric_interpretation: {photometric_interpretation}")
+    _logger.debug(f"samples_per_pixel: {samples_per_pixel}")
+    _logger.debug(f"as_rgb: {as_rgb}")
+    _logger.debug(f"force_rgb: {force_rgb}")
+    _logger.debug(f"force_ybr: {force_ybr}")
 
     # Default: keep color space unchanged
     color_spec = nvimgcodec.ColorSpec.UNCHANGED
@@ -261,17 +282,26 @@ def _get_decode_params(runner: RunnerBase) -> Any:
     if samples_per_pixel > 1:
         # JPEG 2000 color transformations are always returned as RGB (matches Pillow)
         if photometric_interpretation in (PI.YBR_ICT, PI.YBR_RCT):
-            color_spec = nvimgcodec.ColorSpec.RGB
+            color_spec = nvimgcodec.ColorSpec.SRGB
             _logger.debug(
                 f"Using RGB color spec for JPEG 2000 color transformation " f"(PI: {photometric_interpretation})"
             )
+        elif transfer_syntax in (JPEGBaseline8BitDecoder.UID):
+            # approach is similar to pylibjpeg from pydicom - for ybr full and 422 it needs conversion from ycbcr to rgb
+            # for any other PI it just skips color conversion (ignoring what is inside jpeg header)
+            if photometric_interpretation in (PI.YBR_FULL, PI.YBR_FULL_422):
+                # we want to apply ycbcr -> rgb conversion
+                color_spec = nvimgcodec.ColorSpec.SRGB
+            else:
+                # ignore color conversion as image should already be in rgb or grayscale (but jpeg header may contain wrong data)
+                color_spec = nvimgcodec.ColorSpec.SYCC
         else:
             # Check the as_rgb option - same as Pillow decoder
-            convert_to_rgb = runner.get_option("as_rgb", False) and "YBR" in photometric_interpretation
+            convert_to_rgb = as_rgb or (force_rgb and "YBR" in photometric_interpretation)
 
             if convert_to_rgb:
                 # Convert YCbCr to RGB as requested
-                color_spec = nvimgcodec.ColorSpec.RGB
+                color_spec = nvimgcodec.ColorSpec.SRGB
                 _logger.debug(f"Using RGB color spec (as_rgb=True, PI: {photometric_interpretation})")
             else:
                 # Keep YCbCr unchanged - matches Pillow's image.draft("YCbCr") behavior
@@ -280,7 +310,10 @@ def _get_decode_params(runner: RunnerBase) -> Any:
                 )
     else:
         # Grayscale image - keep unchanged
-        _logger.debug(f"Using UNCHANGED color spec for grayscale image " f"(samples_per_pixel: {samples_per_pixel})")
+        _logger.debug(
+            f"Using UNCHANGED color spec for grayscale image (samples_per_pixel: {samples_per_pixel},"
+            f" PI: {photometric_interpretation}, transfer_syntax: {transfer_syntax})"
+        )
 
     return nvimgcodec.DecodeParams(
         allow_any_depth=True,
@@ -289,7 +322,9 @@ def _get_decode_params(runner: RunnerBase) -> Any:
 
 
 def _jpeg2k_precision_bits(runner: DecodeRunner) -> tuple[int, int]:
-    precision = runner.get_frame_option(runner.index, "j2k_precision", runner.bits_stored)  # type: ignore[attr-defined]
+    # precision = runner.get_frame_option(runner.index, "j2k_precision", runner.bits_stored)  # type: ignore[attr-defined]
+    # in pydicom v3.1.0 can use the above call
+    precision = runner.get_option("j2k_precision", runner.bits_stored)
     if 0 < precision <= 8:
         return precision, 8
     elif 8 < precision <= 16:
@@ -302,45 +337,6 @@ def _jpeg2k_precision_bits(runner: DecodeRunner) -> tuple[int, int]:
         raise ValueError(f"Only 'Bits Stored' values up to 16 are supported, got {precision}")
 
 
-def _jpeg2k_sign_correction(arr, dtype, bits_allocated):
-    arr = arr.view(dtype)
-    arr -= np.int32(2 ** (bits_allocated - 1))
-    _logger.debug("Applied J2K sign correction")
-    return arr
-
-
-def _jpeg2k_bitshift(arr, bit_shift):
-    np.right_shift(arr, bit_shift, out=arr)
-    _logger.debug(f"Applied J2K bit shift: {bit_shift} bits")
-    return arr
-
-
-def _jpeg2k_postprocess(np_surface, runner):
-    """Handle JPEG 2000 postprocessing: sign correction and bit shifts."""
-    precision = runner.get_frame_option(runner.index, "j2k_precision", runner.bits_stored)
-    bits_allocated = runner.get_frame_option(runner.index, "bits_allocated", runner.bits_allocated)
-    is_signed = runner.pixel_representation
-    if runner.get_option("apply_j2k_sign_correction", False):
-        is_signed = runner.get_frame_option(runner.index, "j2k_is_signed", is_signed)
-
-    # Sign correction for signed data
-    if is_signed and runner.pixel_representation == 1:
-        dtype = runner.frame_dtype(runner.index)
-        buffer = bytearray(np_surface.tobytes())
-        arr = np.frombuffer(buffer, dtype=f"<u{dtype.itemsize}")
-        np_surface = _jpeg2k_sign_correction(arr, dtype, bits_allocated)
-
-    # Bit shift if bits_allocated > precision
-    bit_shift = bits_allocated - precision
-    if bit_shift:
-        buffer = bytearray(np_surface.tobytes() if isinstance(np_surface, np.ndarray) else np_surface)
-        dtype = runner.frame_dtype(runner.index)
-        arr = np.frombuffer(buffer, dtype=dtype)
-        np_surface = _jpeg2k_bitshift(arr, bit_shift)
-
-    return np_surface
-
-
 def _is_nvimgcodec_available() -> bool:
     """Return ``True`` if nvimgcodec is available, ``False`` otherwise."""
 
@@ -416,7 +412,7 @@ def register_as_decoder_plugin(module_path: str | None = None) -> bool:
             continue
 
         decoder_class.add_plugin(NVIMGCODEC_PLUGIN_LABEL, (module_path, str(func_name)))
-        _logger.info(
+        _logger.debug(
             f"Added plugin for transfer syntax {decoder_class.UID}: "
             f"{NVIMGCODEC_PLUGIN_LABEL} with {func_name} in module path {module_path}."
         )
@@ -437,7 +433,8 @@ def unregister_as_decoder_plugin() -> bool:
     for decoder_class in SUPPORTED_DECODER_CLASSES:
         if NVIMGCODEC_PLUGIN_LABEL in decoder_class.available_plugins:
             decoder_class.remove_plugin(NVIMGCODEC_PLUGIN_LABEL)
-        _logger.info(f"Unregistered plugin for transfer syntax {decoder_class.UID}: {NVIMGCODEC_PLUGIN_LABEL}")
+        _logger.debug(f"Unregistered plugin for transfer syntax {decoder_class.UID}: {NVIMGCODEC_PLUGIN_LABEL}")
+    _logger.info(f"Unregistered plugin {NVIMGCODEC_PLUGIN_LABEL} for all supported transfer syntaxes.")
 
     return True
 

setup.cfg

@@ -55,6 +55,8 @@ ignore =
     B026
     #  B909 editing a loop's mutable iterable often leads to unexpected results/bugs
     B909
+    # F824 global variable is unused: name is never assigned in scope
+    F824
 
 per_file_ignores =
     # e.g. F403 'from holoscan.conditions import *' used; unable to detect undefined names