Read color info from JPX stream + fix color problem #4540 #4565

fkaelberer · 2014-04-05T16:54:09Z

These are two commits that I accicently squashed into one.

The first one gets color information directly from the JPX stream, to define the color space in images.js.
This fixes #1303, #4407 (and all documents that are created with Acrobat 8 from JPX images).
I'm not 100% if this is how it is supposed to be done in image.js, so please have a look at that.

The other commit corrects the coefficient magnitudes in jpx.js and so fixes #4540.

Everything below Line 1000 in jpx.js is just minor cleanup.

I opened another PR concurrently to #4538, because I wanted to fix errors first, and make the performance oriented changes of #4538 optional.

Fix colors problem mozilla#4540 + minor cleanup fix lint warnings

Snuffleupagus · 2014-04-05T17:50:48Z

/botio test

pdfjsbot · 2014-04-05T17:50:49Z

From: Bot.io (Windows)

Received

Command cmd_test from @Snuffleupagus received. Current queue size: 0

Live output at: http://107.22.172.223:8877/adf3588fdb43dfb/output.txt

pdfjsbot · 2014-04-05T17:50:49Z

From: Bot.io (Linux)

Received

Command cmd_test from @Snuffleupagus received. Current queue size: 0

Live output at: http://107.21.233.14:8877/0a0169e6aff2311/output.txt

pdfjsbot · 2014-04-05T18:05:16Z

From: Bot.io (Windows)

Failed

Full output at http://107.22.172.223:8877/adf3588fdb43dfb/output.txt

Total script time: 14.43 mins

Font tests: Passed
Unit tests: Passed
Regression tests: FAILED

Image differences available at: http://107.22.172.223:8877/adf3588fdb43dfb/reftest-analyzer.html#web=eq.log

pdfjsbot · 2014-04-05T18:06:03Z

From: Bot.io (Linux)

Failed

Full output at http://107.21.233.14:8877/0a0169e6aff2311/output.txt

Total script time: 15.23 mins

Font tests: Passed
Unit tests: Passed
Regression tests: FAILED

Image differences available at: http://107.21.233.14:8877/0a0169e6aff2311/reftest-analyzer.html#web=eq.log

timvandermeij · 2014-04-05T20:04:26Z

Let's try this again.

/botio test

pdfjsbot · 2014-04-05T20:04:27Z

From: Bot.io (Linux)

Received

Command cmd_test from @timvandermeij received. Current queue size: 0

Live output at: http://107.21.233.14:8877/a71f4ee1e84c6c0/output.txt

pdfjsbot · 2014-04-05T20:04:27Z

From: Bot.io (Windows)

Received

Command cmd_test from @timvandermeij received. Current queue size: 0

Live output at: http://107.22.172.223:8877/e08495009d4823c/output.txt

pdfjsbot · 2014-04-05T20:19:01Z

From: Bot.io (Windows)

Failed

Full output at http://107.22.172.223:8877/e08495009d4823c/output.txt

Total script time: 14.56 mins

Font tests: Passed
Unit tests: Passed
Regression tests: FAILED

Image differences available at: http://107.22.172.223:8877/e08495009d4823c/reftest-analyzer.html#web=eq.log

pdfjsbot · 2014-04-05T20:19:42Z

From: Bot.io (Linux)

Failed

Full output at http://107.21.233.14:8877/a71f4ee1e84c6c0/output.txt

Total script time: 15.24 mins

Font tests: Passed
Unit tests: Passed
Regression tests: FAILED

Image differences available at: http://107.21.233.14:8877/a71f4ee1e84c6c0/reftest-analyzer.html#web=eq.log

timvandermeij · 2014-04-06T08:55:18Z

/cc @yurydelendik: Could you look into closing the browsers again?

Snuffleupagus · 2014-04-06T09:00:05Z

This timeouts locally as well, so the failures seem to be caused by the patch.
Comparing, for example, lines: https://github.com/mozilla/pdf.js/pull/4565/files#diff-96fc2112f33495889188d418b229afcbL1632 and https://github.com/mozilla/pdf.js/pull/4565/files#diff-96fc2112f33495889188d418b229afcbR1665, I wonder if this could be caused by the removal of k = i * llWidth (and similarly below).

Snuffleupagus · 2014-04-06T09:09:20Z

src/core/image.js

