On Thu, 15 Mar 2018 15:05:39 +0100, Wischer, Timo (ADITG/ESB) wrote:
Hello Takashi,
Well, the fact that the state change is done for PREPARE before the plugin callback can be seen as a generic bug of ioplug. The best would be to fix in ioplug side.
This change could possibly brake other existing IO plugins. Do you think we should really change it?
We need to check it, of course. But the behavior of the prepare function is certainly different from others, so it's more likely a bug.
For RUNNING, instead of keeping an internal shadow state, a saner way would be to have a jack->running bool flag indicating whether it's really running.
With the following pending commits [1] and [2] we would than have 3 additional variables to indicate
- running
That's this patch, and...
- xruns
This should be notified to ioplug via snd_pcm_ioplug_set_state().
- draining
This is also a thing to be fixed in ioplug side caller side. The ioplug should set DRAINING state before calling the callback.
My idea was to avoid this variables and use the already existing snd_pcm_state enum. With further commits which I have not yet sent I want to make the JACK plugin thread save. But instead of using locking I am using atomic lock-less access (to avoid blocking of the JACK daemon). Therefore I cannot call snd_pcm_state() to get the internal state. I have to forward the status by my own to the JACK thread. Due to that I need an internal variable which will be atomically accessed and represents the state. (I think it is not intended by ALSA to extend the ALSA library to update the internal ALSA state atomically so that it can be read atomically from any thread. Therefore I have not yet another idea how to transfer the state to the JACK thread with atomic lock-less access.) I could replace the assignments to the internal state in all IO plug callback functions in the JACK plugin with jack->state = io->state But I am not sure if this is really helpful for the understanding.
That'd be great if everything is ready, but this can't be a reason to add a shadow state and modify the code as in the patch for now.
thanks,
Takashi