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

Support for L colorspace and extra channels. #14

Open
alexjc opened this issue Nov 25, 2022 · 10 comments
Open

Support for L colorspace and extra channels. #14

alexjc opened this issue Nov 25, 2022 · 10 comments

Comments

@alexjc
Copy link
Contributor

alexjc commented Nov 25, 2022

For the record in case people are looking: I have implemented both these features in my own branch at https://github.com/alexjc/jxlpy.

Just need to merge #13 first, then I'll port them over!

@olokelo
Copy link
Owner

olokelo commented Nov 30, 2022

@alexjc Thanks for your awesome work. Unfortunately I've run into an issue with Github actions package building and I have little time to fix it lately. It might also resolve the #10 issue building wheels for Windows and Mac OS. I would like to fix that first in order to make PyPI release with prebuilt wheels. If you have any ideas on how to manage building wheel packages for jxlpy in Github actions I would appreciate any help.

@alexjc
Copy link
Contributor Author

alexjc commented Nov 30, 2022

Is there any more information than in #10?

@olokelo
Copy link
Owner

olokelo commented Nov 30, 2022

Not really. The current build pipeline is a bit hacky, it uses CIBW_BEFORE_ALL to build libjxl and then cibuildwheel to build and fix Python package. The libjxl build instructions are Linux specific and use some workarounds which don't work anymore with latest version of libjxl.
I had ran into issues trying to split build process into steps.

@alexjc
Copy link
Contributor Author

alexjc commented Nov 30, 2022

How do you test it locally? I'll take a look.

@alexjc
Copy link
Contributor Author

alexjc commented Nov 30, 2022

If you could merge #13 that would help, so I can get it working with the latest version.

@olokelo
Copy link
Owner

olokelo commented Nov 30, 2022

How do you test it locally? I'll take a look.

I tested it with Act. Thanks :)

If you could merge #13 that would help, so I can get it working with the latest version.

Okay, will do.

@olokelo
Copy link
Owner

olokelo commented Nov 30, 2022

Awesome! Merged, Thanks again :)

I've also somewhat tested your master branch and it seems like your change in get_frame breaks printing pixel colors in examples. Could you please review it?

@alexjc
Copy link
Contributor Author

alexjc commented Nov 30, 2022

Oh, yes I know what that is — relates to the extra channels. I will fix it...

@alexjc
Copy link
Contributor Author

alexjc commented Mar 9, 2023

Unfortunately, my implementation of L and I;16 modes from PIL doesn't work. It seems to use a different stride somewhere and I don't know where it is...

I set num_channels, num_color_channels, and bits_per_sample correctly — something missing. Advice welcome!

EDIT: Image too big to inline, but you can view it here:
https://user-images.githubusercontent.com/445208/223972746-69abf54b-7920-48ef-bbfe-8f4f8a4eebbb.jpg

@alexjc
Copy link
Contributor Author

alexjc commented Mar 9, 2023

OK, it's fixed it was a bug in ImageMagick actually.

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