search

tazz4843 / coqui-stt

Safe wrapper around the Coqui STT C library
108 stars 9 forks source link

implement Send for Stream? #6

Closed droundy closed 2 years ago

droundy commented 2 years ago

I'm not sure how Stream is supposed to be used. It doesn't implement Send. Is it really not threadsafe, or is this an oversight?

tazz4843 commented 2 years ago

This has come up before in issue #4. I'm not 100% sure if it actually is threadsafe, and I don't want to introduce accidental unsafety. I did run into this issue myself, and I implemented a custom type that actually is Send + Sync (it just spawns a new thread in the background, keeps the Stream on that thread, and uses channels to communicate). That type is gated behind a feature flag: threadsafe-streams.

droundy commented 2 years ago

I've changed my code so I no longer need Stream to be Send, but it does seem with fixing it possible. I'll ask in the coqui-stt gitter.

droundy commented 2 years ago

Never mind, I see you already asked...

tazz4843 commented 2 years ago

image

It appears Stream is Send, but it can't be used from multiple threads at the same time, which makes the current implementation !Sync, unless all functions that currently take &self and access the C API were changed to take &mut self. If you'd like to do that in your PR (#7) I'd be willing to merge that. Otherwise, I'll look into that myself at some point, and merge your PR.

droundy commented 2 years ago

It's already !Sync, because it has a *mut in it.

droundy commented 2 years ago

Never mind, I get it