rikukissa / node-red-contrib-image-output

🏞 Easy way of previewing and examining images in your flows
MIT License
13 stars 6 forks source link

Buffers not displayed #3

Closed bartbutenaers closed 6 years ago

bartbutenaers commented 6 years ago

Hi Riku,

Sorry to disturb you again.

Will first start with good news. When I use my node-red-contrib-multipart-stream-decoder node to get an MJPEG stream from my camera, your node still works great at 10 images per second:

image

But when I add your node directly behind my decoder node (which sends images as buffers) and before the base64 encoder, no image is displayed:

image Some thoughts about this:

Now I will stop posting issues for today ;-)

Kind regards, Bart

bartbutenaers commented 6 years ago

Hi @rikukissa,

The first of my 3 above questions is not valid. Did the test again and now it seems to be working fine:

image

Don't know what I did wrong in my previous test. So it works fine both with Buffers and base64 encoded strings!

Kind regards, Bart

rikukissa commented 6 years ago

Oh, good to hear it works! I was just about to start debugging this, but maybe I'll focus on the other issues you've created :) 100% appreciate your input. Keep it coming!

rikukissa commented 6 years ago

If the input is not correct, perhaps you can log an error or display a (red) node status. Because now you cannot see immediately if there is no input, or whether the input is incorrect...

Yup, you're absolutely right. It should be useful to get some feedback of why the node isn't working as expected. I'd imagine this would've also helped you when you came across your issue.

Perhaps you could also display something like this to make it very user friendly (but only a nice to have)

Totally! I actually found myself needing to fill in empty inputs with a transparent image Buffer while I was using this, which definitely isn't ideal.

Would you have time to drop me a PR addressing these things? Otherwise I can also fix these, but it might take me a while :)

bartbutenaers commented 6 years ago

Deal ;-) I will create a pull request for both items, but need some extra input:

Bart

rikukissa commented 6 years ago

Yep, I would say it's our safest bet to accept Buffers and base64 strings. According to my testing, you can call .toString('base64') to a base64 string and it just returns the original base64 string, so that's probably not an issue and in that way caters for both allowed input types.

In your if statement, you could do something like this:

if ( !(Buffer.isBuffer(msg.payload) || (typeof msg.payload === 'string' && /some_regexp/.test(msg.payload)))) {
      node.error("Input should be a Buffer or a ??????????", msg);
      node.status({fill:"red",shape:"ring", "Invalid input"});
}

RegExp for checking if string is a valid base64 string: https://stackoverflow.com/a/8571649

About the 'no preview available' image:

I would maybe prefer using a constructed SVG element for telling the user there's no preview available rather than using a static image. Maybe we could for instance node.send data: null back to the browser in cases where the msg.payload doesn't pass our validation.

bartbutenaers commented 6 years ago

all 3 topics above are currently implemented in the different pull requests. I will close this issue, and discuss these items in their pull requests (if necessary).