Adding support to reading tiled and YcbCr jpeg tiffs through libtiff #3227

kkopachev · 2018-07-03T05:58:05Z

Fixes #2926 and #3044 probably #3195 and #3206.

Changes proposed in this pull request:

Read tiled images through LibTIFF
Read YCbCr JPEG-encoded TIFFs as RGB
for non-LibTiff case limit tile dimensions to not exceed image dimensions

~~Opening this mostly to gather feedback. Code is working, but missing tests. Would like to get some thoughts before I went too far with this.~~

ignoring whitespace changes might help with code review: https://github.com/python-pillow/Pillow/pull/3227/files?w=1

5.0 release notes had statement that all compressed tiffs require libtiff to be installed. That was introduced in #2899

kkopachev · 2018-07-03T06:02:45Z

src/PIL/TiffImagePlugin.py

-    (MM, 6, (1,), 1, (8, 8, 8, 8, 8), (0, 0)): ("YCbCr", "YCbCrXXX"),
-    (II, 6, (1,), 1, (8, 8, 8, 8, 8, 8), (0, 0, 0)): ("YCbCr", "YCbCrXXX"),
-    (MM, 6, (1,), 1, (8, 8, 8, 8, 8, 8), (0, 0, 0)): ("YCbCr", "YCbCrXXX"),
+    (II, 6, (1,), 1, (8, 8, 8), ()): ("RGB", "RGB"),


LibTiff is able to read YCbCr from jpeg-encoded images, but it does not do upsampling. So it either requires somebody to write upsampling code, or let LibTiff ask libjpeg to output in RGB. I thought this was acceptable, since JpegDecode already outputs RGB by default and making so for tiffs could be acceptable too.

kkopachev · 2018-07-03T06:03:19Z

src/PIL/TiffImagePlugin.py

-    (MM, 6, (1,), 1, (8, 8, 8), ()): ("YCbCr", "YCbCr"),
-    (II, 6, (1,), 1, (8, 8, 8, 8), (0,)): ("YCbCr", "YCbCrX"),
-    (MM, 6, (1,), 1, (8, 8, 8, 8), (0,)): ("YCbCr", "YCbCrX"),
-    (II, 6, (1,), 1, (8, 8, 8, 8, 8), (0, 0)): ("YCbCr", "YCbCrXXX"),


I think there was a typo and rawmode should have being YCbCrXX (extra X removed).

This has now been taken care of with the merge of #3335

I knew that seen this somewhere! )

kkopachev · 2018-07-03T06:05:13Z

src/PIL/TiffImagePlugin.py

                self.tile.append(
                    (self._compression,
-                        (x, y, x+w, y+h),
-                        o, a))
+                     (min(x, xsize),  min(y, ysize), min(x+w, xsize), min(y+h, ysize)),


this to resolve #3044 limiting tile size.

doko42 · 2018-07-17T14:29:32Z

any update on this?

kkopachev · 2018-07-17T18:20:13Z

I think I'm done with changes here. Should be ready to review.

I've noticed, that winbuilds are all without libtiff, because it could not find zlib when compiling libtiff. I'll try to fix that as well, but in another commit.

kkopachev · 2018-07-18T17:35:16Z

Tests pass and I think this is all ready to go now. Please review.

radarhere · 2018-08-25T06:36:27Z

This PR should presumably also change https://github.com/python-pillow/Pillow/blob/c2189235afc3fdc36cb781ee6/src/PIL/TiffTags.py#L441?

kkopachev · 2018-08-27T19:35:56Z

@radarhere From my understanding, the line you referenced is only used for writing through libtiff. This PR adds only reading of tiled images and YCbCr's through libtiff.

radarhere · 2018-09-07T11:11:05Z

You're right that the line is only using for writing - I was thinking of the comment. This PR means that 'We don't have support for tiled images in libtiff' is no longer true.

'We don't have support for writing tiled images with libtiff'?

kkopachev · 2018-09-07T19:40:15Z

@radarhere fixed.

kkopachev · 2018-09-10T17:38:49Z

@homm do you happen to have YCbCr images with extra samples? I think libtiff in case of jpeg compressed tiff will always expect 3 samples and will ignore all extra samples anyway, but I am not sure if it's even possible to make tiff with YCbCrX mode.

kkopachev · 2018-09-10T17:41:36Z

I back-merged master into this PR and dropped X for all JPEG-compressed modes.
It says here at table 5 that YCbCr tiffs must have 3 samples.

homm · 2018-09-11T11:15:56Z

do you happen to have YCbCr images with extra samples?

I'm not sure. I just added XX to all modes where this doesn't break anything )

You are right, according to specification, there is could be only 3 channels for YCbCr images:

It is better to drop all extra RGB → RGB modes for PhotoInterpretation=6 (former YCbCrX, YCbCrXX and YCbCrXXX).

homm · 2018-09-11T11:17:22Z

Could you also drop them from unpack.c?

kkopachev · 2018-09-11T16:27:56Z

@homm @radarhere all done here. Please take a look.

kkopachev · 2018-09-18T20:23:16Z

@radarhere I did rebase. Let me know, if you want commits squashed.

homm · 2018-09-25T12:17:42Z

Tests/test_lib_pack.py

@@ -151,10 +151,6 @@ def test_YCbCr(self):
        self.assert_pack("YCbCr", "YCbCr", 3, (1, 2, 3), (4, 5, 6), (7, 8, 9))
        self.assert_pack("YCbCr", "YCbCr;L", 3,
                         (1, 4, 7), (2, 5, 8), (3, 6, 9))
-        self.assert_pack(


I change my mind. This is actually breaking change, because 'YCbCrX' is allowed for using in user-space:

data = im.tobytes("raw", "YCbCrX") Image.frombytes(im.mode, im.size, data, "raw", "YCbCrX", 0, 1)

I think it's better to leave YCbCrX mode and drop modes with more Xs

This applies only to pack and unpack. No support of YCbCrX for tiff is required.

added them back.

homm · 2018-09-26T12:00:58Z

src/PIL/TiffImagePlugin.py

+            if fillorder == 2:
+                key = (
+                    self.tag_v2.prefix, photo, sampleFormat, 1,
+                    self.tag_v2.get(BITSPERSAMPLE, (1,)),


Are all other values except fillorder always should be the same as in the previous key? If so, it's better to modify existing key.

Option 1:

key = key[:3] + (1,) + key[4:]

Option 2:

key = list(key) key[3] = 1 key = tuple(key)

homm · 2018-09-26T12:21:27Z

src/PIL/TiffImagePlugin.py

-                        (x, y, x+w, y+h),
-                        o, a))
+                     (x, y, min(x+w, xsize), min(y+h, ysize)),
+                        offset, a))


Indent should be the same as in the line above.

homm · 2018-09-26T12:35:43Z

src/libImaging/TiffDecode.c

