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

not optimized = no output #3

Closed
novli opened this issue Jun 20, 2014 · 10 comments
Closed

not optimized = no output #3

novli opened this issue Jun 20, 2014 · 10 comments

Comments

@novli
Copy link

novli commented Jun 20, 2014

When an image is already optimized or has, for example, 15Mpx dimensions and 0.5Mb filesize, jpeg-compress aborts.
So if I use gulp-imagemin, there's no output image, as exec-buffer throws an error for it.
It would be great, if you pass original image in such cases.

@shinnn
Copy link
Contributor

shinnn commented Jun 22, 2014

Thanks for reporting.
I fixed the problem that this plugin didn't pass images if they are already optimized, and released the new version 0.4.0 to npm. It passes the test in this test case.

But, I cannot confirm the compile abortion for too large images. This plugin can optimize a large image even if it has 12003 x 8035 pixels and 20.4 MB file size.

Would you tell me the detailed spec of the image you couldn't optimize?
Or, would you show me the image URL if we can see it on the Internet?

@novli
Copy link
Author

novli commented Jun 22, 2014

Well, sometimes my clients want me to place their pictures to their pages.
And they usually ask about reasonable image sizes for web 'cause they can't just give me all their heavy pictures. I usually answer that 0.5Mb for one picture is OK.
So they take their pictures and optimize them from ~3.5k x ~5k and ~5Mb into same dimensions and awkward 0.5Mb (ex. https://www.sendspace.com/file/c33jl7 ).
I believe that jpeg-compress is adequate enough to abort the optimization of these crappy images.

@shinnn
Copy link
Contributor

shinnn commented Jun 28, 2014

Now I confirmed the compile abortion you ran into.

I can solve this problem by checking the String of error message strictly, instead of chacking the Number of error code.

However, I do not want to include such a hard coding in my software.
And, to be accurate, it's not a problem of this plugin. To solve this problem fundamentally, we should report an issue to JPEG Archive project, or contribute that.

Thanks again, @novli.

@timdp
Copy link
Contributor

timdp commented Jul 9, 2014

Just to chime in from the duplicate issue I filed (sorry again): looking at jpeg-recompress's source code, even checking the exit status won't solve this problem entirely. There are a few instances of return 1 in jpeg-recompress.c, of which only two are related to this particular situation. You'd have to check the exit status first, and then the string value of stderr. I'm all for filing an issue over at the jpeg-archive project.

@shinnn
Copy link
Contributor

shinnn commented Aug 20, 2014

Recently this plugin has been updated. It includes the major update of jpeg-recompress.

Is this issue fixed in current version? @timdp @novli

@timdp
Copy link
Contributor

timdp commented Aug 23, 2014

Looking at the source code of the new jpeg-recompress, by default, it will now copy the original file if recompressing would increase its size. I ran a quick test and it worked as expected. Looks like this issue can be closed indeed!

@shinnn
Copy link
Contributor

shinnn commented Aug 23, 2014

I want to add a test for this issue.
How can I create an image smaller than the one jpeg-recompress optimized?

@timdp
Copy link
Contributor

timdp commented Aug 23, 2014

I like the way you think.

When I get to work on Monday, I'll see if I can find an image that triggered the bug with the previous version. However, since the new algorithm is supposed to be more efficient, I'm actually assuming that it won't happen anymore. But it's worth a shot.

@shinnn shinnn closed this as completed in 049d899 Aug 23, 2014
@shinnn
Copy link
Contributor

shinnn commented Aug 23, 2014

I changed the fixture file to the theoretically smallest JPEG file.
Now the test covers this case.

Thanks again, @timdp.
I couldn't close this issue without your support!

@timdp
Copy link
Contributor

timdp commented Aug 23, 2014

Awesome. Happy to help!

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

No branches or pull requests

3 participants