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

pass in a raw pixelBuffer to the open command #84

Closed
wants to merge 0 commits into from
Closed

pass in a raw pixelBuffer to the open command #84

wants to merge 0 commits into from

Conversation

wouterverweirder
Copy link
Contributor

It would be nice to open raw pixel buffers. This opens the possibility to manipulate pixel data from other sources than image files.

One project where this could be handy, would be my kinect2 nodejs library. In this library, I'm getting raw pixel data from the kinect cameras. I am now using a hacky workaround to do image manipulation & compression using lwip in this example:

https://github.com/wouterverweirder/node-kinect2/blob/master/examples/color-feed-browser-lwip-hack/index.js

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.19%) when pulling 5ca0be1 on wouterverweirder:master into 84ed584 on EyalAr:master.

@EyalAr
Copy link
Owner

EyalAr commented Dec 10, 2014

@wouterverweirder Thanks for the PR!

Looks good, definitely can be useful.
Great to see an example and tests.

A few comments:

  1. The following will pass arguments validation:

      lwip.open('image.jpg', {foo: 'bar'}, function(err, image){
          // ...
      });

    and crash, although it should obviously be caught when validating arguments. Need to fix and add arguments validation tests.

  2. regarding type.trans, maybe better to rename it to type.transparency, just to be consistent with other parameters in the library.

  3. Perhaps this functionality is better suited in a separate function, and not as part of open. It's not about 'opening' an image, but rather 'wrapping', or 'consuming' an existing pixels buffer. I'm not sure about this; what do you think?

  4. Regarding the consumed pixels buffer - I think it would be prudent to do some sanity checks on it before handing it over to the native code. For example, we know what length the buffer should be (based on type.width, type.height and type.trans).

@EyalAr EyalAr added this to the v0.0.6 milestone Dec 10, 2014
@EyalAr
Copy link
Owner

EyalAr commented Dec 10, 2014

BTW, this can land in v0.0.6, so could you modify the PR to be against the version/0.0.6 branch?

@wouterverweirder
Copy link
Contributor Author

created a new pull request #86 to work on the version/0.0.6 branch

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

Successfully merging this pull request may close these issues.

3 participants