Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix JPX "Out of Packets" Error (issues 4358, 4659, 4814) #5426

Merged
merged 2 commits into from Oct 22, 2014

Conversation

fkaelberer
Copy link
Contributor

This PR fixes the handling of JPX packets which caused blurriness in JPEG2000 images (#4814, #4659 page 3) and random blocks at the bottom or right border of the images (#4358).

This PR does not fix #5349, so it does not make #5350 obsolete.

The if condition in this fix should be equivalent to jj2000/PktDecoder.java#L483

@Snuffleupagus
Copy link
Collaborator

/botio-linux preview

@pdfjsbot
Copy link

From: Bot.io (Linux)


Received

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

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

@Snuffleupagus
Copy link
Collaborator

/botio-windows test

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

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

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

@pdfjsbot
Copy link

From: Bot.io (Windows)


Success

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

Total script time: 19.59 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Regression tests: Passed

@fkaelberer
Copy link
Contributor Author

@CodingFabian
Copy link
Contributor

also fixes https://bugzilla.mozilla.org/show_bug.cgi?id=1085042
(it fixes the out of order JPX, not that the pdf is displayed somewhat twice on itself)

@CodingFabian
Copy link
Contributor

checked a few documents and it does indeed solve many of the random block issues. code looks good. it also complements #5350 to render #5349 completely correct.

@Snuffleupagus
Copy link
Collaborator

@fkaelberer Would it be possible to add a reduced test case to this PR, or at least a linked test, to avoid future regressions?

@fkaelberer
Copy link
Contributor Author

@Snuffleupagus I can not produce the images themselves, but I can extract the images from the problematic pdf files to a separate pdf file. However, I don't know if any of the images is 'free' enough to add them to pdf.js's test cases.

@Snuffleupagus
Copy link
Collaborator

I found a file that we should be able to use as a "linked" test: http://web.archive.org/web/20141021115409/http://www.hunterwellman.com/hw_graphics.pdf.
Add an entry in https://github.com/mozilla/pdf.js/blob/master/test/test_manifest.json, e.g. something like this:

{  "id": "pr5426",
   "file": "pdfs/pr5426.pdf",
   "md5": "...",
   "rounds": 1,
   "link": true,
   "firstPage": 13,
   "lastPage": 13,
   "type": "eq"
},

You also need to add a file called test/pdfs/pr5426.pdf.link, containing the link:

http://web.archive.org/web/20141021115409/http://www.hunterwellman.com/hw_graphics.pdf

CodingFabian added a commit to CodingFabian/pdf.js that referenced this pull request Oct 21, 2014
This patch makes the image from mozilla#5349 appear correctly, the artefacts
for the last packet are fixed in mozilla#5426.
This patch also optimizes some "in-checks" and adds a few header parsings.
@CodingFabian
Copy link
Contributor

i think that the patch also needs to be rebased on master.

@fkaelberer
Copy link
Contributor Author

@Snuffleupagus Thank you for the detailed advice. Or should I rather include https://bug865858.bugzilla.mozilla.org/attachment.cgi?id=742273 from bug 865858, simply because its only 0.5MB instead of 1.9MB?

@Snuffleupagus
Copy link
Collaborator

Or should I rather include https://bug865858.bugzilla.mozilla.org/attachment.cgi?id=742273 from bug 865858, simply because its only 0.5MB instead of 1.9MB?

Yes, that sounds like a better alternative.

@fkaelberer
Copy link
Contributor Author

Done.

@Snuffleupagus
Copy link
Collaborator

/botio-windows test

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

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

Live output at: http://107.22.172.223:8877/172739e81d49729/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Success

Full output at http://107.22.172.223:8877/172739e81d49729/output.txt

Total script time: 18.25 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Regression tests: Passed

@yurydelendik
Copy link
Contributor

thank you for the patch!

/botio-windows makeref

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

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

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

yurydelendik added a commit that referenced this pull request Oct 22, 2014
Fix JPX "Out of Packets" Error (issues 4358, 4659, 4814)
@yurydelendik yurydelendik merged commit 04390d8 into mozilla:master Oct 22, 2014
@pdfjsbot
Copy link

From: Bot.io (Windows)


Success

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

Total script time: 17.91 mins

  • Lint: Passed
  • Make references: Passed
  • Check references: Passed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Blocky/blurry image in first two pages of a PDF
5 participants