Leaking thread objects when encoding with BC6 / BC7 and disabled multithreading

[ChristianFischer]

When I was trying to use the Compressonator SDK to encode textures using BC6 / BC7, I've encountered several issues after trying to disable multithreading. This can easily be reproduced by adding this lines to sdk_sample1.cpp:

    options.dwnumThreads           = 1;
    options.bDisableMultiThreading = true;

and run the command sdk_sample1 runtime/images/ruby.dds runtime/images/ruby_bc7.dds BC7 0.05

• After creating CCodec_BC7, it's members m_NumThreads is set to 1 and m_Use_MultiThreading is set to false.
• Later in InitializeBC7Library, members are initialized to hold objects for this one thread without checking m_Use_MultiThreading and m_LiveThreads becomes 1.
• On destruction of CCodec_BC7, m_Use_MultiThreading IS checked and it never enters the branch to delete the previously created threads. The exit-Flags will never be set, the threads continue running after the codec object was destroyed.
• Same for CCodec_BC6H, but in it's destructor there's a else branch that detaches the thread object. However, since the exit-flag would also be set in the multithreading-branch only, this thread would leak as well.

Without clearing the threads properly, this could lead into serious performance issues when encoding a large amount of images during the lifetime of an application. It would be preferred when those threads wouldnt be created in the first place.

Additionally to this issues, there would be also an opportunity for optimization. With their default values, the codecs always create <CPU count> threads for encoding as well as a decoder object. Even when cleaned up properly, either the encoder or decoder objects won't be used, so it could be useful to tell the codec whether to be used as an encoder or decoder and just initialize the required objects.

[ProbablePrime]

We (Yellow Dog Man Studios), have users experiencing crashes due to an underlying BC6H compression from this library, so I took a look into your issue as i thought it could have been related.

I think you're referring to: https://github.com/GPUOpen-Tools/compressonator/blob/master/cmp_compressonatorlib/bc7/codec_bc7.cpp#L283 in your second bullet point and.

https://github.com/GPUOpen-Tools/compressonator/blob/master/cmp_compressonatorlib/bc7/codec_bc7.cpp#L185 for your third.

I do see the issue as only certain parts of the de-structuring are handled when threads > 1(multithreading = true).

I'm not sure if this specific issue is what's causing our woes, but it is certainly related and provided a good start, so thank you!

[ProbablePrime]

As an update here, it looks like in most cases, we can just discount m_Use_MultiThreading checks because the destruction of threads is forloop based so should work regardless of the amount of threads.

This is clearer in BC6H's codec, but also applies to BC7.

[ProbablePrime]

You can see this change here: https://github.com/Yellow-Dog-Man/compressonator/compare/3dcd895b0ce4%5E...be1d06af64dd

where I basically just remove the m_Use_MultiThreading checks in the deconstructors.