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.

Sign up for GitHub

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

Jump to bottom

installation failed on Windows #39

Closed
seantw opened this issue Aug 4, 2018 · 46 comments
Closed

installation failed on Windows #39

seantw opened this issue Aug 4, 2018 · 46 comments

Comments

@seantw
Copy link

seantw commented Aug 4, 2018

I get some error messages:

C:\>npm install -g utif
npm ERR! code ENOGIT
npm ERR! Error while executing:
npm ERR! undefined ls-remote -h -t ssh://[email protected]/makr28/jpgjs.git
npm ERR!
npm ERR! undefined
npm ERR! No git binary found in $PATH
npm ERR!
npm ERR! Failed using git.
npm ERR! Please check if you have git installed and in your PATH.

npm ERR! A complete log of this run can be found in:
npm ERR!     C:\Users\Sean\AppData\Roaming\npm-cache\_logs\2018-08-04T09_18_23_181Z-debug.log

C:\>

install log file:
2018-08-04T09_18_23_181Z-debug.log
@photopea
Copy link
Owner

photopea commented Aug 4, 2018

Hi, I never used NPM. @Scimonster could you help us with this?

@seantw seantw mentioned this issue Aug 4, 2018
Closed
@revoyrm
Copy link

revoyrm commented Aug 11, 2018

hmm That repo is still on my git, so it should work. I'll look into it

@revoyrm
Copy link

revoyrm commented Aug 11, 2018

Do you have git installed on your PC? You can check by typing: git in your command prompt. You will need git installed to install that package.

@makr28
Copy link

makr28 commented Aug 11, 2018

Sorry I commented with the wrong user. revoyrm is me, the jpgjs fork this uses is mine.

Anyway you can download git here --> https://git-scm.com/downloads

@seantw
Copy link
Author

seantw commented Aug 12, 2018

I just installed this module using NPM.
I don't need to use git. and I don't know how to use git.
Just confused , why does image processing require git?
UTIF.js only 40KB, But installing git requires more than 40MB.

@makr28
Copy link

makr28 commented Aug 12, 2018

So earlier there was an issue with JPGJS which is needed for the jpeg decoders. This issue was only for when you were using nodejs. Which if you're doing npm, you are.

Jpgjs is a dead project, they dont accept contributions. So i made a fork and fixed it so it would work with nodejs projects. Since I was just fixing it in a fork and it wasn't my original project I didn't release it to npm. So we used to github link in the package.json.

I don't know what the etiquette for releasing projects that are just Forks to npm but I will look into it. That said it will be a lot quicker for you to just install git.

The installer I linked you to it was pretty straightforward to follow, just click next a bunch. It won't try to install anything extra and it's pretty small. You do not have to use git or anything. But npm installing utif will use git to get to get the jpgJS package

After installing it you will need to close the command prompt or terminal you are using and reopen it to have git show up in your path. Then this will work.

@hipstersmoothie
Copy link
Contributor

@makr28 how do you feel about moving to jpeg-js instead? it's hosted on NPM and is a much smaller library.

@revoyrm
Copy link

revoyrm commented Aug 15, 2018 via email

@hipstersmoothie
Copy link
Contributor

Yeah jpeg-js actaully builds off of jpgjs. It even says so in the readme! I think it should have the necessary features.

"Decoding
This library builds on the work of two other JPEG javascript libraries, namely jpgjs for the decoding which is licensed under the Apache 2.0 License below:"

@revoyrm
Copy link

revoyrm commented Aug 15, 2018 via email

@photopea
Copy link
Owner

Guys, I suggest to switch to the JPEG decoder from pdf.js : mozilla/pdf.js#9729

@revoyrm
Copy link

revoyrm commented Aug 15, 2018 via email

@hipstersmoothie
Copy link
Contributor

hipstersmoothie commented Aug 15, 2018 •
edited
Loading

I'd say its not too good. That would increase your code by 200kb! Most being unused.

https://bundlephobia.com/[email protected]

What is the resistance to switching to jpeg-js?

  • same funcitonality
  • 16.6kb vs a whopping 402.6kB

cons:

  • ????

If you guys choose to install pdfjs I'm gonna be forced to fork and publish UTIF so my bundle doesn't explode.

current: 441.35kb (jpgjs 189.99kb + pako 213.83kb + utif 37.53kb)
with pdfjs: 653.96kb (pngjs 402.6kb + pako 213.83kb + utif 37.53kb)
with jpeg-js: 268.06kb (jpeg-js 16.7kb + pako 213.83kb + utif 37.53kb)

@hipstersmoothie
Copy link
Contributor

just utif alone adds 2-4 seconds to page load time on a GOOD connection

@revoyrm
Copy link

revoyrm commented Aug 15, 2018 via email

@hipstersmoothie
Copy link
Contributor

did they separate them into a separate package? how would you go about only pulling the jpeg-decoder in?

@hipstersmoothie
Copy link
Contributor

hipstersmoothie commented Aug 15, 2018 •
edited
Loading

something like this might work

const jpgjs = require('pdfjs-dist/lib/core/jpg')

but it still doesn't address the fact that the dependency is still enormous

I just tried installing the above in my testing project and it adds ~300kb to my project

@hipstersmoothie
Copy link
Contributor

It also looks like jpgjs from pdf-dist uses streams so you might have trouble using it in browser

@revoyrm
Copy link

revoyrm commented Aug 15, 2018 via email

@photopea
Copy link
Owner

Guys, I never used NPM or Node.js , so I don't understand some fragments, that you are mentioning.

