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

spng_encode_image doesn't fail if a pixel index is bigger than spng_plte::n_entries #233

Open
MrSapps opened this issue Nov 6, 2022 · 2 comments
Labels
bug Something isn't working

Comments

@MrSapps
Copy link

MrSapps commented Nov 6, 2022

Describe the bug

When calling spng_encode_image with spng_set_plte where the pal n_entries is 255 but the image data contains an index that is exactly 255 it gets clamped to 254 instead of failing.

To Reproduce

See the following code that saves a PNG:

(https://github.com/paulsapps/alive_reversing/blob/36ed9e560207e88afac77ba0694377ba50258820/Source/relive_lib/data_conversion/PNGFile.cpp#L351)

Specifically that a pal is set with 255 entries and that the source image data has at least 1 pixel with an index value of 255.

Expected behavior
spng_encode_image should fail due to one of the source image index values being outside of the range of the pal n_entries.

Platform (please complete the following information):

Additional context
This auto clamping to max pal value resulted in an annoying rendering bug that took a while to track down to this :). Would have been much faster to find if the png encode failed due to the pal index being out of bounds.

@MrSapps MrSapps added the bug Something isn't working label Nov 6, 2022
@randy408
Copy link
Owner

randy408 commented Nov 7, 2022

Currently there is no option to check palette indices, but there is also no clamping going on, did you decode with SPNG_FMT_PNG/RAW to check the values? Maybe the default handling of out-of-range palette values is confusing (but is the same for most PNG libraries when decoding), they are decoded as black, opaque pixels.

I believe the default behavior of the reference implementation when encoding is to handle this as an error.

@svgeesus
Copy link

This decoding behavior is correct per specification:

When decoding an indexed-color PNG, if out-of-range indexes are encountered, decoders have historically varied in their handling of this error. Displaying the pixel as opaque black is one common error recovery tactic, and is now required by this specification. Older implementations will vary, and so the behavior must not be relied on by encoders.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants