[alsa-devel] [PATCH 4/6] alsa-lib:pcm: fix the hw_ptr update until the boundary available.

[Mahendran Kuppusamy (RBEI/ECF3)]

Hello Takashi Iwan, Hello Sutar,

Sorry for the late response. 

Yes, Fractional calculation is required for the overwrapping case only. So, Please do optimization and go ahead of "doing this conditionally only for the overwrapping case".

Please let me know, Any changes required for me.

Thanks in advance.

Best regards,

Mahendran Kuppusamy
RBEI/ECF3  

Tel. +91 80 6136-4450 


-----Original Message-----
From: Takashi Iwai [mailto:tiwai at suse.de] 
Sent: Monday, January 02, 2017 7:29 PM
To: sutar.mounesh at gmail.com
Cc: alsa-devel at alsa-project.org; mounesh_sutar at mentor.com; Mahendran Kuppusamy (RBEI/ECF32) <Mahendran.Kuppusamy at in.bosch.com>
Subject: Re: [PATCH 4/6] alsa-lib:pcm: fix the hw_ptr update until the boundary available.

On Fri, 30 Dec 2016 07:29:27 +0100,
sutar.mounesh at gmail.com wrote:
> 
> From: mahendran.k <mahendran.kuppusamy at in.bosch.com>
> 
> For long time test case, the slave_hw_ptr will exceed the boundary and 
> wraparound the slave_hw_ptr. This slave boundary wraparound will cause 
> the rate->hw_ptr to wraparound irrespective of the rate->boundary 
> availability and due to that the available size goes wrong.
> 
> Hence, To get the correct available size,
> - Its necessary to increment the rate->hw_ptr upto the rate->boundary 
> and then wraparound the rate->hw_ptr.
> - While handling fraction part of slave period, rounded value will be 
> introduced by input_frames(). To eliminate rounding issue on 
> rate->hw_ptr, subtract last rounded value from rate->hw_ptr and add 
> new rounded value of present slave_hw_ptr fraction part to rate->hw_ptr.
> 
> Signed-off-by: mahendran.k <mahendran.kuppusamy at in.bosch.com>
> Signed-off-by: Mounesh Sutar <mounesh_sutar at mentor.com>

Thanks, it's been a long-standing issue, indeed.

I applied this one (again) with a slight coding style fix now.
But...

> diff --git a/src/pcm/pcm_rate.c b/src/pcm/pcm_rate.c index 
> 1f830dd..5bee77c 100644
> --- a/src/pcm/pcm_rate.c
> +++ b/src/pcm/pcm_rate.c
> @@ -50,7 +50,7 @@ typedef struct _snd_pcm_rate snd_pcm_rate_t;
>  
>  struct _snd_pcm_rate {
>  	snd_pcm_generic_t gen;
> -	snd_pcm_uframes_t appl_ptr, hw_ptr;
> +	snd_pcm_uframes_t appl_ptr, hw_ptr, last_slave_hw_ptr;
>  	snd_pcm_uframes_t last_commit_ptr;
>  	snd_pcm_uframes_t orig_avail_min;
>  	snd_pcm_sw_params_t sw_params;
> @@ -563,14 +563,28 @@ static inline void 
> snd_pcm_rate_sync_hwptr0(snd_pcm_t *pcm, snd_pcm_uframes_t sl  {
>  	snd_pcm_rate_t *rate = pcm->private_data;
>  
> +	snd_pcm_sframes_t slave_hw_ptr_diff = slave_hw_ptr - rate->last_slave_hw_ptr;
> +	snd_pcm_sframes_t last_slave_hw_ptr_frac;
> +
>  	if (pcm->stream != SND_PCM_STREAM_PLAYBACK)
>  		return;
> -	/* FIXME: boundary overlap of slave hw_ptr isn't evaluated here!
> -	 *        e.g. if slave rate is small... 
> -	 */
> -	rate->hw_ptr =
> -		(slave_hw_ptr / rate->gen.slave->period_size) * pcm->period_size +
> -		rate->ops.input_frames(rate->obj, slave_hw_ptr % rate->gen.slave->period_size);
> +
> +	if (slave_hw_ptr_diff < 0)
> +		slave_hw_ptr_diff += rate->gen.slave->boundary; /* slave boundary wraparound */
> +	else if (slave_hw_ptr_diff == 0)
> +		return;
> +	last_slave_hw_ptr_frac = rate->last_slave_hw_ptr % rate->gen.slave->period_size;
> +	/* While handling fraction part fo slave period, rounded value will be introduced by input_frames().
> +	 * To eliminate rounding issue on rate->hw_ptr, subtract last rounded value from rate->hw_ptr and
> +	 * add new rounded value of present slave_hw_ptr fraction part to rate->hw_ptr. Hence,
> +	 * rate->hw_ptr += [ (no. of updated slave periods * pcm rate period size) -
> +	 * 				fractional part of last_slave_hw_ptr rounded value +
> +	 * 				fractional part of updated slave hw ptr's rounded value ] */
> +	rate->hw_ptr += (
> +			(((last_slave_hw_ptr_frac + slave_hw_ptr_diff) / rate->gen.slave->period_size) * pcm->period_size) -
> +			rate->ops.input_frames(rate->obj, last_slave_hw_ptr_frac) +
> +			rate->ops.input_frames(rate->obj, (last_slave_hw_ptr_frac + slave_hw_ptr_diff) % rate->gen.slave->period_size));
> +	rate->last_slave_hw_ptr = slave_hw_ptr;

.... this ended up with a substantial increase of computation.
Can we optimize, e.g. doing this conditionally only for the overwrapping case?


thanks,

Takashi