Support tiled TIFFs #4201
Merged
Support tiled TIFFs #4201
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
szkarpinski
force-pushed
the
tiff-tiled
branch
from
8a95ff9
to
9cbc30e
Compare
klecki
approved these changes
Sep 7, 2022
Comment on lines
+360
to
+364
| DALI_ENFORCE(TIFFReadTile(tiff.get(), buf.get(), tile_x, tile_y, 0, 0) > 0, | ||
| "TIFFReadTile failed"); | ||
| } else { | ||
| LIBTIFF_CALL(TIFFReadScanline(tiff.get(), buf.get(), tile_y, 0)); |
There was a problem hiding this comment.
This looks weird, that we use both LIBTIFF_CALL and DALI_ENFORCE for TIFFRead***
There was a problem hiding this comment.
That is weird, but that's libtiff. Almost all of its functions, including TIFFReadScanline, return 1 on success and that's what LIBTIFF_CALL checks for. But TIFFReadTile returns the number of bytes read, because why not
klecki
approved these changes
Sep 7, 2022
Signed-off-by: Szymon Karpiński <skarpinski@nvidia.com>
Signed-off-by: Szymon Karpiński <skarpinski@nvidia.com>
Signed-off-by: Szymon Karpiński <skarpinski@nvidia.com>
Signed-off-by: Szymon Karpiński <skarpinski@nvidia.com>
Signed-off-by: Szymon Karpiński <skarpinski@nvidia.com>
Signed-off-by: Szymon Karpiński <skarpinski@nvidia.com>
Signed-off-by: Szymon Karpiński <skarpinski@nvidia.com>
Signed-off-by: Szymon Karpiński <skarpinski@nvidia.com>
Signed-off-by: Szymon Karpiński <skarpinski@nvidia.com>
szkarpinski
force-pushed
the
tiff-tiled
branch
from
f16855d
to
da680e4
Compare
| // TODO(skarpinski) Support tiled images | ||
| if (info.is_tiled) | ||
| return {false, make_exception_ptr(std::logic_error("Tiled images are not yet supported"))}; | ||
| if (info.is_tiled && info.tile_width % 16 != 0 && info.tile_height % 16 != 0) { |
There was a problem hiding this comment.
Suggested change
| if (info.is_tiled && info.tile_width % 16 != 0 && info.tile_height % 16 != 0) { | |
| if (info.is_tiled && (info.tile_width % 16 != 0 || info.tile_height % 16 != 0)) { |
Shouldn't be both a multiple of 16?
| @@ -234,7 +234,7 @@ class DecoderTestBase : public ::testing::Test { | |||
| Tensor<CPUBackend> Crop(const Tensor<CPUBackend> &input, const ROI &roi) { | |||
| int ndim = input.shape().sample_dim(); | |||
| VALUE_SWITCH(ndim, Dims, (2, 3, 4), ( | |||
| TYPE_SWITCH(input.type(), type2id, InputType, (IMGCODEC_TYPES), ( | |||
| TYPE_SWITCH(input.type(), type2id, InputType, (IMGCODEC_TYPES, double), ( | |||
There was a problem hiding this comment.
what's the purpose of adding double here?
There was a problem hiding this comment.
I guess numpy reference data.
jantonguirao
approved these changes
Sep 20, 2022
|
!build |
|
CI MESSAGE: [5957668]: BUILD STARTED |
|
CI MESSAGE: [5957668]: BUILD PASSED |
staniewzki
pushed a commit
to staniewzki/DALI
that referenced
this pull request
Sep 21, 2022
Until now, only striped TIFFs were supported. In this PR I add support for tiled TIFFs. Additional information: The main disadvantage of striped TIFFs is that efficient ROI decoding is effectively impossible - you always need to read a whole strip. This is the reason why Tiled TIFFs are common in applications where the data is bigger, for example in spatial information or microscopy. That's why we want to support them. Before, this decoder was reading TIFF line-by-line. Now I extended it to support tiles and, in order to avoid code duplication, I simply pretend that lines are 1 x imagewidth tiles. We do not support exotic bitdepths (i.e. other than 8, 16 and 32), because I was unable to find software capable of correctly writing or reading those, so it was impossible to test reliably. Signed-off-by: Szymon Karpiński <skarpinski@nvidia.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Category:
New feature (non-breaking change which adds functionality)
Description:
Until now, only striped TIFFs were supported. In this PR I add support for tiled TIFFs.
Additional information:
The main disadvantage of striped TIFFs is that efficient ROI decoding is effectively impossible - you always need to read a whole strip. This is the reason why Tiled TIFFs are common in applications where the data is bigger, for example in spatial information or microscopy. That's why we want to support them.
Before, this decoder was reading TIFF line-by-line. Now I extended it to support tiles and, in order to avoid code duplication, I simply pretend that lines are
1 x imagewidthtiles.I do not support exotic bitdepths (i.e. other than 8, 16 and 32), because I was unable to find software capable of correctly writing or reading those, so it was impossible to test reliably.
Affected modules and functionalities:
I've temporarily fixed the testing framework, but I'll rebase on the proper fix (#4217) once it's merged.
Key points relevant for the review:
Tests:
The tests require images from NVIDIA/DALI_extra#101
Checklist
Documentation
DALI team only
Requirements
REQ IDs: N/A
JIRA TASK: DALI-2924