Hi, did this one slip past? I have not see any response. Alan.
-------- Forwarded Message --------
Subject: [PATCH] pcm_rate: fix hw_ptr boundary wrapping issue
Date: Thu, 5 Jan 2023 15:35:46 +0000
From: Alan Young [1]<consult.awy@xxxxxxxxx>
To: [2]alsa-devel@xxxxxxxxxxxxxxxx
CC: [3]tiwai@xxxxxxx, [4]mahendran.kuppusamy@xxxxxxxxxxxx, Alan Young
[5]<consult.awy@xxxxxxxxx>
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 [6]<consult.awy@xxxxxxxxx>
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
References
1. mailto:consult.awy@xxxxxxxxx
2. mailto:alsa-devel@xxxxxxxxxxxxxxxx
3. mailto:tiwai@xxxxxxx
4. mailto:mahendran.kuppusamy@xxxxxxxxxxxx
5. mailto:consult.awy@xxxxxxxxx
6. mailto:consult.awy@xxxxxxxxx