-	// with shuffle.  (or, just alloc a buffer myself, then figure out how to get it
-	// back in. Can't use read encoded stripe.
+	if (TIFFIsTiled(tiff)) {
+	    uint32 x, y, tile_y;


Historical most C files have tabs and spaces mix. Please, use only four spaces as indentation in a new code (from line 239: if (TIFFIsTiled(tiff)) to line 294: } else { here).

If you want, you can create another PR with tabs to spaces conversion in TiffDecode.c or in the whole codebase.

Added release notes for #3227

Old-style JPEG compression in TIFFs are able to be read using Strip/Tile APIs. Although, it should be possible to read them using Scanline API, it does not work for some reason. Anyway, reading subsampled YCbCr formats through Strip/Tile/Scanline libtiff API does not de-subsample the data, so caller should unpack data to whatever format is appropriate. New-style JPEG compressed images were already read through libtiff as RGB images (python-pillow#3227). Unfortunately, there is no flag to ask libtiff to de-subsample old jpeg, but it provides a way to read any image as 32bit RGBA. This commit adds ability to read old-style JPEG TIFFs through reading *all* YCbCr images as RGBX using Tile and Strip reading API. This supersedes previous work (PR python-pillow#3227) to read new-style JPEG-TIFFs.

kkopachev commented Jul 3, 2018

View reviewed changes

kkopachev force-pushed the master branch 2 times, most recently from d3c8550 to d11d8ca Compare July 4, 2018 06:21

kkopachev changed the title ~~[WIP] Adding support to reading tiled and YcbCr jpegs tiffs through libtiff~~ Adding support to reading tiled and YcbCr jpegs tiffs through libtiff Jul 17, 2018

kkopachev force-pushed the master branch 2 times, most recently from 2380f0a to 66459df Compare July 17, 2018 17:45

kkopachev force-pushed the master branch from 00e9856 to e3a79c8 Compare July 17, 2018 19:40

kkopachev mentioned this pull request Jul 30, 2018

module 'PIL._imaging' has no attribute 'tiff_lzw_decoder' #3195

Closed

kkopachev mentioned this pull request Aug 24, 2018

TypeError when trying to save a pyramidal TIFF as JPEG in Pillow 5.0.0 #3053

Closed

radarhere changed the title ~~Adding support to reading tiled and YcbCr jpegs tiffs through libtiff~~ Adding support to reading tiled and YcbCr jpeg tiffs through libtiff Sep 10, 2018

radarhere added the Needs Rebase label Sep 17, 2018

kkopachev added 5 commits September 18, 2018 12:05

Adding support to reading tiled and YcbCr jpegs tiffs through libtiff

f0436a4

Fix tiled raw tiff read and add tests

1f63abd

move libtiff-related tests into libtiff test file

52637a4

Remove redundant decompression; adjust jpeg read mode to be RGBX (pad)

e6c510e

Add tests for uncompressed tiff with interleaved bands

98d88fd

kkopachev added 6 commits September 18, 2018 12:05

Fix libtiff build on windows

e102f48

move variable declaration

c2ac460

Update test files to be closer match of originals

b0e5e5f

Change comment wording to reflect ability to read tiled tiffs [ci skip]

1e4b317

Fix typo and add explanatory comment

f874e6f

Drop YCbCrX(XX) modes, as TIFF does not have to support that

1877824

kkopachev force-pushed the master branch from a5cfd97 to 1877824 Compare September 18, 2018 19:07

radarhere removed the Needs Rebase label Sep 18, 2018

homm reviewed Sep 25, 2018

View reviewed changes

homm added the TIFF label Sep 25, 2018

Bring back YCbCrX in pack and unpack

300bdaa

homm requested changes Sep 26, 2018

View reviewed changes

PR feedback: simplified key construct for fillorder=2; indentation fixes

69f315a

homm approved these changes Sep 27, 2018

View reviewed changes

hugovk merged commit d36365f into python-pillow:master Sep 29, 2018

This was referenced Sep 29, 2018

ValueError: tile cannot extend outside image #3044

Closed

OSError: decoder tiff_adobe_deflate not available #3206

Closed

Release Pillow 5.3.0 on October 1, 2018 #3236

Closed

radarhere added a commit to radarhere/Pillow that referenced this pull request Sep 30, 2018

Added release notes for python-pillow#3227 [ci skip]

da2c125

hugovk added a commit that referenced this pull request Sep 30, 2018

Merge pull request #3389 from radarhere/release_notes

748ec87

Added release notes for #3227

radarhere mentioned this pull request Sep 30, 2018

Tiff not being fully read(?) #2754

Closed

kkopachev mentioned this pull request Dec 9, 2018

Add reading old-JPEG compressed TIFFs #3489

Merged

radarhere mentioned this pull request Jun 15, 2019

Cannot get Pillow TIFF support using Python Docker image #3905

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding support to reading tiled and YcbCr jpeg tiffs through libtiff #3227

Adding support to reading tiled and YcbCr jpeg tiffs through libtiff #3227

kkopachev commented Jul 3, 2018 •

edited

Loading

kkopachev Jul 3, 2018 •

edited

Loading

kkopachev Jul 3, 2018

radarhere Sep 10, 2018

homm Sep 10, 2018

kkopachev Jul 3, 2018

doko42 commented Jul 17, 2018

kkopachev commented Jul 17, 2018

kkopachev commented Jul 18, 2018

radarhere commented Aug 25, 2018

kkopachev commented Aug 27, 2018

radarhere commented Sep 7, 2018

kkopachev commented Sep 7, 2018

kkopachev commented Sep 10, 2018

kkopachev commented Sep 10, 2018 •

edited

Loading

homm commented Sep 11, 2018

homm commented Sep 11, 2018

kkopachev commented Sep 11, 2018

kkopachev commented Sep 18, 2018

homm Sep 25, 2018

homm Sep 25, 2018

kkopachev Sep 25, 2018

homm Sep 26, 2018

homm Sep 26, 2018

homm Sep 26, 2018

homm Sep 26, 2018

Adding support to reading tiled and YcbCr jpeg tiffs through libtiff #3227

Adding support to reading tiled and YcbCr jpeg tiffs through libtiff #3227

Conversation

kkopachev commented Jul 3, 2018 • edited Loading

kkopachev Jul 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

doko42 commented Jul 17, 2018

kkopachev commented Jul 17, 2018

kkopachev commented Jul 18, 2018

radarhere commented Aug 25, 2018

kkopachev commented Aug 27, 2018

radarhere commented Sep 7, 2018

kkopachev commented Sep 7, 2018

kkopachev commented Sep 10, 2018

kkopachev commented Sep 10, 2018 • edited Loading

homm commented Sep 11, 2018

homm commented Sep 11, 2018

kkopachev commented Sep 11, 2018

kkopachev commented Sep 18, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kkopachev commented Jul 3, 2018 •

edited

Loading

kkopachev Jul 3, 2018 •

edited

Loading

kkopachev commented Sep 10, 2018 •

edited

Loading