search

randy408 / libspng

Simple, modern libpng alternative
https://libspng.org
BSD 2-Clause "Simplified" License
741 stars 75 forks source link

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

Open MrSapps opened 2 years ago

MrSapps commented 2 years ago

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.

randy408 commented 2 years ago

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 commented 1 year ago

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.