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

Tweaked jpeg-js library encoder to allow for RGB image data (rather than... #2

Closed
wants to merge 2 commits into from
Closed

Tweaked jpeg-js library encoder to allow for RGB image data (rather than... #2

wants to merge 2 commits into from

Conversation

emma-floored
Copy link

... assuming RGBA)

Now call with jpg.encode(imageData, quality, channels), channels defaults to 3.

@eugeneware
Copy link
Collaborator

Hi @emma-floored. Why was this closed. Did it break something?

@emma-floored
Copy link
Author

@eugeneware It did, but I have fixed it now. When switching to a new project, I forgot to copy over some of my changes. I'm still new to git and didn't mean to make the pull request yet. Now that I have fixed my code I could open it again.

@emma-floored emma-floored reopened this Jul 2, 2014
@eugeneware
Copy link
Collaborator

Thanks @emma-floored

I had a quick look, however, the tests (just run npm test when in the root dir) are failing.

If you could make the tests pass, and a few tests for some images with different numbers of channels that would be great.

@benwiley4000
Copy link
Collaborator

This has been discussed in #3 and it may be preferable to accept a number of image data formats for encoding (rgba, rgb, bgr, etc). Either way, since this library is widely used now it would be preferable not to change the default channel count.

As-is I don't think we can merge this PR, but @emma-floored if you're still interested in this sort of change we'd be happy to consider a new pull request, perhaps using the format discussed in #3! 🙂

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

Successfully merging this pull request may close these issues.

None yet

3 participants