I manually extracted JPEG decoder from PDF.js . It is "worth" attaching it for me in www.Photopea.com , as it is also used for opening JPG files.

TIFFs with JPG compression are quite rare and I would recommend not to attach any JPEG decoder, unless you are sure your TIFFs will need it. But I don't know how to do it in NPM / Node.js terms.

@hipstersmoothie
Copy link
Contributor

In npm there is no such thing as an optional peer dependancy. So you cannot optionally include specific dependancies based on what the user needs. This is a dead end/

I think you should keep the JPEG decoder in UTIF. Even though it may not be common, it exists and is handle in the current package, so it should be kept that way. Handling all the cases is better in my mind, and @photopea even mentioned it in another issue of mine.

For UTIF to pull in just the jpgjs package from pngjs, they would of had to code it in a specific way that makes it possible to pull in just a part of the package. So this may be hard to do without submitting PRs to the pngjs repo.

The best route that I see is going with jpeg-js. I have not heard from either of you why it wouldn't work or how jpgjs is better. I have listed reasons why jpeg-js is better above. Mainly it's feature parity and small size.

Regardless of you npm experience you have to recognize that the browser downloading 16kb is better than the browser downloading 400kb. This effects page load time, something you probably care about for your web app photopea.

@photopea do you have objections to switching to jpeg-js? What are your specific concerns?

@revoyrm
Copy link

revoyrm commented Aug 15, 2018 via email

@hipstersmoothie
Copy link
Contributor

  1. If he is doing that it is a highly inefficient way to do it. You are potentially includeing the jpeg decoder twice. and since jpgjs is huge it's probably adding .5mb to initial page load.

  2. Def not included in utif as you are currently maintaining a working fork of jpgjs

@revoyrm Can you try with jpeg-js first? I still see no reason to go with a package that is 12.5x bigger for no gain

@revoyrm
Copy link

revoyrm commented Aug 16, 2018 via email

@hipstersmoothie
Copy link
Contributor

I can whip it together tonight. It's really more about if @photopea will merge it

@photopea
Copy link
Owner

@syvaidya Hi, you are the one who understands JPEG, and who implemented the support of Type 6 and Type 7 compression for UTIF.js .

Would it be possible to cut out the smallest sufficient part of a JPEG library and attach it directly into UTIF.js ? I guess such part will not require any changes in the future. How large that part would be? Could it fit into 10 - 15 kB?

It wold be very nice to get rid of any dependencies in UTIF.js, as many people are complaining about them (see above).

@hipstersmoothie hipstersmoothie mentioned this issue Aug 16, 2018
Closed
@hipstersmoothie
Copy link
Contributor

I opened #40 but I would love to see it get down lower like @photopea mentioned above

@hipstersmoothie hipstersmoothie mentioned this issue Aug 23, 2018
Closed
@hipstersmoothie
Copy link
Contributor

@photopea we have more users hitting this bug. It would be great if you could take a look at my PR

@rxluz
Copy link

rxluz commented Aug 24, 2018

@photopea this package is breaking when I try to upload my application to AWS, is possible replace this to jpeg-js?

@photopea
Copy link
Owner

Hi Guys, I just inserted JpegDecoder into UTIF.js (about 11kB extra) , so the only dependency is pako.js now.

@Scimonster could you please update the NPM configuration files and upload it to NPM? :) Thanks!

@revoyrm
Copy link

revoyrm commented Aug 24, 2018 via email

@makr28
Copy link

makr28 commented Aug 24, 2018

@photopea during my initial testing of the current code, it is working with old style jpegs but not new style jpegs. I am going to look into it more before I open an issue.

@photopea
Copy link
Owner

Please, just send me a TIFF file, which does not work.

@revoyrm
Copy link

revoyrm commented Aug 24, 2018 via email

@photopea
Copy link
Owner

photopea commented Aug 24, 2018 •
edited
Loading

Hi, I tried to open your TIFF file Type7-TTN2.tif in www.Photopea.com (which still uses an old UTIF.js), and it does not work either. So I think that your file is corrupted. I tested some other Type7 compressed TIFFs with the new version of UTIF.js, and they work well.

@revoyrm
Copy link

revoyrm commented Aug 24, 2018 via email

@revoyrm
Copy link

revoyrm commented Aug 24, 2018 via email

@photopea
Copy link
Owner

@revoyrm Could you please tell me, which "jpgjs" do you have in mind?

@revoyrm
Copy link

revoyrm commented Aug 24, 2018 via email

@photopea
Copy link
Owner

I tried to use UTIF.js with this code: https://github.com/makr28/jpgjs/blob/master/jpg.js and I am getting absolutely the same error for your TIFF Type 7 files: JPEG error: SOI not found . So are you really sure it used to work with your files in the past?

@revoyrm
Copy link

revoyrm commented Aug 24, 2018 via email

@photopea
Copy link
Owner

photopea commented Aug 24, 2018 •
edited
Loading

Could you send me the UTIF.js and JPG.js that you are using? You can copy-paste the JS code into www.pastebin.com .

@hipstersmoothie
Copy link
Contributor

@photopea would you merge it if I wrote tests? Upgrades generally go easier when there are unit tests

@revoyrm
Copy link

revoyrm commented Aug 24, 2018 via email

@photopea
Copy link
Owner

It was fixed and it should work well now.

@revoyrm
Copy link

revoyrm commented Aug 25, 2018 via email

@hipstersmoothie hipstersmoothie mentioned this issue Aug 26, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

switch to jpgjs-dist hipstersmoothie/UTIF.js
6 participants
@hipstersmoothie @seantw @revoyrm @photopea @rxluz @makr28