search

rust-av / ffms2-rs

ffms2 bindings
MIT License
10 stars 5 forks source link

Allow safe multithreading by implementing "Send" #20

Closed Christopher22 closed 3 years ago

Christopher22 commented 3 years ago

Currently, the crate is too restrictive in terms of multithreading. While the underlying FFMS2 pointers might not allow concurrent access from different threads (and are consequently not "Sync"), they rely neither on thread-local storage nor thread-specific locks and is therefore safe to send.

Luni-4 commented 3 years ago

Thanks a lot!

Could you please add a minimal integration test for your safe-multithreading implementation? In this way we can immediately verify if a problem arises.

Christopher22 commented 3 years ago

Any idea for such a test? To the best of my knowledge, there is no way replicating test behavior exactly between at least two threads. Even if there would be an error, it is most likely a Heisenbug.

However, I would call it overkill nevertheless. The C world is almost completely "Send"y by default. I think we are safe by referring to the FFMS2 documentation which does not give any hint for thread-local storage or thread-specific locks. Besides, some functions of the library are explicitly NOT sync: If they would be, that might be a hint for mutexes or something like that. :)

Luni-4 commented 3 years ago

I can't come come up with a test that does not consider ffsm2 directly, so we can merge this PR as-is for now

Christopher22 commented 3 years ago

Nice!