From a7bfb11ac02a9274289cee11ebf10dacb3bc8f97 Mon Sep 17 00:00:00 2001 From: M Q Date: Mon, 8 Dec 2025 18:03:02 -0800 Subject: [PATCH 1/3] Added saving decoded pixels for in deepth review if needed Signed-off-by: M Q --- tests/unit/test_decoder_nvimgcodec.py | 127 ++++++++++++++++++++++++-- 1 file changed, 121 insertions(+), 6 deletions(-) diff --git a/tests/unit/test_decoder_nvimgcodec.py b/tests/unit/test_decoder_nvimgcodec.py index 7556f00f..1976e59c 100644 --- a/tests/unit/test_decoder_nvimgcodec.py +++ b/tests/unit/test_decoder_nvimgcodec.py @@ -1,6 +1,6 @@ import time from pathlib import Path -from typing import Any +from typing import TYPE_CHECKING, Any, cast import numpy as np import pytest @@ -14,6 +14,108 @@ unregister_as_decoder_plugin, ) +try: + from PIL import Image as PILImage +except Exception: # pragma: no cover - Pillow may be unavailable in some environments + PILImage = None # type: ignore[assignment] + +if TYPE_CHECKING: + from PIL.Image import Image as PILImageType +else: + PILImageType = Any + +_PNG_EXPORT_WARNING_EMITTED = False + + +def _iter_frames(pixel_array: np.ndarray): + """Yield per-frame arrays and whether they represent color data.""" + arr = np.asarray(pixel_array) + if arr.ndim == 2: + yield 0, arr, False + return + + if arr.ndim == 3: + if arr.shape[-1] in (3, 4): + yield 0, arr, True + else: + for index in range(arr.shape[0]): + frame = arr[index] + is_color = frame.ndim == 3 and frame.shape[-1] in (3, 4) + yield index, frame, is_color + return + + if arr.ndim == 4: + for index in range(arr.shape[0]): + frame = arr[index] + is_color = frame.ndim == 3 and frame.shape[-1] in (3, 4) + yield index, frame, is_color + return + + raise ValueError(f"Unsupported pixel array shape {arr.shape!r} for PNG export") + + +def _prepare_frame_for_png(frame: np.ndarray, is_color: bool) -> np.ndarray: + """Convert a decoded frame into a dtype supported by PNG writers.""" + arr = np.nan_to_num(np.asarray(frame), copy=False) + + # Remove singleton channel dimension for grayscale data. + if not is_color and arr.ndim == 3 and arr.shape[-1] == 1: + arr = arr[..., 0] + + if is_color: + if arr.dtype == np.uint8: + return arr + arr_float = arr.astype(np.float64, copy=False) + arr_min = float(arr_float.min()) + arr_max = float(arr_float.max()) + if arr_max == arr_min: + return np.zeros_like(arr, dtype=np.uint8) + scaled = (arr_float - arr_min) / (arr_max - arr_min) + return np.clip(np.round(scaled * 255.0), 0, 255).astype(np.uint8) + + # Grayscale path + arr_min = float(arr.min()) + arr_max = float(arr.max()) + + if np.issubdtype(arr.dtype, np.integer): + if arr_min >= 0 and arr_max <= 255: + return arr.astype(np.uint8, copy=False) + if arr_min >= 0 and arr_max <= 65535: + return arr.astype(np.uint16, copy=False) + + arr_float = arr.astype(np.float64, copy=False) + arr_min = float(arr_float.min()) + arr_max = float(arr_float.max()) + if arr_max == arr_min: + return np.zeros_like(arr_float, dtype=np.uint8) + + use_uint16 = arr_max - arr_min > 255.0 + scale = 65535.0 if use_uint16 else 255.0 + scaled = (arr_float - arr_min) / (arr_max - arr_min) + scaled = np.clip(np.round(scaled * scale), 0, scale) + target_dtype = np.uint16 if use_uint16 else np.uint8 + return scaled.astype(target_dtype) + + +def _save_frames_as_png(pixel_array: np.ndarray, output_dir: Path, file_stem: str) -> None: + """Persist each frame as a PNG image in the specified directory.""" + global _PNG_EXPORT_WARNING_EMITTED + + if PILImage is None: + if not _PNG_EXPORT_WARNING_EMITTED: + print("Skipping PNG export because Pillow is not installed.") + _PNG_EXPORT_WARNING_EMITTED = True + return + + output_dir.mkdir(parents=True, exist_ok=True) + pil_image_cls = cast(Any, PILImage) + + for frame_index, frame, is_color in _iter_frames(pixel_array): + frame_for_png = _prepare_frame_for_png(frame, is_color) + image = pil_image_cls.fromarray(frame_for_png) + filename = output_dir / f"{file_stem}_frame_{frame_index:04d}.png" + image.save(filename) + def get_test_dicoms(folder_path: str | None = None): """Use pydicom package's embedded test DICOM files for testing or a custom folder of DICOM files.""" @@ -98,15 +200,20 @@ def test_nvimgcodec_decoder_matches_default(path: str) -> None: np.testing.assert_allclose(baseline_pixels, nv_pixels, rtol=rtol, atol=atol) -def performance_test_nvimgcodec_decoder_against_defaults(folder_path: str | None = None) -> None: +def performance_test_nvimgcodec_decoder_against_defaults( + folder_path: str | None = None, png_output_dir: str | None = None +) -> None: """Test and compare the performance of the nvimgcodec decoder against the default decoders - with all DICOM files of supported transfer syntaxes in a custom folder or pidicom dataset""" + with all DICOM files of supported transfer syntaxes in a custom folder or pidicom dataset. + + If `png_output_dir` is provided, decoded frames are saved as PNG files for both decoders.""" total_baseline_time = 0.0 total_nvimgcodec_time = 0.0 files_tested_with_perf: dict[str, dict[str, Any]] = {} # key: path, value: performance_metrics files_with_errors = [] + png_root = Path(png_output_dir).expanduser() if png_output_dir else None try: unregister_as_decoder_plugin() # Make sure nvimgcodec decoder plugin is not registered @@ -118,7 +225,7 @@ def performance_test_nvimgcodec_decoder_against_defaults(folder_path: str | None ds_default = dcmread(path) transfer_syntax = ds_default.file_meta.TransferSyntaxUID start = time.perf_counter() - _ = ds_default.pixel_array + baseline_pixels = ds_default.pixel_array baseline_execution_time = time.perf_counter() - start total_baseline_time += baseline_execution_time @@ -126,6 +233,10 @@ def performance_test_nvimgcodec_decoder_against_defaults(folder_path: str | None perf["transfer_syntax"] = transfer_syntax perf["baseline_execution_time"] = baseline_execution_time files_tested_with_perf[path] = perf + + if png_root is not None: + baseline_dir = png_root / Path(path).stem / "default" + _save_frames_as_png(baseline_pixels, baseline_dir, Path(path).stem) except Exception: files_with_errors.append(Path(path).name) continue @@ -137,10 +248,14 @@ def performance_test_nvimgcodec_decoder_against_defaults(folder_path: str | None try: ds_custom = dcmread(path) start = time.perf_counter() - _ = ds_custom.pixel_array + nv_pixels = ds_custom.pixel_array perf["nvimgcodec_execution_time"] = time.perf_counter() - start total_nvimgcodec_time += perf["nvimgcodec_execution_time"] combined_perf[path] = perf + + if png_root is not None: + nv_dir = png_root / Path(path).stem / "nvimgcodec" + _save_frames_as_png(nv_pixels, nv_dir, Path(path).stem) except Exception: continue unregister_as_decoder_plugin() @@ -174,4 +289,4 @@ def performance_test_nvimgcodec_decoder_against_defaults(folder_path: str | None # # The following compares the performance of the nvimgcodec decoder against the default decoders # with DICOM files in pidicom embedded dataset or an optional custom folder - performance_test_nvimgcodec_decoder_against_defaults() # e.g. "/tmp/multi-frame-dcm" + performance_test_nvimgcodec_decoder_against_defaults() # or use ("/tmp/multi-frame-dcm", png_output_dir="decoded_png") From 3ca3647ab3df79f015b0328f67018a071ca33dd4 Mon Sep 17 00:00:00 2001 From: M Q Date: Mon, 8 Dec 2025 18:19:20 -0800 Subject: [PATCH 2/3] Fixed linting complaints Signed-off-by: M Q --- tests/unit/test_decoder_nvimgcodec.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/unit/test_decoder_nvimgcodec.py b/tests/unit/test_decoder_nvimgcodec.py index 1976e59c..52ccce7a 100644 --- a/tests/unit/test_decoder_nvimgcodec.py +++ b/tests/unit/test_decoder_nvimgcodec.py @@ -1,6 +1,6 @@ import time from pathlib import Path -from typing import TYPE_CHECKING, Any, cast +from typing import TYPE_CHECKING, Any, Iterator, cast import numpy as np import pytest @@ -27,7 +27,7 @@ _PNG_EXPORT_WARNING_EMITTED = False -def _iter_frames(pixel_array: np.ndarray): +def _iter_frames(pixel_array: np.ndarray) -> Iterator[tuple[int, np.ndarray, bool]]: """Yield per-frame arrays and whether they represent color data.""" arr = np.asarray(pixel_array) if arr.ndim == 2: @@ -204,7 +204,7 @@ def performance_test_nvimgcodec_decoder_against_defaults( folder_path: str | None = None, png_output_dir: str | None = None ) -> None: """Test and compare the performance of the nvimgcodec decoder against the default decoders - with all DICOM files of supported transfer syntaxes in a custom folder or pidicom dataset. + with all DICOM files of supported transfer syntaxes in a custom folder or pydicom embedded dataset. If `png_output_dir` is provided, decoded frames are saved as PNG files for both decoders.""" @@ -288,5 +288,5 @@ def performance_test_nvimgcodec_decoder_against_defaults( # python -m pytest test_decoder_nvimgcodec.py # # The following compares the performance of the nvimgcodec decoder against the default decoders - # with DICOM files in pidicom embedded dataset or an optional custom folder + # with DICOM files in pydicom embedded dataset or an optional custom folder performance_test_nvimgcodec_decoder_against_defaults() # or use ("/tmp/multi-frame-dcm", png_output_dir="decoded_png") From 1a437ddd55abba10b9b4861d0a2edaf9b6030cb1 Mon Sep 17 00:00:00 2001 From: M Q Date: Tue, 9 Dec 2025 18:57:02 -0800 Subject: [PATCH 3/3] Fixed the code and improve the tests with failed tests to be addressed. Signed-off-by: M Q --- monai/deploy/operators/decoder_nvimgcodec.py | 48 ++++++++++++----- setup.cfg | 2 + tests/unit/test_decoder_nvimgcodec.py | 55 +++++++++++++++++--- 3 files changed, 83 insertions(+), 22 deletions(-) diff --git a/monai/deploy/operators/decoder_nvimgcodec.py b/monai/deploy/operators/decoder_nvimgcodec.py index 74d0d7ad..98a1c9f9 100644 --- a/monai/deploy/operators/decoder_nvimgcodec.py +++ b/monai/deploy/operators/decoder_nvimgcodec.py @@ -180,8 +180,9 @@ 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") # type: ignore[attr-defined] + # in pydicom v3.1.0 can use the above call + runner.set_option("decoding_plugin", "nvimgcodec") is_jpeg2k = tsyntax in JPEG2000TransferSyntaxes samples_per_pixel = runner.samples_per_pixel photometric_interpretation = runner.photometric_interpretation @@ -189,7 +190,9 @@ def _decode_frame(src: bytes, runner: DecodeRunner) -> bytearray | bytes: # --- 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 abover 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,8 +202,12 @@ 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)) + decoded_data = decoder.decode(src, params=params) + if decoded_data: + decoded_data = decoded_data.cpu() + else: + raise RuntimeError(f"Decoded data is None: {type(decoded_data)}") + np_surface = np.ascontiguousarray(np.asarray(decoded_data)) # Handle JPEG2000-specific postprocessing separately if is_jpeg2k: @@ -208,7 +215,9 @@ def _decode_frame(src: bytes, runner: DecodeRunner) -> bytearray | bytes: # 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 pydicon 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 @@ -261,7 +270,7 @@ 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})" ) @@ -271,7 +280,7 @@ def _get_decode_params(runner: RunnerBase) -> Any: 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 @@ -289,7 +298,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: @@ -317,15 +328,22 @@ def _jpeg2k_bitshift(arr, bit_shift): 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) + # precision = runner.get_frame_option("j2k_precision", runner.bits_stored) + # bits_allocated = runner.get_frame_option(runner.index, "bits_allocated", runner.bits_allocated) + # in pydicom v3.1.0 can use the above calls + precision = runner.get_option("j2k_precision", runner.bits_stored) + bits_allocated = runner.get_option("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) + # is_signed = runner.get_frame_option(runner.index, "j2k_is_signed", is_signed) + # in pydicom v3.1.0 can use the above call + is_signed = runner.get_option("j2k_is_signed", is_signed) # Sign correction for signed data if is_signed and runner.pixel_representation == 1: - dtype = runner.frame_dtype(runner.index) + # dtype = runner.frame_dtype(runner.index) + # in pydicomv3.1.0 can use the above call + dtype = runner.pixel_dtype buffer = bytearray(np_surface.tobytes()) arr = np.frombuffer(buffer, dtype=f" Iterator[tuple[int, np.ndarray, bool]]: """Yield per-frame arrays and whether they represent color data.""" @@ -103,7 +108,7 @@ def _save_frames_as_png(pixel_array: np.ndarray, output_dir: Path, file_stem: st if PILImage is None: if not _PNG_EXPORT_WARNING_EMITTED: - print("Skipping PNG export because Pillow is not installed.") + _logger.info("Skipping PNG export because Pillow is not installed.") _PNG_EXPORT_WARNING_EMITTED = True return @@ -174,6 +179,8 @@ def test_nvimgcodec_decoder_matches_default(path: str) -> None: default_decoder_error_message = f"{e}" default_decoder_errored = True + # Remove and cache the other default decoder plugins first + _remove_default_plugins() # Register the nvimgcodec decoder plugin and unregister it after each use. register_as_decoder_plugin() try: @@ -184,20 +191,26 @@ def test_nvimgcodec_decoder_matches_default(path: str) -> None: nvimgcodec_decoder_errored = True finally: unregister_as_decoder_plugin() + _restore_default_plugins() if default_decoder_errored and nvimgcodec_decoder_errored: - print( + _logger.info( f"All decoders encountered errors for transfer syntax {transfer_syntax} in {Path(path).name}:\n" f"Default decoder error: {default_decoder_error_message}\n" f"nvimgcodec decoder error: {nvimgcodec_decoder_error_message}" ) return elif nvimgcodec_decoder_errored and not default_decoder_errored: - raise AssertionError(f"nvimgcodec decoder errored: {nvimgcodec_decoder_errored} but default decoder succeeded") + raise AssertionError( + f"nvimgcodec decoder errored but default decoder succeeded for transfer syntax {transfer_syntax}" + ) - assert baseline_pixels.shape == nv_pixels.shape, f"Shape mismatch for {Path(path).name}" - assert baseline_pixels.dtype == nv_pixels.dtype, f"Dtype mismatch for {Path(path).name}" - np.testing.assert_allclose(baseline_pixels, nv_pixels, rtol=rtol, atol=atol) + assert baseline_pixels.shape == nv_pixels.shape, f"Shape mismatch with transfer syntax {transfer_syntax}" + assert baseline_pixels.dtype == nv_pixels.dtype, f"Dtype mismatch with transfer syntax {transfer_syntax}" + try: + np.testing.assert_allclose(baseline_pixels, nv_pixels, rtol=rtol, atol=atol) + except AssertionError as e: + raise AssertionError(f"Pixels values mismatch with transfer syntax {transfer_syntax}") from e def performance_test_nvimgcodec_decoder_against_defaults( @@ -241,8 +254,10 @@ def performance_test_nvimgcodec_decoder_against_defaults( files_with_errors.append(Path(path).name) continue + _remove_default_plugins() # Register the nvimgcodec decoder plugin and unregister it after each use. register_as_decoder_plugin() + combined_perf = {} for path, perf in files_tested_with_perf.items(): try: @@ -256,9 +271,11 @@ def performance_test_nvimgcodec_decoder_against_defaults( if png_root is not None: nv_dir = png_root / Path(path).stem / "nvimgcodec" _save_frames_as_png(nv_pixels, nv_dir, Path(path).stem) - except Exception: + except Exception as e: + _logger.info(f"Error decoding {path} with nvimgcodec decoder: {e}") continue unregister_as_decoder_plugin() + _restore_default_plugins() # Performance of the nvimgcodec decoder against the default decoders # with all DICOM files of supported transfer syntaxes @@ -282,6 +299,26 @@ def performance_test_nvimgcodec_decoder_against_defaults( print(f"\n\n__Files not tested due to errors encountered by default decoders__: \n{files_with_errors}") +def _remove_default_plugins(): + """Remove the default plugins from the supported decoder classes.""" + + global _DEFAULT_PLUGIN_CACHE + for decoder_class in SUPPORTED_DECODER_CLASSES: + _DEFAULT_PLUGIN_CACHE[decoder_class.UID.name] = ( + decoder_class._available + ) # while box, no API to get DecodeFunction + _logger.info(f"Removing default plugins of {decoder_class}: {decoder_class.available_plugins}.") + decoder_class._available = {} # remove all plugins, ref is still held by _DEFAULT_PLUGIN_CACHE + _logger.info(f"Removed default plugins of {decoder_class}: {decoder_class.available_plugins}.") + + +def _restore_default_plugins(): + """Restore the default plugins to the supported decoder classes.""" + for decoder_class in SUPPORTED_DECODER_CLASSES: + decoder_class._available = _DEFAULT_PLUGIN_CACHE[decoder_class.UID.name] # restore all plugins + _logger.info(f"Restored default plugins of {decoder_class}: {decoder_class.available_plugins}.") + + if __name__ == "__main__": # Use pytest to test the functionality with pydicom embedded DICOM files of supported transfer syntaxes individually @@ -289,4 +326,6 @@ def performance_test_nvimgcodec_decoder_against_defaults( # # The following compares the performance of the nvimgcodec decoder against the default decoders # with DICOM files in pydicom embedded dataset or an optional custom folder - performance_test_nvimgcodec_decoder_against_defaults() # or use ("/tmp/multi-frame-dcm", png_output_dir="decoded_png") + performance_test_nvimgcodec_decoder_against_defaults( + png_output_dir="decoded_png" + ) # or use ("/tmp/multi-frame-dcm", png_output_dir="decoded_png")