[PATCH] pcm_rate: fix hw_ptr boundary wrapping issue

[Alan Young]

Wrap the hw_ptr using the total position of the slave hw_ptr, including
boundary wraps. Otherwise, small errors can creep in due to residuals
(when boundary is not a multiple of period size) and which can
accumulate.

Signed-off-by: Alan Young <consult.awy at gmail.com>
Fixes: 7570e5d7 ("pcm: rate: fix the hw_ptr update until the boundary available")
---
 src/pcm/pcm_rate.c | 54 ++++++++++++++++++++++++++++++----------------
 1 file changed, 36 insertions(+), 18 deletions(-)

diff --git a/src/pcm/pcm_rate.c b/src/pcm/pcm_rate.c
index c8076859..a29fc5a9 100644
--- a/src/pcm/pcm_rate.c
+++ b/src/pcm/pcm_rate.c
@@ -52,6 +52,7 @@ struct _snd_pcm_rate {
 	snd_pcm_generic_t gen;
 	snd_pcm_uframes_t appl_ptr, hw_ptr, last_slave_hw_ptr;
 	snd_pcm_uframes_t last_commit_ptr;
+	u_int64_t slave_hw_ptr_wrap;
 	snd_pcm_uframes_t orig_avail_min;
 	snd_pcm_sw_params_t sw_params;
 	snd_pcm_format_t sformat;
@@ -562,6 +563,8 @@ static int snd_pcm_rate_init(snd_pcm_t *pcm)
 	if (rate->ops.reset)
 		rate->ops.reset(rate->obj);
 	rate->last_commit_ptr = 0;
+	rate->last_slave_hw_ptr = 0;
+	rate->slave_hw_ptr_wrap = 0;
 	rate->start_pending = 0;
 	return 0;
 }
@@ -638,33 +641,48 @@ snd_pcm_rate_read_areas1(snd_pcm_t *pcm,
 static inline void snd_pcm_rate_sync_hwptr0(snd_pcm_t *pcm, snd_pcm_uframes_t slave_hw_ptr)
 {
 	snd_pcm_rate_t *rate;
-	snd_pcm_sframes_t slave_hw_ptr_diff;
-	snd_pcm_sframes_t last_slave_hw_ptr_frac;
+	snd_pcm_uframes_t new_hw_ptr;
+	snd_pcm_uframes_t slave_residual;
 
 	if (pcm->stream != SND_PCM_STREAM_PLAYBACK)
 		return;
 
 	rate = pcm->private_data;
-	slave_hw_ptr_diff = pcm_frame_diff(slave_hw_ptr, rate->last_slave_hw_ptr, rate->gen.slave->boundary);
-	if (slave_hw_ptr_diff == 0)
+	if (slave_hw_ptr == rate->last_slave_hw_ptr)
 		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 ]
+
+	/*
+	 * Our hw_ptr may wrap at a different time to that of the slave as the
+	 * number of period / boundary may differ. It is essential that our hw_ptr
+	 * wraps at the correct point, along with our appl_ptr. Otherwise the
+	 * avail calculation will be incorrect.
+	 *
+	 * Track the number of slave hw_ptr boundary wraps and use that to calculate
+	 * our new_hw_ptr from the total number of samples converted.
 	 */
-	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));
+	if (slave_hw_ptr < rate->last_slave_hw_ptr) {
+		rate->slave_hw_ptr_wrap += rate->gen.slave->boundary;
+	}
 	rate->last_slave_hw_ptr = slave_hw_ptr;
 
-	rate->hw_ptr %= pcm->boundary;
+	/*
+	 * Calculate rate->hw_ptr using total number of frames consumed by slave hw_ptr,
+	 * including any boundary wraps.
+	 */
+	if (rate->slave_hw_ptr_wrap) {
+		/*
+		 * Restrict explicit 64-bit calculations to case where rate->slave_hw_ptr_wrap
+		 * is non-zero, which will only happen in 32-bit environments.
+		 */
+		u_int64_t wrapped_slave_hw_ptr = slave_hw_ptr + rate->slave_hw_ptr_wrap;
+		new_hw_ptr = ((wrapped_slave_hw_ptr / rate->gen.slave->period_size) * pcm->period_size) % pcm->boundary;
+		slave_residual = wrapped_slave_hw_ptr % rate->gen.slave->period_size;
+	} else {
+		new_hw_ptr = (slave_hw_ptr / rate->gen.slave->period_size) * pcm->period_size;
+		slave_residual = slave_hw_ptr % rate->gen.slave->period_size;
+	}
+	new_hw_ptr += rate->ops.input_frames(rate->obj, slave_residual);
+	rate->hw_ptr = new_hw_ptr % pcm->boundary;
 }
 
 static inline void snd_pcm_rate_sync_hwptr(snd_pcm_t *pcm)
-- 
2.25.1