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

Pull in decoder updates from pdf.js #30

Open
patrickhulce opened this issue Jul 16, 2017 · 4 comments
Open

Pull in decoder updates from pdf.js #30

patrickhulce opened this issue Jul 16, 2017 · 4 comments

Comments

@patrickhulce
Copy link
Collaborator

pdf.js used by Firefox is a fairly well maintained fork of the same base file that the decoder is based on. It has a number of useful fixes including nice options for the colorspace outputs I mentioned in #3.

Here's the git history for the main file. I had a local branch that attempted to bring in a minimal diff since they've had a lot of style changes without significant logic changes, but it ended up failing a few tests. If we could get an automated flow going for pulling in updates that'd really ease the maintenance burden here. (They're also missing a few changes we've had like the progressive JPEG fix #25).

@benwiley4000
Copy link
Collaborator

benwiley4000 commented Jul 16, 2017

Wow good find!

The truly ideal scenario would be if that jpeg decoder could be spun out as a separate module which we could contribute our changes to, and depend upon. However that might not be possible since their version uses special linting rules as well as newer ECMAScript.

Do you have any idea about a timeline/logistics for making this work?

I'm also curious if the pdf.js maintainers have any thoughts @Snuffleupagus @yurydelendik

This was referenced Jul 16, 2017
@metabench
Copy link

I've just got the latest decoder from pdf.js integrated into a version of this decoder file, with a modified decode function that supports 3 bytes per pixel images, RGB, not just RGBA. The alpha channel isn't ever present in JPEGs anyway, as far as I know.

@benwiley4000 @patrickhulce Would it be worth making a new branch for the upgraded decoder?

@patrickhulce
Copy link
Collaborator Author

Woah that's great @metabench! I think that depends on what the diff ends up like. We want to make sure we don't regress any of the fixes we've landed and last time I tried the PDF.js decoder did behave differently for some of our cases.

If it's easy to review and it's not essentially an entire rewrite from git diff perspective 😆 let's do it!. IMO, at this stage in the project's life it seems more important to be stable though if it's scary-looking.

@metabench
Copy link

@patrickhulce I'm sorry, a while ago I thought I had the answer, I ran a file which I thought had these enhancements but I think I ran the wrong file. Sorry for not clearing this up earlier.

I'm working on coming up with an all-round enchanced library, and will bear in mind PDF.js's JPEG capabilities.

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

No branches or pull requests

3 participants