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

Enable writing I;16B images as PNG #7301

Closed
FirefoxMetzger opened this issue Jul 24, 2023 · 3 comments · Fixed by #7302
Closed

Enable writing I;16B images as PNG #7301

FirefoxMetzger opened this issue Jul 24, 2023 · 3 comments · Fixed by #7302

Comments

@FirefoxMetzger
Copy link
Contributor

While investigating an ImageIO issue concerning behavior on big-endian architectures I've noticed that pillow's PNG plugin currently doesn't allow writing 16-bit big-endian images to PNG. I;16 is already supported, so I was wondering if it would be possible to add I;16B as well.

From what I can see, the only change needed to support this would be to add the line "I;16B": ("I;16B", b"\x10\x00"), to this dict:

_OUTMODES = {
# supported PIL modes, and corresponding rawmodes/bits/color combinations
"1": ("1", b"\x01\x00"),
"L;1": ("L;1", b"\x01\x00"),
"L;2": ("L;2", b"\x02\x00"),
"L;4": ("L;4", b"\x04\x00"),
"L": ("L", b"\x08\x00"),
"LA": ("LA", b"\x08\x04"),
"I": ("I;16B", b"\x10\x00"),
"I;16": ("I;16B", b"\x10\x00"),
"P;1": ("P;1", b"\x01\x03"),
"P;2": ("P;2", b"\x02\x03"),
"P;4": ("P;4", b"\x04\x03"),
"P": ("P", b"\x08\x03"),
"RGB": ("RGB", b"\x08\x02"),
"RGBA": ("RGBA", b"\x08\x06"),
}

With this change we would be able to do 16 bit round-trips on big-endian, i.e., we can decode the pixel data directly into a 16 bit big-endian buffer and then use that same buffer when encoding as PNG. This would improve performance and streamline working with native encoding on big-endian machines.

@radarhere
Copy link
Member

I've created PR #7302 to resolve this.

@homm
Copy link
Member

homm commented Jul 24, 2023

This would improve performance and streamline working with native encoding on big-endian machines.

I;16 is already native order for any machines, isn't it?

@FirefoxMetzger
Copy link
Contributor Author

FirefoxMetzger commented Jul 25, 2023

I;16 is already native order for any machines, isn't it?

Yes, I always thought so as well. However, if you check the unpack function for I;16 in Unpack.c it looks like I;16 is treated as little-endian:

Pillow/src/libImaging/Unpack.c

Lines 1152 to 1160 in 3c5324b

unpackI16B_I16(UINT8 *out, const UINT8 *in, int pixels) {
int i;
for (i = 0; i < pixels; i++) {
out[0] = in[1];
out[1] = in[0];
in += 2;
out += 2;
}
}

So I am not sure if I;16 is supposed to be native (and there is a macro missing in the function), or if it is supposed to be little-endian.

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

Successfully merging a pull request may close this issue.

3 participants