Use RGBX rawmode for RGB JPEG images

[homm]

libjpeg-turbo supports different output formats.
We are choosing Pillow's native format (3 color bytes + 1 padding) to avoid extra conversion in Unpack.c.

This speeds up JPEG loading by 19% and saving by 14%.

	JCS_RGB (master)	JCS_EXT_RGBX
libjpeg-turbo 1.3.0
Load	133.80 Mpx/s	160.04 Mpx/s
Save	145.08 Mpx/s	166.38 Mpx/s
libjpeg-turbo 1.5.80 (dev branch)
Load	141.53 Mpx/s	168.21 Mpx/s
Save	172.14 Mpx/s	200.08 Mpx/s

[homm]

Don't understand why tiff loading fails in Travis environment. Investigating.

[wiredfool]

Well, it's a jpeg encoded tiff, so at least there's some basis for why it's failing.

This is im.tile from that image:
[('jpeg', (0, 0, 128, 32), 8, ('RGB', '')), ('jpeg', (0, 32, 128, 64), 4445, ('RGB', '')), ('jpeg', (0, 64, 128, 96), 8722, ('RGB', '')), ('jpeg', (0, 96, 128, 128), 12726, ('RGB', ''))]

[aclark4life]

@homm 3.5.0 then?

[homm]

Sure

[homm]

As always, weird bug. I don't have it on Ubuntu 14.04, so I installed Ubuntu 12.04.

stock libjpeg-turbo 1.1.90, no modifications: no problems
recent libjpeg-turbo 1.5.1, no modifications: broken data stream
stock libjpeg-turbo 1.1.90, JCS_EXT_RGB for RGB: broken data stream
recent libjpeg-turbo 1.5.1, JCS_EXT_RGB for RGB: broken data stream
stock libjpeg-turbo 1.1.90, JCS_EXT_RGBX for RGBX: broken data stream
recent libjpeg-turbo 1.5.1, JCS_EXT_RGBX for RGBX: broken data stream

It is absolutely unclear how JCS_RGB differ from JCS_EXT_RGB and why libjpeg-turbo 1.5.1 also leads to the error.

[homm]

After night debugging I can say what is going on.

First, libjpeg-turbo 1.5.1 fails due to compilation issue, not relevant to this case.

The real problem was that the TIFF file is in RGB colorspace, while all other JPEGs for tests are YCbCr (or maybe CMYK). On Ubuntu 12.04. LTS the default libjpeg-turbo version is 1.1.90. And it doesn't support RGB → RGB transformations, including JCS_EXT_RGB which should be equivalent for JCS_RGB. See source code

Starting from 1.2.1 libjpeg-turbo supports all JCS_EXT_* transformations for YCbCr, Gray and RGB colorspaces. See changelog and source code

So before implementing this small feature, we need to do the following:

• Propagate errors from libjpeg (any version) to python exception. It is a key to understanding issues.
• Add tests for all JPEG input colorspaces. It is luck that we fail with TIFF.
• Add one build on Travis with recent libjpeg-turbo version.

As for solution: in theory, we can do not switch to RGBX in the case when the source file is in RGB colorspace and libjpeg-turbo version < 1.2.1. But we need to make a decision in PyImaging_JpegDecoderNew, where we don't have information about colorspace (this happens only in ImagingJpegDecode). So in my opinion it is better to use RGBX only with libjpeg-turbo >= 1.2.1.

[homm]

Unfortunately, this not so easy to detect current libjpeg-turbo version: libjpeg-turbo/libjpeg-turbo#166

[homm]

Resolved the libjpeg-turbo version issue.
Implementing the same approach for saving.
Updated benchmarks.

[homm]

This simple check is enough because anything else is checked when rawmode is set to RGBX.

[homm]

LIBJPEG_TURBO_VERSION_NUMBER is defined starting from 1.5.0 (1005000). Compare it one more time to be sure.

[homm]

this literally means:

use_jcs_extensions = (libjpeg_turbo_version >= "1.2.1")

[homm]

@wiredfool Could you review this. I've added explanations for all significant lines.

[wiredfool]

How are you feeling about the three bullet points above? (#1989 (comment))

• Fedora 26 has version 1.5.1. (this provides libjpeg, which is what we're requesting in our Dockerfile)

I'm not sure about the status of the other bullets.

(edit: do you want to rebase to pull in the .travis.yml that runs against Fedora?)

[homm]

How are you feeling about the three bullet points above?

Not for this release, unfortunately.

The most important thing is "Add tests for all JPEG input colorspaces" but as I can see, we had pil_sample_cmyk.jpg in tests at least from 24 may 2016 and I don't know why we hadn't hit the issue with it.

[homm]

Both Fedoras passed

[homm]

@wiredfool I meant that I'm not ready to implement bullets, «Propagate errors from libjpeg», for example. As of this PR, I think it is ready and relatively safe for all systems.

[wiredfool]

Ok, fair enough.