Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Thread safety of low level transcoder #25

Closed
Saticmotion opened this issue Jun 2, 2019 · 6 comments
Closed

Thread safety of low level transcoder #25

Saticmotion opened this issue Jun 2, 2019 · 6 comments

Comments

@Saticmotion
Copy link

Saticmotion commented Jun 2, 2019

Hey Rich!

We're working on an integration for the Unity Engine. The WebGL sample creates an instance of basisu_transcoder per texture, but as far as I can tell we only need one basisu_transcoder in our entire dll.

We're also looking into multithreading, but we can't quite tell if a single basisu_transcoder for the entire library would be thread safe. Nearly all functions in basisu_transcoder seem to be pure functions. In the readme you've mentioned that transcode_image_level and transcode_slice are thread safe.

That leaves start_transcoding, which I can't tell right away if it's thread safe, because it interacts with the low level decoder.

More details of our investigation are here: atteneder/KtxUnity#1

@richgel999
Copy link
Contributor

Hi - The current design has some state in the basisu_transcoder object. There's m_lowlevel_decoder (the m_pFile_data/m_file_data_size members are dead code and I'll remove them on the next checkin). There's also m_block_endpoint_preds[], which is only temporarily used while transcoding.

m_lowlevel_decoder doesn't became valid until you call start_transcoding(), which signals your intention to start decoding texture slices. So you can use a single global instance of basisu_transcoder to query for basic file information (like get_file_info() etc.), and that would be thread safe in the current design. But once you call start_transcoding(), the local state becomes valid, and is used during transcoding.

So you can't have a single global object and call start_transcoding() on it from multiple threads. If you have multiple .basis files that you want to transcode simultaneously, you'll need multiple basisu_transcoder's.

Right now, you can't call transcode_image_level() from multiple threads on the same object, because m_block_endpoint_preds[] (which is used during transcoding) is per-object state. I will fix this this week. The intention is that you can transcode multiple images or mipmaps from the same .basis file on multiple threads.

Does this make sense?

-Rich

@Saticmotion
Copy link
Author

Thanks Rich, this is exactly what I wanted to know!

@richgel999
Copy link
Contributor

Thanks, I'll update this issue once it's possible to transcode multiple slices of a single .basis file from multiple threads.

@richgel999
Copy link
Contributor

I'm adding an optional pointer to a transcoding state struct to all the transcoding API's. For video, this state struct needs to stay persistent as it holds the last frame's indices. I'll have this checked in hopefully tomorrow or Sat.

@richgel999
Copy link
Contributor

I have checked in an optional state struct that you can use during transcoding for threading purposes.

@Saticmotion
Copy link
Author

Great stuff, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants