[alsa-devel] [PATCH 2/6] alsa-lib: Fix for sync issue on xrun recover

[Takashi Iwai]

On Fri, 30 Dec 2016 07:28:48 +0100,
sutar.mounesh at gmail.com wrote:
> 
> From: Andreas Pape <apape at de.adit-jv.com>
> 
> If using very short periods, DSHARE/DSNOOP/DMIX may report underruns while in
> status 'prepared'. This prohibits correct recovery. Now slave xrun conditions
> for DSHARE/DSNOOP/DMIX are being handled properly.
> 
> Signed-off-by: Andreas Pape <apape at de.adit-jv.com>
> Signed-off-by: Joshua Frkuska <joshua_frkuska at mentor.com>
> 
> diff --git a/src/pcm/pcm_direct.c b/src/pcm/pcm_direct.c
> index 8f42b19..4234d66 100644
> --- a/src/pcm/pcm_direct.c
> +++ b/src/pcm/pcm_direct.c
> @@ -572,6 +572,9 @@ int snd_pcm_direct_poll_revents(snd_pcm_t *pcm, struct pollfd *pfds, unsigned in
>  	}
>  	switch (snd_pcm_state(dmix->spcm)) {
>  	case SND_PCM_STATE_XRUN:
> +		/*recover slave and update client state to xrun before returning POLLERR*/
> +		snd_pcm_direct_slave_recover(dmix);
> +		snd_pcm_direct_client_chk_xrun(dmix, pcm);
>  	case SND_PCM_STATE_SUSPENDED:
>  	case SND_PCM_STATE_SETUP:
>  		events |= POLLERR;
> @@ -965,6 +968,83 @@ static void save_slave_setting(snd_pcm_direct_t *dmix, snd_pcm_t *spcm)
>  
>  #undef COPY_SLAVE
>  
> +#define direct_sem_down_chk_ret(d, id) {\
> +	int semerr = snd_pcm_direct_semaphore_down(d, DIRECT_IPC_SEM_CLIENT); \
> +	if (semerr) {\
> +		SNDERR("SEMDOWN FAILED with err %d", semerr);\
> +		return semerr;\
> +	}\
> +}
> +
> +#define direct_sem_up_chk_ret(d, id) {\
> +	int semerr = snd_pcm_direct_semaphore_up(d, DIRECT_IPC_SEM_CLIENT); \
> +	if (semerr) {\
> +		SNDERR("SEMUP FAILED with err %d", semerr);\
> +		return semerr;\
> +	}\
> +}

While the check itself is good, I don't like this type of coding.
It hides the code flow inside the macro unnecessarily.

Putting the error message inside the macro is fine, but returning from
a macro isn't.

Also, could you try to align the patches and descriptions in 80
columns as much as possible like in Linux kernel code?  We have no
strict rule, but the generic rule for kernel coding style still
applies.

Since this patch is missing, patches 5 and 6 are pending, too.
Please brush up all and resubmit.


Thanks!

Takashi