Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Ensure TIFF RowsPerStrip is multiple of 8 for JPEG compression#5588

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
radarhere merged 2 commits intopython-pillow:masterfromkmilos:patch-2
Jul 18, 2021

Conversation

@kmilos
Copy link
Contributor

Fixes#5586

@kmilos
Copy link
ContributorAuthor

kmilos commentedJul 7, 2021
edited
Loading

This works only for RGB mode 4:4:4 data (no subsampling). AFAICT, we don't support YCbCr subsampling directly when saving to TIFF, but the tag might be copied over from another info structure, so might be an idea to handle it?

However, there's something weird going on wrt to saving YCbCr mode instead of RGB, even if using single strip (i.e. 8.2.0):

OSError: encoder error -9 when writing image filejunk.tiff: Warning, fractional scanline discarded.JPEGLib: Application transferred too few scanlines.

@radarhereradarhere mentioned this pull requestJul 7, 2021
@kmilos
Copy link
ContributorAuthor

kmilos commentedJul 7, 2021
edited
Loading

Thanks, that leaves YCbCr mode TIFF saving which it seems never worked w/ JPEG compression?

I also tried settingifd[530] = (1,1) (YCbCr subsampling to 4:4:4) as the default is (2,2) which doesn't correspond to the data, but the encoder doesn't like that - I think unfortunatelyTIFFSetField interface is different for this tag (2 ints) compared to the expected count+pointer (e.g. ExtraSamples)? Settingatts[530] = 1 didn't work either.

@kmilos
Copy link
ContributorAuthor

kmilos commentedJul 7, 2021
edited
Loading

YCbCr saving doesn't even work uncompressed (i.e. no libtiff involved), even if I add correct tags 530, 531, and 532. I guess this is a separate issue.

Uncompressed YCbCr 4:4:4 seems to work after all (if correct tags 530 and 532 are added). Just that most viewers don't seem to support it...

However, there is a clearbug w/ libtiff_encoder, as it tries to save tag 530 2-tuple with a "default" type LONG (i.e. count+offset), which doesn't fit into the expected 4 bytes value/offset sub-field as 2 SHORTs would.

rows_per_strip=min((2**16+stride-1)//stride,im.size[1])
# JPEG encoder expects multiple of 8 rows
ifcompression=="jpeg":
rows_per_strip=min(((rows_per_strip+7)//8)*8,im.size[1])
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
rows_per_strip=min(((rows_per_strip+7)//8)*8,im.size[1])
rows_per_strip=((rows_per_strip+7)//8)*8

Becauserows_per_strip has already been set to beim.size[1] at minimum, and the calculation here only ever rounds up, themin part is unnecessary.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

I believe it is needed, precisely because it rounds up. This is to support the rare case of small images where im.size[1]<8 and you end up as a single strip. Libtiff/jpeg codec have no problem handling the single/last strip that is not factor of 8, as it is padded internally.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, thanks

Copy link
Member

@radarhereradarhereJul 10, 2021
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
rows_per_strip=min(((rows_per_strip+7)//8)*8,im.size[1])
rows_per_strip=min(((rows_per_strip+7)//8)*8,rows_per_strip)

What about this?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

That won't work for the corner case where you have u multi-strip w/ e.g. rows_per_strip=2 and im.size[1]=6. For JPEG, you want to end up as single strip of 6 rows.

@radarhereradarhere merged commit7484bb0 intopython-pillow:masterJul 18, 2021
@kmiloskmilos deleted the patch-2 branchJuly 18, 2021 09:36
@radarhereradarhere mentioned this pull requestAug 27, 2021
9 tasks
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@radarhereradarhereradarhere left review comments

Assignees

No one assigned

Labels

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Recent change in TIFF rows per strip break saving with JPEG compression

2 participants

@kmilos@radarhere

[8]ページ先頭

©2009-2025 Movatter.jp