Skip to content

fix(tiff): care with missing rowsperstrip#5160

Merged
lgritz merged 2 commits intoAcademySoftwareFoundation:mainfrom
lgritz:lg-rowsperstrip
Apr 23, 2026
Merged

fix(tiff): care with missing rowsperstrip#5160
lgritz merged 2 commits intoAcademySoftwareFoundation:mainfrom
lgritz:lg-rowsperstrip

Conversation

@lgritz
Copy link
Copy Markdown
Collaborator

@lgritz lgritz commented Apr 23, 2026

According to the TIFF spec, TIFF files with a missing rowsperstrip should assume the whole image is one strip.

Somewhat related (found in debugging): add a DASSERT to help debug if fmath.h's round_to_multiple() is given an invalid denominator.

According to the TIFF spec, TIFF files with a missing rowsperstrip
should assume the whole image is one strip.

Related (found in debugging): add a DASSERT to help debug if
fmath.h's round_to_multiple() is given an invalid denominator.

Signed-off-by: Larry Gritz <lg@larrygritz.com>
if (!m_spec.tile_width) {
TIFFGetField(m_tif, TIFFTAG_ROWSPERSTRIP, &m_rowsperstrip);
if (m_rowsperstrip > 0)
if (m_rowsperstrip > 0) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should also cap the m_rowsperstrip to the height of the image in all cases since downstream computations and memory allocations use the value too.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good suggestion. Done.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, is there a reason not to clamp the value before setting the tiff:RowsPerStrip attribute value too?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, two reasons:
(1) The attribute should reflect what's actually in the file (for the sake of iinfo -v and the like), and there's nothing invalid about having it larger than the number of scanlines (just like happens for the last strip any time the strip size does not evenly divide into the height);
(2) I want it to make its way to the output image and be set there as well, so that something like iconvert in.tif out.tif will be as close as possible to cp in.tif out.tif and not change TIFF tag values that are perfectly legal.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha

Signed-off-by: Larry Gritz <lg@larrygritz.com>
@lgritz lgritz merged commit d0429b5 into AcademySoftwareFoundation:main Apr 23, 2026
31 checks passed
@lgritz lgritz deleted the lg-rowsperstrip branch April 23, 2026 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants