Issues With ImageManipulation.js about image-sequencer HOT 31 CLOSED

Update: Been working on it for the past two days. The same code used to work flawlessly on Browsers. Also, the DataURL is corrupted it seems.

The latest commit is on PR #24

from image-sequencer.

First, let me say it's really cool to be seeing image processing before/after images, even if there is a problem. We should put such examples into the README as it really brings the project to life!

To be clear, are you saying that it does work in browsers, but not in node, or that it used to work in browsers, and now works in neither?

Second, here's the link to the save-pixels package: https://www.npmjs.com/package/save-pixels

Here's the source: https://github.com/scijs/save-pixels/blob/master/save-pixels.js, and it has a test suite here: https://github.com/scijs/save-pixels/blob/master/test/test.js

Is the saturation issue reproducible using just the save-pixels module? If so, we could submit a test to demonstrate it.

My instincts are:

1. the saturation could relate to a mistaken dynamic range - are values stored as 0-255, but in the original file, they have far more depth?
2. A debugging method: you could try to sample single pixels and see if there is a consistent change, like is it 33% lower or higher value?
3. If you rerun the module on the desaturated output image, does it desaturate further, or is it just the same? If it's the same, that supports the (1) theory above, right?
4. It's possible that defaults for precision vary between browser and nodejs... i don't know. The library seems designed for nodejs use, so knowing if this occurs only on the browser would be helpful. Maybe the maintainer of save-pixels would be willing to release a new version to address this?

from image-sequencer.

One odd thing is that the larger images don't seem as affected by the cutoff issue. I'd have guessed that this was due to running out of memory or something, but i'm interested in whether the cutoff with transparent pixels happens the same way each time, or if it's gets a different amount through the image each time. This would tell us if it's a consistent baked-in limitation or if it's some other issue.

from image-sequencer.

Also - where's the code for DoNothing? can you link that too?

from image-sequencer.

Hey @jywarren Thanks for the debugging ideas. Will look into them later today.

Clarification: The save-pixels thing did work on browsers earlier. Earlier I did get the same once issue but i really don't remember how I fixed that. As of now, due to lack of a proper UI, testing this on a browser is really tedious. That is due to the fact that we can not access user files due to browser limitations, unlike node.js. So the user must enter a file via the HTML File Input. Since I was focusing on the functionality, I left out the the UI part as of now. So all I know is that we don't get any problems on the browser one (the one in ./index.html) No disappearing pixels, no saturation issues. This is an older browserified version, though.

The Code for the DoNothing module is in PR #24

from image-sequencer.

Update: The issue of incomplete pixels occurs only with PNG images and not with JPEG images.
But the desaturation problem is still there.

from image-sequencer.

from image-sequencer.

from image-sequencer.

Hey Jeff, Thanks a lot for your inputs! I was pondering over some things. Pretty mind-boggling results.

Some Tests

Test 1 : Creating a PNG DataURL From PNG Image (The DataURL is corrupted, image is desaturated)

      var buffer = require('fs').createWriteStream('output.txt');
      var enc = require('base64-stream').encode();
      savePixels(pixels, 'png').on('end', function() {
        //Writes a DataURL to  output.txt
        buffer.write("data:image/png;base64,"+enc.read().toString());
      }).pipe(enc);

Test 2 : Creating a binary PNG File from PNG Image (The Image is desaturated but COMPLETE)

      var buffer = require('fs').createWriteStream('output.png');
      savePixels(pixels, 'png').on('end', function() {
        //Writes an Image to  output.png
        console.log("Image Written.");
      }).pipe(buffer);

Test 3 : Creating a JPEG DataURL from PNG Image (The DataURL is not corrupted, Image desaturated)

      var buffer = require('fs').createWriteStream('output.txt');
      var enc = require('base64-stream').encode();
      savePixels(pixels, 'jpeg').on('end', function() {
        buffer.write("data:image/jpeg;base64,"+enc.read().toString());
      }).pipe(enc);

Known Test 4 : Works flawlessly on browsers. No desaturation or loss of image data

Test 5 : Comparison of JPEG DataURLs generated by The 'urify' npm module I am using to make initial DataURLs from an input image and the ones generated by get-pixels & save-pixels

There is a module, DoNothingPix which takes the output of previous step, runs get-pixels on it and then save-pixels on it.
There is another module DoNothing which takes the output DataURL of previous image and copies it directly to the output of current step without any get-pixels or save-pixels.

This is my module stack [do-nothing,do-nothing-pix,do-nothing-pix]
So there are four DataURLs, The initial one and the output of each of these three modules.

1. Initial DataURL = A
2. After Module 1 = B
3. After Module 2 = C
4. After Module 3 = D

A=B and C=D but B is not equal to C
Maybe the desaturation plays a role. The DataURL changes after do-nothing-pix but then remains constant after further do-nothing-pix modules. Maybe the encoding technique is different?

Conclusions

• The problem is not with the save-pixels module as far as the incomplete pixels thing is concerned, the issue is with storing the output as a PNG DataURL.
• The problem is not with PNG Images, but with storing these PNG Images as a PNG DataURL after modifying using get-pixels, save-pixels. Saving a PNG Image as a JPEG DataURL or Binary PNG works fine (except for the desaturation thing.)

Further Steps

