lvandeve / lodepng

PNG encoder and decoder in C and C++.
zlib License
2.03k stars 420 forks source link

Decode inserts onto end of array, but should probably resize instead. #140

Closed alecazam closed 3 years ago

alecazam commented 3 years ago

There are a couple spots where lodepng inserts onto the end of the out buffer passed in. Callers more likely expect a resize instead of a constantly growing buffer. This would happen if the out buffer was reused to load another image.

Alternately, clearing the out array before the insert would also work.

unsigned decode(std::vector& out, unsigned& w, unsigned& h, const unsigned char in, size_t insize, LodePNGColorType colortype, unsigned bitdepth) { unsigned char buffer; unsigned error = lodepng_decode_memory(&buffer, &w, &h, in, insize, colortype, bitdepth); if(buffer && !error) { State state; state.info_raw.colortype = colortype; state.info_raw.bitdepth = bitdepth; size_t buffersize = lodepng_get_raw_size(w, h, &state.info_raw); out.insert(out.end(), &buffer[0], &buffer[buffersize]); <- should this use reize lodepng_free(buffer); } return error; }

lvandeve commented 3 years ago

This is for the following reason: there are potential scenarios where one may want to append, e.g. encode a png image into some stream that embeds a PNG image. For decoding it's in case one wants to decode raw pixels at the end of some existing stream.

It's easy to clear the array before calling encode/decode, and in most cases one would use a freshly initialized array anyway.

On the other hand if the encoder/decoder would overwrite an existing array, the user has to append which is slightly harder.

So since it's easier to clear, than to append, the behavior chosen is to append, not to clear and overwrite

alecazam commented 3 years ago

I think some commentary there or bufferToAppendTo as a variable name would help on the call. Typically out parameters are not an append, but created fresh.