On Thu, Jun 11, 2020 at 03:14:23PM +0530, Vinod Koul wrote: > On 11-06-20, 11:09, Jaroslav Kysela wrote: > > Dne 11. 06. 20 v 10:46 Charles Keepax napsal(a): > > > On Wed, Jun 10, 2020 at 04:28:20PM +0530, Vinod Koul wrote: > > > > On 10-06-20, 12:40, Jaroslav Kysela wrote: > > > > > Dne 10. 06. 20 v 12:07 Srinivas Kandagatla napsal(a): > > > > > > Here is the sequence of events and state transitions: > > > > > > > > > > > > 1. set_params (Track 1), state = SNDRV_PCM_STATE_SETUP > > > > > > 2. set_metadata (Track 1), no state change, state = SNDRV_PCM_STATE_SETUP > > > > > > 3. fill and trigger start (Track 1), state = SNDRV_PCM_STATE_RUNNING > > > > > > 4. set_next_track (Track 2), state = SNDRV_PCM_STATE_RUNNING > > > > > > 5. partial_drain (Track 1), state = SNDRV_PCM_STATE_SETUP > > > > > > 6 snd_compr_drain_notify (Track 1), state = SNDRV_PCM_STATE_SETUP > > > > > > 7. fill data (Track 2), state = SNDRV_PCM_STATE_PREPARED > > > > > > 8. set_metadata (Track 3), no state change, state = SNDRV_PCM_STATE_PREPARED > > > > > > 9. set_next_track (Track 3), !! FAILURE as state != SNDRV_PCM_STATE_RUNNING > Yeah sorry I overlooked that case and was thinking of it being invoked > from driver! > > Yes this would make the snd_compr_stop() behave incorrectly.. so this > cant be done as proposed. > > But we still need to set the draining stream state properly and I am > thinking below now: > > diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c > index 509290f2efa8..9aba851732d7 100644 > --- a/sound/core/compress_offload.c > +++ b/sound/core/compress_offload.c > @@ -929,7 +929,9 @@ static int snd_compr_partial_drain(struct snd_compr_stream *stream) > } > > stream->next_track = false; > - return snd_compress_wait_for_drain(stream); > + retval = snd_compress_wait_for_drain(stream); > + stream->runtime->state = SNDRV_PCM_STATE_RUNNING; > + return retval; This is looking better, I think you probably need to make the switch to RUNNING conditional on snd_compress_wait_for_drain succeeding, as the state should remain in DRAINING if we are interrupted or some such. I also have a slight concern that since snd_compress_wait_for_drain, releases the lock the set_next_track could come in before the state is moved to RUNNING, but I guess from user-spaces perspective that is the same as it coming in whilst the state is still DRAINING, so should be handled fine? Thanks, Charles
