-
Notifications
You must be signed in to change notification settings - Fork 764
Description
Hi all,
I only started looking at libpng source code yesterday, so I hope this isn't too presumptuous, but I believe there's a bug in png_set_filter, in pngwrite.c, at line 1166 (from the file as in lpng1654.zip), or line 1176 (current file as seen on GitHub):
png_ptr->do_filter = (png_byte)filters;
Following the logic of the function as best as I can, and from examining older versions, it seems that this line should be one line higher, above the preceding close bracket. The code in that block only runs if rows have already been written, but if rows haven't yet been written, then the line above is still running, and it is overriding the outcome of code further up which updates png_ptr->do_filter with the user's preference.
Specifically: if the user passes a PNG_FILTER_* bitmask value, this passes through (same value gets set twice) and happens to work as expected. But if the user passes PNG_FILTER_VALUE_*, the code at the top correctly sets the value of png_ptr->do_filter to the associated PNG_FILTER_* value (bitmask value), but then the line above overrides this and puts PNG_FILTER_VALUE_* (which is still the value of filters) into png_ptr->do_filter. The result is that no filtering occurs (because PNG_FILTER_VALUE_* is, I think, not a valid value for do_filter).
E.g. Passing PNG_FILTER_SUB results in a 3.7mb file. Passing PNG_FILTER_VALUE_SUB results in a 5.2mb file, the same size as with PNG_FILTER_NONE.
I believe it also interferes with the passing of PNG_NO_FILTERS (but not PNG_FILTER_NONE), as this also currently has no effect (output file is the same size as default/PNG_ALL_FILTERS).
I tested a correction, moving that line up into the block directly above, and it now seems to behave as expected - PNG_FILTER_VALUE_SUB and PNG_FILTER_SUB result in the same output filesize, and PNG_NO_FILTERS now switches off filtering.