Separate chunk decoding and image decoding & inflation

[MarkCallow]

These changes make it possible for the user to first decode all of the chunks, to learn all the details of the image in the file, then decompress, and possibly convert, the image data to the desired format.

My use case is creating KTX containers for GPU textures where, typically, we want to read the image as is, i.e what the hidden decodeGeneric function does. In the API prior to this change we had to

1. Read the whole file decoding the image into some format that is likely not the format we want.
2. Determine the format of the image and modify decode parameters to get what we really want.
3. Rewind the file.
4. Read the whole file, decoding all the chunks and the image again.

With this change:

1. Decode the chunks by calling lodepng_decode_chunks.
2. Determine from that info and command-line options what format we want.
3. Call lodepng_finish_decode to decode the image into the desired format.

The multi-format image file input library I use has the model of providing all the information about the image data to the application which then decides on decoding and conversion. I know of other libraries, e.g. OpenImageIO, which use the same model. This change makes use in such a library much easier.

Change Details:

• Split the decodeGeneric function into the public lodepng_decode_chunks and static inflateIdat.
• Reimplement decodeGeneric to call these 2 functions.
• Add public lodepng_finish_decode function which calls inflateIdat and handles any necessary conversion.
• New warning disables have been added: 4267 for MS VC++ and the equivalent -Wshorten-64-to-32 for clang.
• "32-bit shift implicitly converted to 64 bits" warnings have been fixed by changing 1u constants to 1ull.

The warning fixes were made before I updated the code to LodePNG version 20241228. It is possible the disables are no longer needed. IIRC the 20241228 version has an alternate fix for the last warning: casts instead of my 1ull. I changed my fixes to use the casts but I think 1ull is better.

[MarkCallow]

@lvandeve are you likely to merge this? If there is no chance, tell me as there is then no point in spending more time resolving conflicts from updates to master.

[ReallyNiceGuy]

This would be useful for me. +1

[ReallyNiceGuy]

Did you run the unit test on your changes? It is crashing with a double free.

[MarkCallow]

Did you run the unit test on your changes? It is crashing with a double free.

I had not. When I submitted the PR I expected there would be CI to do so. After I realized there was not, I was distracted by other things. Thanks for reported the problem. I've fixed the problems (there were tow both resulting from deliberately broken PNG files) in the commits I've just pushed.

In order to debug these problems I needed an Xcode project. The fastest way for me to make one was to set up a CMake build. I've submitted that along with a CI workflow in PR #212.

[ReallyNiceGuy]

Your changes leak memory now.

Please run:
valgrind --leak-check=full ./unittest

This will show you the places where you are allocating memory that is not being freed.

[MarkCallow]

I fixed the leaks. Thanks for the heads-up.

valgrind appears to be for Linux only:

brew install valgrind
==> Downloading https://formulae.brew.sh/api/formula.jws.json
==> Downloading https://formulae.brew.sh/api/cask.jws.json
valgrind: Linux is required for this software.
Error: valgrind: An unsatisfied requirement failed this build.

Given that, I am surprised there is even a Brew package.

I used Xcode's Instruments to find the location of the two leaks. That was only 10% of the investigation though. Since the leaks happened only once in a run of unittest, which called the location for every test, much more investigation was needed.

[MarkCallow]

I just pushed a fix for a warning from MSVC about a "use of a potentially uninitialized variable". The warning was bogus but I can see why the compiler thought it might be a problem..

Would be nice to have PR #212 merged so I can rebase this to include it leading to the workflow being run and such warnings being spotted here.