Fix uint32/int32 TIFF decoding (missing dtype mapping) by jantonguirao · Pull Request #6510 · NVIDIA/DALI

jantonguirao · 2026-09-29T08:03:54Z

Category:

Bug fix (non-breaking change which fixes an issue)

Description:

to_dali_dtype() in dali/operators/imgcodec/util/nvimagecodec_types.h was missing case
branches for NVIMGCODEC_SAMPLE_DATA_TYPE_UINT32 and NVIMGCODEC_SAMPLE_DATA_TYPE_INT32. Any
source image whose decoded sample type is 32-bit integer (e.g. a uint32 TIFF) hit the
default: return DALI_NO_TYPE; branch, which the decoder then turned into a hard runtime error:
Invalid sample_type: 8198.

While adding a proper regression test with a real signed 32-bit source, a second, related bug
surfaced: dali/operators/imgcodec/image_decoder.h's dynamic-range-scaling logic silently
corrupted pixel values for any signed integer source using the full width of its type (the
common case) — e.g. a genuinely signed int32 TIFF decoded to exactly half its correct value. This
is fixed too (see commit "Fix dynamic-range scaling for full-width signed integer sources").

The rest of the imgcodec conversion pipeline (dali/operators/imgcodec/util/convert.h's
IMGCODEC_TYPES, used by both the CPU and GPU conversion paths) already supports uint32_t/
int32_t samples, and DALI_UINT32/DALI_INT32 already exist as DALI dtypes — so both were
narrow, contained bugs rather than missing features.

This fixes the decoding half of NVIDIA/DALI#1246
for 32-bit TIFF. Note: the cpu (libtiff) backend still explicitly rejects any signed-format
TIFF outright (SAMPLEFORMAT_INT), regardless of bit depth — that's a separate limitation inside
nvImageCodec's LibTIFF extension itself, out of scope here. Only the mixed (GPU/nvtiff)
backend can currently decode a signed-integer TIFF at all.

Additional information:

Affected modules and functionalities:

