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

implement Send on encoder and decoder wrappers #34

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gdesmott
Copy link

@gdesmott gdesmott commented May 7, 2024

Make API more convenient to use.

Make API more convenient to use.
@blaind
Copy link
Owner

blaind commented May 15, 2024

Thank you for your pull request!

I see that you've proposed adding an unsafe impl Send for Decode/EncodeWrapper's. Given the potential risks associated with unsafe code and multi-threading, could you please provide a safety comment in the code?

It would be helpful to explain why this implementation is safe, particularly in light of the following considerations:

  • The (libwebp) seems not to mention specific guarantees regarding thread safety for the components used in this struct.

The Decoder and Encoder in this library manage the access to the pointer, and don't expose any concurrent safety hazards. But wondering if the transfer between threads itself is ok.

@gdesmott
Copy link
Author

But wondering if the transfer between threads itself is ok.

Tbh I'm not totally sure either. Send is usually ok, but if you do not know about this either it may be best to check with libwebp upstream.

@blaind
Copy link
Owner

blaind commented May 15, 2024

Thanks. I'll try to check it out. Agreed, Send should be ok here but want to err on the safe side.

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

Successfully merging this pull request may close these issues.

2 participants