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

PNG format seems broken since 0.0.8 #194

Closed
gautaz opened this issue Oct 13, 2015 · 5 comments
Closed

PNG format seems broken since 0.0.8 #194

gautaz opened this issue Oct 13, 2015 · 5 comments

Comments

@gautaz
Copy link

gautaz commented Oct 13, 2015

Hello,

I'm using lwip to generate image thumbnails.
In order to test my code, I'm comparing a generated thumbnail to a standard one in PNG format.
Everything was fine since 0.0.8 on which my test raises an error.

In order to diagnose the issue, I have tried to compare the generated image with the standard one by using ImageMagick, I've got the following error:

11:17 $ LANG=C compare old.png new.png diff.png
compare: improper image header `new.png' @ error/png.c/ReadPNGImage/3921.
11:27 $ compare --version
Version: ImageMagick 6.9.2-3 Q16 x86_64 2015-10-02 http://www.imagemagick.org

new.png is the newly created thumbnail produced by lwip 0.0.8.

This might be related to #165 but I am not sure whether this is an ImageMagick issue or an lwip one.

@EyalAr
Copy link
Owner

EyalAr commented Oct 13, 2015

Can you open new.png in another software?

@gautaz
Copy link
Author

gautaz commented Oct 13, 2015

new.png can still be opened in any image viewer.
I'm just concerned that the latest changes in lwip might not comply with the PNG standard.
As said before, the issue might be an ImageMagick issue...

@EyalAr
Copy link
Owner

EyalAr commented Oct 13, 2015

You are correct that #165 changes the way PNGs are encoded, but the additional code only runs if metadata is actually passed in.
If you are not passing in metadata I don't see any reason why the encoding would be different.

Another thing that changed in 0.0.8 is the version of pnglib, which could somehow be related to this.

This will require more thorough checks to see if the problem is LWIP, ImageMagick or the new version of pnglib.

I'll install ImageMagick on my machine to check.

But before I do -
Does this happen with every image?
Can you upload here an image on which it does happen?
Can you verify you are not setting any metadata?
Can you verify this does not happen with LWIP 0.0.7?

Thanks.

@gautaz
Copy link
Author

gautaz commented Oct 13, 2015

In the meantime, I have reinstalled lwip 0.0.7 to work around the issue and reinstalled lwip 0.0.8 back again to reproduce the failing sample.
And now, I can just not reproduce the issue. What is bugging me is that before reinstalling, the issue was reproducible.
I still have this difference between the old PNG sample and the new one but now ImageMagick accepts it without complaining.

The original image I'm using to operate my tests is available here.

Here are both thumbnails produced by lwip 0.0.7 and 0.0.8:
thumbnail3-0 0 7 thumbnail3-0 0 8

And here is the difference that ImageMagick is now accepting to produce:
diff

The source code I'm using to produce thumbnails:

var storeImageAsIcon = function (buffer, format, callback) {
    lwip.open(buffer, format, function (err, image) {
        if (err) { return callback(err); }
        var width = Math.ceil(50 * image.width() / image.height());

        image.resize(width, 50, function (err, image) {
            if (err) { return callback(err); }

            image.toBuffer('png', function (err, buffer) {
                if (err) { return callback(err); }
                var writestream = gfs().createWriteStream({
                    metadata: { type: 'icon' },
                    chunk_size: 1024,
                    content_type: 'image/png'
                });
                writestream.on('close', function (file) {
                    log.info('icon', file._id, 'stored');
                    return callback(null, file._id);
                });
                streamifier.createReadStream(buffer).pipe(writestream);
            });
        });
    });
};

As you can see, the only metadata I'm manipulating is for gridfs and not for lwip.

I have tried with a bunch of other images and I still cannot reproduce the issue, sorry.
If you cannot reproduce on your side, I will just close the issue...

@EyalAr
Copy link
Owner

EyalAr commented Oct 14, 2015

I'll close it for now. But please let me know if the issue comes back.

A possible explanation is that when you first upgraded from 0.0.7 to 0.0.8, somehow the old binaries were not cleaned properly and were used with the new JS.

@EyalAr EyalAr closed this as completed Oct 14, 2015
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

2 participants