dali/operators/imgcodec/util/nvimagecodec_types.h — to_dali_dtype().
dali/operators/imgcodec/image_decoder.h — dynamic-range-scaling precision handling for
signed dtypes.
dali/test/python/decoder/test_imgcodec.py — new test_tiff_uint32 and test_tiff_int32
test cases.
DALI_EXTRA_VERSION — bumped to pick up new uint32/int32 TIFF fixtures and their reference
.npy files (NVIDIA/DALI_extra#136).

Key points relevant for the review:

The core dtype-mapping change is a pure switch-statement extension; no other code paths change.
The dynamic-range-scaling fix is narrowly scoped to signed dtypes (IsSigned(orig_dtype)), so it
cannot affect any existing unsigned-type behavior — verified by regression-testing 7 other TIFF
variants + JPEG/PNG/BMP on both backends after the change (see Tests below).

FIXME before merge: DALI_EXTRA_VERSION currently points at a commit on my fork
(jantonguirao/DALI_extra), not NVIDIA/DALI_extra — it's only there so CI can see the new
fixtures ahead of the DALI_extra PR being merged. This must be updated to a commit on
NVIDIA/DALI_extra main once
NVIDIA/DALI_extra#136 merges, or CI checkout of
DALI_extra will fail for anyone without that fork remote configured.

Tests:

test_tiff_uint32 and test_tiff_int32 in dali/test/python/decoder/test_imgcodec.py:

test_tiff_uint32: decodes a uint32 TIFF with dtype=types.UINT32 on both cpu and mixed,
and compares the full-precision output against an independent reference decode (tifffile,
checked into DALI_extra as a .npy).
test_tiff_int32: decodes a signed int32 TIFF. On cpu, asserts the current (documented)
rejection from the LibTIFF extension. On mixed, compares the full-precision output against
the same kind of independent reference — this is what caught the dynamic-range scaling bug.

Also manually regression-checked 7 other TIFF variants (default uint8, uint16, grayscale, tiled,
BigTIFF, palette, JPEG-compressed-in-TIFF) plus JPEG/PNG/BMP on both cpu/mixed — all
unchanged after the scaling fix.

Checklist

Documentation

DALI team only

Requirements

Implements new requirements
Affects existing requirements
N/A

REQ IDs: N/A

JIRA TASK: N/A

copy-pr-bot · 2026-09-29T08:03:59Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

Copilot

Copilot review overview

🟡 Changes recommended

The decoding regression needs an automated uint32 TIFF test covering CPU and mixed backends.

Review effort: Balanced
Findings: 1

Open (1)

Add uint32 TIFF decode regression test · New

What changed in this PR

Adds missing 32-bit integer sample-type mappings to enable uint32 TIFF decoding.

Changes:

Maps nvImageCodec UINT32 and INT32 samples to corresponding DALI types.
Completes the existing imgcodec conversion pipeline’s type support.

File	Description
`dali/operators/imgcodec/util/nvimagecodec_types.h`	Adds UINT32 and INT32 dtype mappings.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

greptile-apps · 2026-09-29T08:12:17Z

Confidence Score: 5/5

[Medium risk] Fixes signed integer TIFF decoding and color conversion.

No new or outstanding review finding blocks this PR, but the fixture pin still needs the upstream update noted by the author before merge.

Summary

The PR adds 32-bit integer TIFF dtype mappings, corrects signed-sample scaling and CPU color conversion, and adds reference-based decoder and conversion tests. The latest change shares TIFF fixture filtering with the fused decoder tests.

_{Reviews (7) · Last reviewed commit: "Exclude signed TIFF fixture from test_im..."}

to_dali_dtype() was missing cases for NVIMGCODEC_SAMPLE_DATA_TYPE_UINT32 and NVIMGCODEC_SAMPLE_DATA_TYPE_INT32, causing the image decoder to throw "Invalid sample_type" for any 32-bit integer source (e.g. uint32 TIFF), even though the rest of the imgcodec conversion pipeline already supports these types via IMGCODEC_TYPES. Signed-off-by: Joaquin Anton Guirao <janton@nvidia.com>

Bumps DALI_EXTRA_VERSION to pick up the new db/single/tiff/0/cat-300572_640_uint32.tiff fixture (NVIDIA/DALI_extra#136). Signed-off-by: Joaquin Anton Guirao <janton@nvidia.com>

PositiveBits(dtype) subtracts one bit for signed DALI dtypes (reserving the sign bit), but the source `precision` reported by nvImageCodec (e.g. TIFF BitsPerSample) is the raw storage width, which already includes the sign bit for a signed sample format. Comparing them directly caused NeedDynamicRangeScaling() to spuriously trigger for any signed source using the full width of its type (the common case), silently corrupting pixel values (observed: exactly half the correct value for a signed int32 TIFF, since (2^31-1)/(2^32-1) ~= 0.5). Convert `precision` to the same "positive bits" basis as PositiveBits() before comparing, for signed sources. Also strengthens the uint32 TIFF decoder test to compare full-precision output against an independent reference (tifffile-derived .npy) on both cpu and mixed backends instead of only the top byte on cpu, and adds an analogous int32 test -- including an explicit assertion that the cpu (libtiff) backend still rejects signed TIFF samples, a known, separate limitation. Signed-off-by: Joaquin Anton Guirao <janton@nvidia.com>

Addresses the repo's test-edge-cases convention: covers a genuinely malformed/truncated 32-bit TIFF (valid header, no pixel data), distinct from the existing cpu signed-format-rejection case which tests a known backend limitation rather than invalid input. Signed-off-by: Joaquin Anton Guirao <janton@nvidia.com>

ConvertCPU ran the color-conversion matrix directly on raw sample values, but the coefficients/biases are calibrated for a channel in [0, max_value<T>()], as for a typical unsigned pixel type. A signed source's raw value spans [min_value<In>(), max_value<In>()], so a negative sample contributed a large negative term to the matrix instead of a small positive one, producing wildly wrong, unclamped output (only reachable now that mixed_decoder maps int32/uint32 dtypes and the CPU nvImageCodec extension can decode signed TIFF). ConvertGPU already avoids this by normalizing to the output dtype first and only then running the color conversion on same-typed data. Do the same on the CPU path, reusing the already-correct signed-aware scaling in ConvertSatNorm instead of teaching the color-math formulas about signedness. Signed-off-by: Joaquin Anton Guirao <janton@nvidia.com>

Addresses Greptile review comment: the new signed int32 TIFF tests exercise ConvertCPU only through the CPU TIFF decoder, which rejects all signed sample formats outright, so the color-conversion fix in 53afab5 was never actually exercised by a CPU-reachable path. Add a ConvertCPU-level GTest instead, calling the conversion directly with a synthetic signed source and comparing against an independently computed expected value. Confirmed it fails without the 53afab5 fix and passes with it. Also bump convert.h's copyright year for this substantially modified file. Signed-off-by: Joaquin Anton Guirao <janton@nvidia.com>

…r tests test_tiff_int32 assumed the CPU (libtiff) backend now decodes signed int32 TIFF, but the CPU nvImageCodec extension rejects all signed TIFF sample formats outright ("Unsupported sample format: 2"), regardless of bit depth - confirmed by actually building and running this on GPU hardware. Restore the cpu-rejection assertion; only the mixed/GPU backend is expected to decode it, per the PR description. The same fixture also broke test_image_decoder, test_FastDCT, and test_image_decoder_consistency: those tests glob/list every file under db/single/tiff/ and decode it via the generic fn.decoders.image() path without requesting dtype=INT32, which fails on both cpu and mixed for this file. Exclude it from those directory-wide file lists; it has its own dedicated coverage in test_tiff_int32. Also bump this substantially modified file's copyright year. Signed-off-by: Joaquin Anton Guirao <janton@nvidia.com>

Addresses Greptile review comment: create_decoder_slice_pipeline, create_decoder_crop_pipeline and create_decoder_random_crop_pipeline independently list every file in the TIFF directory via fn.readers.file(file_root=...), same as decoder_pipe, so they hit the same known CPU signed-TIFF rejection for a reason unrelated to fused decoding. Factor the exclude_names filtering used by decoder_pipe into a shared _reader_file_kwargs helper and apply it here too. Signed-off-by: Joaquin Anton Guirao <janton@nvidia.com>

jantonguirao · 2026-09-29T16:54:07Z

!build

dali-automaton · 2026-09-29T17:00:17Z

CI MESSAGE: [70490760]: BUILD STARTED

dali-automaton · 2026-09-29T18:07:04Z

CI MESSAGE: [70490760]: BUILD FAILED

Copilot AI balanced review requested due to automatic review settings September 29, 2026 08:03

Copilot started reviewing on behalf of jantonguirao September 29, 2026 08:04 View session

Copilot AI reviewed Sep 29, 2026

View reviewed changes

Comment thread dali/operators/imgcodec/util/nvimagecodec_types.h

jantonguirao mentioned this pull request Sep 29, 2026

Add (u)int32 TIFF fixtures for decoder testing NVIDIA/DALI_extra#136

Open

greptile-apps Bot reviewed Sep 29, 2026

View reviewed changes

Comment thread dali/test/python/decoder/test_imgcodec.py Outdated

Comment thread dali/test/python/decoder/test_imgcodec.py

greptile-apps Bot reviewed Sep 29, 2026

View reviewed changes

Comment thread DALI_EXTRA_VERSION

Comment thread dali/test/python/decoder/test_imgcodec.py

jantonguirao force-pushed the fix/tiff-uint32-decode branch from 3b8a37f to 6ef2224 Compare September 29, 2026 09:31

dali-automaton assigned mdabek-nvidia and banasraf Sep 29, 2026

greptile-apps Bot reviewed Sep 29, 2026

View reviewed changes

Comment thread dali/test/python/decoder/test_imgcodec.py

Comment thread dali/operators/imgcodec/util/convert.h

jantonguirao mentioned this pull request Sep 29, 2026

Fix RGB<->YCbCr/GRAY color conversion for signed and float pixel types #6514

Open

5 of 18 tasks

jantonguirao added 7 commits September 29, 2026 18:10

Add test for uint32 TIFF decoding

5c48c88

Bumps DALI_EXTRA_VERSION to pick up the new db/single/tiff/0/cat-300572_640_uint32.tiff fixture (NVIDIA/DALI_extra#136). Signed-off-by: Joaquin Anton Guirao <janton@nvidia.com>

jantonguirao force-pushed the fix/tiff-uint32-decode branch from 53afab5 to 530f595 Compare September 29, 2026 16:10

greptile-apps Bot reviewed Sep 29, 2026

View reviewed changes

Comment thread dali/test/python/decoder/test_imgcodec.py

Navigation Menu

Fix uint32/int32 TIFF decoding (missing dtype mapping) - #6510

Fix uint32/int32 TIFF decoding (missing dtype mapping)#6510
jantonguirao wants to merge 8 commits into
NVIDIA:mainfrom
jantonguirao:fix/tiff-uint32-decode

jantonguirao commented Sep 29, 2026 •

edited

Loading

Uh oh!

copy-pr-bot Bot commented Sep 29, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

greptile-apps Bot commented Sep 29, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jantonguirao commented Sep 29, 2026

Uh oh!

dali-automaton commented Sep 29, 2026

Uh oh!

dali-automaton commented Sep 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

jantonguirao commented Sep 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Category:

Description:

Additional information:

Affected modules and functionalities:

Key points relevant for the review:

Tests:

Checklist

Documentation

DALI team only

Requirements

Uh oh!

copy-pr-bot Bot commented Sep 29, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

greptile-apps Bot commented Sep 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 5/5

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jantonguirao commented Sep 29, 2026

Uh oh!

dali-automaton commented Sep 29, 2026

Uh oh!

dali-automaton commented Sep 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jantonguirao commented Sep 29, 2026 •

edited

Loading

greptile-apps Bot commented Sep 29, 2026 •

edited

Loading