-      // var bits = ...
-      // var colorspace = ...
+    var dict = image.dict;
+    if (dict.get('Filter').name === 'JPXDecode') {


Is the Filter entry guaranteed to be present in the dictionary? If not, I suggest adding if (dict.has('Filter')) { above to prevent issues.

fkaelberer · 2014-04-06T09:32:13Z

Sorry, I should have tested this.
The test fails at issue3371.pdf, which is encrypted. In this case,

var data = image.stream.bytes;
jpxImage.parseImageProperties(data, 0, data.length);

will fail, because data will contain the encrypted bytes. I'll try pass the stream instead to fix that.

fkaelberer · 2014-04-06T10:30:11Z

I wonder if this could be caused by the removal of k = i * llWidth (and similarly below).

No, that was not the problem. Once all code that does not affect k is removed, it is more obvious to see that the assignment k = i * lhWidth is redundant.

k=0;
for (i = 0; i < lhHeight; i++) {
  k = i * lhWidth;
  for (j = 0; j < lhWidth; j++) {
    k++;
  }
}

fkaelberer · 2014-04-06T11:44:03Z

All tests but S2.pdf should pass now.

Snuffleupagus · 2014-04-06T11:51:55Z

/botio test

pdfjsbot · 2014-04-06T11:51:55Z

From: Bot.io (Linux)

Received

Command cmd_test from @Snuffleupagus received. Current queue size: 0

Live output at: http://107.21.233.14:8877/a0ed46ac055f9ca/output.txt

pdfjsbot · 2014-04-06T12:17:33Z

From: Bot.io (Linux)

Failed

Full output at http://107.21.233.14:8877/a0ed46ac055f9ca/output.txt

Total script time: 25.63 mins

Font tests: Passed
Unit tests: Passed
Regression tests: FAILED

Image differences available at: http://107.21.233.14:8877/a0ed46ac055f9ca/reftest-analyzer.html#web=eq.log

yurydelendik · 2014-04-06T13:49:38Z

(@Snuffleupagus don't try to re-run windows botio when Font tests: FAILED is observed -- node.js is not able to close an application on windows, which holds directory)

/botio-windows test

Snuffleupagus · 2014-04-06T14:12:40Z

(@yurydelendik Sorry, I didn't pay enough attention to which tests that actually failed!)

yurydelendik · 2014-04-07T13:02:27Z

/botio-windows test

pdfjsbot · 2014-04-07T13:02:28Z

From: Bot.io (Windows)

Received

Command cmd_test from @yurydelendik received. Current queue size: 0

Live output at: http://107.22.172.223:8877/d9cba8fc0c5f65d/output.txt

pdfjsbot · 2014-04-07T13:24:11Z

From: Bot.io (Windows)

Failed

Full output at http://107.22.172.223:8877/d9cba8fc0c5f65d/output.txt

Total script time: 21.69 mins

Font tests: Passed
Unit tests: Passed
Regression tests: FAILED

Image differences available at: http://107.22.172.223:8877/d9cba8fc0c5f65d/reftest-analyzer.html#web=eq.log

yurydelendik · 2014-04-07T14:03:07Z

/botio makeref

pdfjsbot · 2014-04-07T14:03:07Z

From: Bot.io (Windows)

Received

Command cmd_makeref from @yurydelendik received. Current queue size: 0

Live output at: http://107.22.172.223:8877/37e70fcb5e373b8/output.txt

pdfjsbot · 2014-04-07T14:03:07Z

From: Bot.io (Linux)

Received

Command cmd_makeref from @yurydelendik received. Current queue size: 0

Live output at: http://107.21.233.14:8877/b6fd76fe93372b7/output.txt

pdfjsbot · 2014-04-07T14:24:48Z

From: Bot.io (Windows)

Failed

Full output at http://107.22.172.223:8877/37e70fcb5e373b8/output.txt

Total script time: 21.66 mins

Lint: Ignored
Make references: FAILED

pdfjsbot · 2014-04-07T14:28:44Z

From: Bot.io (Linux)

Success

Full output at http://107.21.233.14:8877/b6fd76fe93372b7/output.txt

Total script time: 25.61 mins

Lint: Passed
Make references: Passed
Check references: Passed

yurydelendik · 2014-04-07T14:32:01Z

Thank you for the patch

Read color info from JPX stream + fix color problem #4540

yurydelendik · 2014-04-07T14:33:46Z

/botio-windows makeref

pdfjsbot · 2014-04-07T14:33:47Z

From: Bot.io (Windows)

Received

Command cmd_makeref from @yurydelendik received. Current queue size: 0

Live output at: http://107.22.172.223:8877/b64a05a3e20e07f/output.txt

pdfjsbot · 2014-04-07T14:56:29Z

From: Bot.io (Windows)

Success

Full output at http://107.22.172.223:8877/b64a05a3e20e07f/output.txt

Total script time: 22.67 mins

Lint: Ignored
Make references: Passed
Check references: Passed

Read color info from JPX stream

1ccc8a6

Fix colors problem mozilla#4540 + minor cleanup fix lint warnings

fkaelberer mentioned this pull request Apr 5, 2014

Less copying in the JPX decoder #4538

Merged

Snuffleupagus reviewed Apr 6, 2014
View reviewed changes

Use Stream instead of byte array access

2982de8

yurydelendik added a commit that referenced this pull request Apr 7, 2014

Merge pull request #4565 from fkaelberer/fixJPXparsing

31c260a

Read color info from JPX stream + fix color problem #4540

yurydelendik merged commit 31c260a into mozilla:master Apr 7, 2014

Snuffleupagus mentioned this pull request Apr 7, 2014

Scanned pages PDF triggers a lot of warnings #4407

Closed

fkaelberer deleted the fixJPXparsing branch April 7, 2014 16:50

Snuffleupagus mentioned this pull request Apr 7, 2014

Jpx optimization #4568

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Read color info from JPX stream + fix color problem #4540 #4565

Read color info from JPX stream + fix color problem #4540 #4565

fkaelberer commented Apr 5, 2014

Snuffleupagus commented Apr 5, 2014

pdfjsbot commented Apr 5, 2014

pdfjsbot commented Apr 5, 2014

pdfjsbot commented Apr 5, 2014

pdfjsbot commented Apr 5, 2014

timvandermeij commented Apr 5, 2014

pdfjsbot commented Apr 5, 2014

pdfjsbot commented Apr 5, 2014

pdfjsbot commented Apr 5, 2014

pdfjsbot commented Apr 5, 2014

timvandermeij commented Apr 6, 2014

Snuffleupagus commented Apr 6, 2014

Snuffleupagus Apr 6, 2014

fkaelberer commented Apr 6, 2014

fkaelberer commented Apr 6, 2014

fkaelberer commented Apr 6, 2014

Snuffleupagus commented Apr 6, 2014

pdfjsbot commented Apr 6, 2014

pdfjsbot commented Apr 6, 2014

yurydelendik commented Apr 6, 2014

Snuffleupagus commented Apr 6, 2014

yurydelendik commented Apr 7, 2014

pdfjsbot commented Apr 7, 2014

pdfjsbot commented Apr 7, 2014

yurydelendik commented Apr 7, 2014

pdfjsbot commented Apr 7, 2014

pdfjsbot commented Apr 7, 2014

pdfjsbot commented Apr 7, 2014

pdfjsbot commented Apr 7, 2014

yurydelendik commented Apr 7, 2014

yurydelendik commented Apr 7, 2014

pdfjsbot commented Apr 7, 2014

pdfjsbot commented Apr 7, 2014

Read color info from JPX stream + fix color problem #4540 #4565

Read color info from JPX stream + fix color problem #4540 #4565

Conversation

fkaelberer commented Apr 5, 2014

Snuffleupagus commented Apr 5, 2014

pdfjsbot commented Apr 5, 2014

From: Bot.io (Windows)

Received

pdfjsbot commented Apr 5, 2014

From: Bot.io (Linux)

Received

pdfjsbot commented Apr 5, 2014

From: Bot.io (Windows)

Failed

pdfjsbot commented Apr 5, 2014

From: Bot.io (Linux)

Failed

timvandermeij commented Apr 5, 2014

pdfjsbot commented Apr 5, 2014

From: Bot.io (Linux)

Received

pdfjsbot commented Apr 5, 2014

From: Bot.io (Windows)

Received

pdfjsbot commented Apr 5, 2014

From: Bot.io (Windows)

Failed

pdfjsbot commented Apr 5, 2014

From: Bot.io (Linux)

Failed

timvandermeij commented Apr 6, 2014

Snuffleupagus commented Apr 6, 2014

Snuffleupagus Apr 6, 2014

Choose a reason for hiding this comment

fkaelberer commented Apr 6, 2014

fkaelberer commented Apr 6, 2014

fkaelberer commented Apr 6, 2014

Snuffleupagus commented Apr 6, 2014

pdfjsbot commented Apr 6, 2014

From: Bot.io (Linux)

Received

pdfjsbot commented Apr 6, 2014

From: Bot.io (Linux)

Failed

yurydelendik commented Apr 6, 2014

Snuffleupagus commented Apr 6, 2014

yurydelendik commented Apr 7, 2014

pdfjsbot commented Apr 7, 2014

From: Bot.io (Windows)

Received

pdfjsbot commented Apr 7, 2014

From: Bot.io (Windows)

Failed

yurydelendik commented Apr 7, 2014

pdfjsbot commented Apr 7, 2014

From: Bot.io (Windows)

Received

pdfjsbot commented Apr 7, 2014

From: Bot.io (Linux)

Received

pdfjsbot commented Apr 7, 2014

From: Bot.io (Windows)

Failed

pdfjsbot commented Apr 7, 2014

From: Bot.io (Linux)

Success

yurydelendik commented Apr 7, 2014

yurydelendik commented Apr 7, 2014

pdfjsbot commented Apr 7, 2014

From: Bot.io (Windows)

Received

pdfjsbot commented Apr 7, 2014

From: Bot.io (Windows)

Success