mozilla / cubeb-rs

ISC License
61 stars 19 forks source link

Relax constraints on callbacks by dropping Sync requirement #69

Open kinetiknz opened 2 years ago

kinetiknz commented 2 years ago

A cubeb backend may run user callbacks on any thread it chooses to, but must never run the same user callback on multiple threads concurrently. If I understand correctly, the Send constraint is sufficient to represent the appropriate constraint for this, making Sync unnecessary.

Fixes https://github.com/mozilla/cubeb-rs/issues/68

eyeplum commented 2 years ago

I noticed that the state changed callback and device changed callback can be called from threads other than the audio I/O thread.

E.g. with the macOS CoreAudio backend:

This makes me think that maybe the other callbacks need to be kept as Sync?

ChunMinChang commented 2 years ago

A cubeb backend may run user callbacks on any thread it chooses to, but must never run the same user callback on multiple threads concurrently.

This seems true for data callback, but I guess cubeb can fire two different state callbacks at a time on the Mac platform. Suppose we unplug the device after the stream start in the following procedure:

  1. The stream starts successfully
  2. Before firing started state callback on the caller's thread, the device is unplugged, then cubeb will try to reinit the stream. If the device is the last device in the system, then we will fire an error state callback on the stream-task thread.
  3. The started state callback can be fired on the caller's thread at the same time when error state callback on the stream-task thread.

We should probably fix this on the cubeb-coreaudio and check it we have similar problems on other backends.

We could remove Sync for data callback for now and others later, what do you think?