[alsa-devel] [PATCH - JACK 1/1] jack: Use internal snd_pcm_state to reduce amount of additional variables

[Takashi Iwai]

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