Suppress spurious libtiff warnings when decoding GeoTIFF files#6336
Suppress spurious libtiff warnings when decoding GeoTIFF files#6336shreyaskommuri wants to merge 4 commits intoNVIDIA:mainfrom
Conversation
GeoTIFF files embed geographic metadata in five TIFF tags (ModelPixelScaleTag 33550, ModelTiepointTag 33922, GeoKeyDirectoryTag 34735, GeoDoubleParamsTag 34736, GeoAsciiParamsTag 34737) plus two common GDAL tags (42112, 42113). libtiff does not know these tags natively and emits "Unknown field with tag X encountered" warnings for each one. Install a custom TIFFErrorHandler in the ImageDecoder constructor (guarded by LIBTIFF_ENABLED) that silently drops warnings for these known GeoTIFF/GDAL tags and forwards all other warnings to stderr unchanged. The handler is installed once per process via std::call_once. Also add test_image_decoder_geotiff which builds a minimal GeoTIFF from scratch (no third-party dependency) and decodes it with both CPU and mixed backends, asserting correct pixel values. Fixes: NVIDIA#6114 Signed-off-by: shreyaskommuri <shreyaskommuri@gmail.com>
|
| Filename | Overview |
|---|---|
| dali/operators/imgcodec/image_decoder.h | Adds an inline GeoTIFF warning filter installed via TIFFSetWarningHandler in the ImageDecoder constructor. The null-module guard and va_copy usage are correct; the strstr coupling to libtiff's internal format string is a minor fragility. |
| dali/test/python/decoder/test_image.py | Adds _create_geotiff helper and test_image_decoder_geotiff test. The synthetic TIFF construction is TIFF-spec-correct. Three of the eight suppressed tags (34264, 42112, 42113) are absent from the test fixture, leaving those suppression paths untested. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[libtiff emits warning\nmodule, fmt, va_list ap] --> B{strstr fmt\n'Unknown field with tag'?}
B -- No --> F[vsnprintf buf fmt ap\nwrite to stderr]
B -- Yes --> C[va_copy ap → ap_copy\nva_arg ap_copy → tag id]
C --> D{tag in kGeoTIFFTags?\n33550 33922 34264\n34735 34736 34737\n42112 42113}
D -- Yes --> E[Return — warning suppressed]
D -- No --> F
F --> G[module != null?]
G -- Yes --> H[cerr: module + buf]
G -- No --> I[cerr: buf]
Reviews (5): Last reviewed commit: "Remove unused get_gpu_num import (fixes ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| char buf[1024]; | ||
| vsnprintf(buf, sizeof(buf), fmt, ap); | ||
| std::cerr << module << ": " << buf << "\n"; |
There was a problem hiding this comment.
module is null. The TIFFWarningHandler contract permits a null module argument — libtiff calls TIFFWarning(NULL, fmt, ...) in several places. Writing std::cerr << module with a null const char* is undefined behaviour that manifests as a crash or garbled output on most platforms. Guard before the fallthrough write.
| char buf[1024]; | |
| vsnprintf(buf, sizeof(buf), fmt, ap); | |
| std::cerr << module << ": " << buf << "\n"; | |
| char buf[1024]; | |
| vsnprintf(buf, sizeof(buf), fmt, ap); | |
| if (module) | |
| std::cerr << module << ": " << buf << "\n"; | |
| else | |
| std::cerr << buf << "\n"; |
| void InstallGeoTIFFWarningFilter() { | ||
| static std::once_flag flag; | ||
| std::call_once(flag, [] { TIFFSetWarningHandler(SuppressGeoTIFFTagWarnings); }); | ||
| } |
There was a problem hiding this comment.
std::call_once guarantees "at most once per std::once_flag instance." Because InstallGeoTIFFWarningFilter lives in an anonymous namespace inside a header, each translation unit that includes this header gets its own copy of the function and its own static once_flag. If two .cc files include image_decoder.h, both will call TIFFSetWarningHandler (once per TU), overwriting each other's handler. The last-written handler wins and is still functionally correct (since all copies are identical), but the "install at most once per process" comment overstates the guarantee. Consider defining the flag and the call in a single .cc file with external linkage, or accepting the current behaviour and updating the comment.
| for device in ["cpu", "mixed"]: | ||
|
|
||
| @pipeline_def(batch_size=1, device_id=0, num_threads=1) | ||
| def geo_pipe(files): | ||
| encoded, _ = fn.readers.file(files=files) | ||
| decoded = fn.decoders.image(encoded, device=device, output_type=types.ANY_DATA) | ||
| return decoded | ||
|
|
||
| p = geo_pipe(files=[geo_path]) | ||
| out = p.run()[0] | ||
| if device == "mixed": | ||
| out = out.as_cpu() | ||
| result = np.array(out[0]).squeeze() | ||
| np.testing.assert_array_equal(result, expected) |
There was a problem hiding this comment.
"mixed" device path requires CUDA and will fail (or hang) on CPU-only CI runners. The rest of the test file uses SkipTest (imported at the top) or nose_utils guards to gate GPU-dependent paths. Without a skip guard here, this test will produce a hard failure rather than a clean skip on CPU-only machines. Consider wrapping or splitting the "mixed" iteration with a GPU-availability check, for example using test_utils.get_gpu_num() or calling SkipTest when no CUDA device is present.
- Guard against null module name in SuppressGeoTIFFTagWarnings (libtiff permits a null module argument in some code paths) - Clarify comment on once_flag scope: each TU gets its own copy because the function lives in an anonymous namespace in a header; behaviour is still correct since all copies install the same handler - Skip the mixed-backend path in test_image_decoder_geotiff on CPU-only runners (guard with get_gpu_num() > 0) - Update copyright year to 2026 Signed-off-by: shreyaskommuri <shreyaskommuri@gmail.com>
| Returns the expected pixel array (HxW uint8) so callers can compare against decoded output. | ||
| The file is constructed with struct so no third-party TIFF library is required. | ||
| """ | ||
| import struct |
There was a problem hiding this comment.
Can this be moved to the top of the file?
|
|
||
| GeoTIFF tags (ModelPixelScale, ModelTiepoint, GeoKeyDirectory, GeoDoubleParams, | ||
| GeoAsciiParams) are not natively known to libtiff, which emits "Unknown field with tag" | ||
| warnings for them. The fix installs a custom libtiff warning handler that suppresses |
There was a problem hiding this comment.
| warnings for them. The fix installs a custom libtiff warning handler that suppresses | |
| warnings for them. The fix installs a custom libtiff warning handler that suppresses |
| if get_gpu_num() > 0: | ||
| devices.append("mixed") | ||
|
|
||
| for device in devices: |
There was a problem hiding this comment.
Can you make it a test param (although it would create the test file for each param but it should not impose big overhead):
@params(["cpu", "gpu"])
def test_image_decoder_geotiff(device):
| devices = ["cpu"] | ||
| if get_gpu_num() > 0: | ||
| devices.append("mixed") |
There was a problem hiding this comment.
| devices = ["cpu"] | |
| if get_gpu_num() > 0: | |
| devices.append("mixed") | |
| if devices == "cpu" and get_gpu_num() <= 0: | |
| raise SkipTest() |
There was a problem hiding this comment.
Although I don't think test can run without GPU in general, so this check is not needed as other test will fail anyway.
|
Hi @shreyaskommuri, Thank you for your contribution! Please review my comments at your convenience. Also please run lint & black on touched files. It seems there are things to improve (see this and this). |
- Remove anonymous namespace from image_decoder.h (cpplint build/namespaces) - Mark SuppressGeoTIFFTagWarnings and InstallGeoTIFFWarningFilter inline, which also makes the once_flag truly process-wide across TUs - Move import struct to top of test_image.py - Apply black formatting to _create_geotiff - Fix double-space in docstring - Refactor test_image_decoder_geotiff to use @params Signed-off-by: shreyaskommuri <shreyaskommuri@gmail.com>
|
Thanks for the review @JanuszL! I've addressed all the feedback in the latest commit:
Skipped the GPU skip-test guard per your note that it's not needed. |
|
Fix flake8 F401: removed the unused |
The import was left over after refactoring the geotiff test to use @params instead of a manual GPU check. Signed-off-by: shreyaskommuri <shreyaskommuri@gmail.com>
shreyaskommuri
force-pushed
the
fix/geotiff-libtiff-warnings
branch
from
58e1ea3 to
d619ac6
Compare
|
Hi @shreyaskommuri — thank you so much for digging into this and putting together such a clean fix! The synthetic GeoTIFF helper and the targeted tag allowlist are great, and we really appreciate the careful write-up. Before we merge, though, we'd like to ask you to redirect this contribution: the right home for the filter is nvImageCodec, not DALI. The "Unknown field with tag X" warnings originate from libtiff, and libtiff is invoked from inside the nvImageCodec libtiff extension (extensions/libtiff/libtiff_decoder.cpp) — DALI doesn't call libtiff directly. Installing TIFFSetWarningHandler in DALI's ImageDecoder works, but it's a layering workaround: it only helps DALI users, leaves every other nvImageCodec consumer with the same noise, and routes a global libtiff hook through the wrong layer. Putting it in the nvImageCodec libtiff extension fixes the warning at its source for everyone, and DALI picks it up automatically through its nvImageCodec dependency. Would you be willing to open the PR against nvImageCodec instead? 👉 https://github.com/NVIDIA/nvImageCodec The natural spot is extensions/libtiff/ — most likely the warning handler in error_handling.h and the one-time std::call_once install from libtiff_ext.cpp (LibtiffImgCodecsExtension::libtiffExtensionCreate). Your test approach (struct-built GeoTIFF + pixel comparison) translates directly to test/python/ against nvimgcodec.Decoder. If you submit it there, it would be considered for the next nvImageCodec release, after which DALI would inherit the fix on its next bump. Thanks again for the contribution — looking forward to seeing this land in the right place! |
|
Hi @jantonguirao — I've opened the fix in nvImageCodec as requested: NVIDIA/nvImageCodec#52 The implementation matches your suggestion exactly:
All review feedback from this PR (null-module guard, |
3 participants
Summary
Suppress the `TIFFReadDirectory: Warning, Unknown field with tag X encountered` warnings that libtiff emits when decoding GeoTIFF files. GeoTIFF embeds geographic metadata using five well-known tags that libtiff does not recognise natively — the image data is decoded correctly, but the warnings pollute stderr on every decode.
Related Issue
Closes #6114
Changes
Type of Change
Verification
The tags suppressed are:
AI Disclosure
Signed-off-by: shreyaskommuri shreyaskommuri@gmail.com