I believe that what we are doing as

      var buffer = require('fs').createWriteStream('output.txt');
      var enc = require('base64-stream').encode();
      savePixels(pixels, 'png').on('end', function() {
        //Writes a DataURL to  output.txt
        buffer.write("data:image/png;base64,"+enc.read().toString());
      }).pipe(enc);

In the enc.read().toString() part is lacking something. As I am not so good with WriteStreams, could you help me out with this, @jywarren ? Is there a better way to convert a stream object into a dataURL, maybe using a library? I had a look at this option, but failed.

Could you please pull the development branch from ccpandhare/image-sequencer (because the changes from #24 haven't been merged in yet.) and have a look?

Some Doubts (Concerns/Follow-ups)

If you rerun the module on the desaturated output image, does it desaturate further, or is it just the same? If it's the same, that supports the (1) theory above, right?

Yes, you are right. It doesn't desaturate further.

the saturation could relate to a mistaken dynamic range - are values stored as 0-255, but in the original file, they have far more depth?

How can I verify this?

A debugging method: you could try to sample single pixels and see if there is a consistent change, like is it 33% lower or higher value?

I actually tested an image with a 100x100 red square in JPEG format. Couldn't see any noticeable change unlike the red in the before-after example shown above. (I would like to admit here that I did not check the RGB Values, Sorry for that.)

from image-sequencer.

Hi, just so I can go through this methodically:

Could you please pull the development branch from ccpandhare/image-sequencer (because the changes from #24 haven't been merged in yet.) and have a look?

Can you tell me exactly what to try out?

In the enc.read().toString() part is lacking something. As I am not so good with WriteStreams, could you help me out with this, @jywarren ? Is there a better way to convert a stream object into a dataURL, maybe using a library? I had a look at this option, but failed.

I think that's possible -- but we need to more narrowly test the issue. I'd like to suggest you debug this by console.logging the string result and/or doing a comparison somehow if you think that's the problem. Do you suspect this because of something you read? Or (as I do) do you think it's possible that since it's a stream, it expects to run continuously, and by using toString() we may be introducing a non-asynchronous step which overloads memory or otherwise messes up?

Maybe the best way to approach this is to encapsulate the narrow portion we suspect in a separate method, and to unit test that method with actual jpg and png data to try to (with automated tests) confirm what's going wrong.

In general, I'd love to try to

And above:

How can I verify this?

My suggestion for looking for a 3x difference or something like that in the RGB values was an attempt at this. Can we narrow the error any more? Does it affect red only, or other colors? Can you run a test image with pure RGB through it? Something like these: https://duckduckgo.com/?q=calibration+images&atb=v62-4&ia=images

And then you could upload the before/after images here -- http://www.imagecolorpicker.com/ and see if the values are different by a round factor. But this is just a guess.

Finally, you could also try to post a narrow description of this error (linking back to here for full context) to StackOverflow -- a pretty good idea, i think -- and also ask your fellow students and mentors to help you brainstorm a bit...

@publiclab/reviewers -- any ideas here?

from image-sequencer.

Did this make sense? Keep us updated -- don't want you to get stuck and frustrated! Were you able to post to StackOverflow?

from image-sequencer.

Yes, @jywarren thanks for your help!
The issue is certainly surprising. I am having a look.
Was engaged with the Phase-1 completion of the Image Sequencer, as you might have seen.
As soon as I am done with some small bugfixes and minor jobs with that, I will focus on this.
Was too saturated, so shifted my focus for a while! Hope you don't mind.

I will surely take action upon all that you have suggested and revert to you.

from image-sequencer.

from image-sequencer.

I opened up an issue on the save-pixels GitHub repository. Hope that would help a bit and we'll also get some insight on these issuees:

• Is toString() the right way to produce the DatURI
• Why is there a desaturation happening?

from image-sequencer.

from image-sequencer.

Hi, can you open up a StackOverflow issue too and link it from here? Thanks!

from image-sequencer.

I made the StackOverflow Issue. Here's the link.

from image-sequencer.

OK -- let's move forward with other tasks while we wait for a response. I can do some additional testing and outreach to find a solution if we don't get anything in a few days.

from image-sequencer.

Perhaps you should change the title on SO to include "red channel desaturated and/or cut off" and "using save-pixels" -- there's an art to a well-written issue title, and I think it'll get better input. :-)

from image-sequencer.

Done that! Thanks :)

from image-sequencer.

Guess what? I've found the problem with this!
It should be

savePixels(...).pipe(buffer).on('finish',function(){});

and not

savePixels(...).on('finish',function(){}).pipe(buffer);

from image-sequencer.

Now I feel like a fool.

from image-sequencer.

@ccpandhare it happens to the best of us. I am glad you found the solution to this :)

from image-sequencer.

from image-sequencer.

Let's get some of these issues closed up now! Things are getting exciting :-)

Be sure to update your StackOverflow and other issues with the solution!

from image-sequencer.

Thanks a lot for the motivation @jywarren and @ryzokuken! I really appreciate that!

Yes, I'll write an answer to the SO question and GitHub issue at the earliest. Thanks.

from image-sequencer.

@jywarren you could remove the bounty for this issue now :-)

from image-sequencer.

from image-sequencer.

So should I do that? Is it okay?
Not sure about that...

from image-sequencer.

from image-sequencer.

Woah, Okay! Thanks!

from image-sequencer.