[alsa-devel] [PATCH 1/5] pcm: direct: Add generic hw_ptr_alignment function for dmix, dshare and dsnoop
From: Vanitha Channaiah vanitha.channaiah@in.bosch.com
This patch is same as the fix in below commit commit 6b058fda9dce ("pcm: dmix: Add option to allow alignment of slave pointers")
Signed-off-by: Vanitha Channaiah vanitha.channaiah@in.bosch.com --- src/pcm/pcm_direct.c | 19 +++++++++++++++++++ src/pcm/pcm_direct.h | 8 ++++++++ src/pcm/pcm_dmix.c | 25 ++----------------------- 3 files changed, 29 insertions(+), 23 deletions(-)
diff --git a/src/pcm/pcm_direct.c b/src/pcm/pcm_direct.c index 666a8ce..411a035 100644 --- a/src/pcm/pcm_direct.c +++ b/src/pcm/pcm_direct.c @@ -2040,3 +2040,22 @@ int snd_pcm_direct_parse_open_conf(snd_config_t *root, snd_config_t *conf,
return 0; } + +void snd_pcm_direct_reset_slave_ptr(snd_pcm_t *pcm, snd_pcm_direct_t *dmix) +{ + dmix->slave_appl_ptr = dmix->slave_hw_ptr = *dmix->spcm->hw.ptr; + + if (dmix->hw_ptr_alignment == SND_PCM_HW_PTR_ALIGNMENT_ROUNDUP || + (dmix->hw_ptr_alignment == SND_PCM_HW_PTR_ALIGNMENT_AUTO && + pcm->buffer_size <= pcm->period_size * 2)) + dmix->slave_appl_ptr = + ((dmix->slave_appl_ptr + dmix->slave_period_size - 1) / + dmix->slave_period_size) * dmix->slave_period_size; + else if (dmix->hw_ptr_alignment == SND_PCM_HW_PTR_ALIGNMENT_ROUNDDOWN || + (dmix->hw_ptr_alignment == SND_PCM_HW_PTR_ALIGNMENT_AUTO && + (dmix->slave_period_size * SEC_TO_MS) / + pcm->rate < LOW_LATENCY_PERIOD_TIME)) + dmix->slave_appl_ptr = dmix->slave_hw_ptr = + ((dmix->slave_hw_ptr / dmix->slave_period_size) * + dmix->slave_period_size); +} diff --git a/src/pcm/pcm_direct.h b/src/pcm/pcm_direct.h index da5e280..a71aab1 100644 --- a/src/pcm/pcm_direct.h +++ b/src/pcm/pcm_direct.h @@ -24,6 +24,11 @@
#define DIRECT_IPC_SEMS 1 #define DIRECT_IPC_SEM_CLIENT 0 +/* Seconds representing in Milli seconds */ +#define SEC_TO_MS 1000 +/* slave_period time for low latency requirements in ms */ +#define LOW_LATENCY_PERIOD_TIME 10 +
typedef void (mix_areas_t)(unsigned int size, volatile void *dst, void *src, @@ -257,6 +262,8 @@ struct snd_pcm_direct { snd1_pcm_direct_get_chmap #define snd_pcm_direct_set_chmap \ snd1_pcm_direct_set_chmap +#define snd_pcm_direct_reset_slave_ptr \ + snd1_pcm_direct_reset_slave_ptr
int snd_pcm_direct_semaphore_create_or_connect(snd_pcm_direct_t *dmix);
@@ -341,6 +348,7 @@ int snd_pcm_direct_slave_recover(snd_pcm_direct_t *direct); int snd_pcm_direct_client_chk_xrun(snd_pcm_direct_t *direct, snd_pcm_t *pcm); int snd_timer_async(snd_timer_t *timer, int sig, pid_t pid); struct timespec snd_pcm_hw_fast_tstamp(snd_pcm_t *pcm); +void snd_pcm_direct_reset_slave_ptr(snd_pcm_t *pcm, snd_pcm_direct_t *dmix);
struct snd_pcm_direct_open_conf { key_t ipc_key; diff --git a/src/pcm/pcm_dmix.c b/src/pcm/pcm_dmix.c index c5592cd..dcde40d 100644 --- a/src/pcm/pcm_dmix.c +++ b/src/pcm/pcm_dmix.c @@ -55,9 +55,6 @@ const char *_snd_module_pcm_dmix = ""; #define STATE_RUN_PENDING 1024 #endif
-#define SEC_TO_MS 1000 /* Seconds representing in Milli seconds */ -#define LOW_LATENCY_PERIOD_TIME 10 /* slave_period time for low latency requirements in ms */ - /* * */ @@ -560,30 +557,12 @@ static int snd_pcm_dmix_hwsync(snd_pcm_t *pcm) } }
-static void reset_slave_ptr(snd_pcm_t *pcm, snd_pcm_direct_t *dmix) -{ - dmix->slave_appl_ptr = dmix->slave_hw_ptr = *dmix->spcm->hw.ptr; - - if (dmix->hw_ptr_alignment == SND_PCM_HW_PTR_ALIGNMENT_ROUNDUP || - (dmix->hw_ptr_alignment == SND_PCM_HW_PTR_ALIGNMENT_AUTO && - pcm->buffer_size <= pcm->period_size * 2)) - dmix->slave_appl_ptr = - ((dmix->slave_appl_ptr + dmix->slave_period_size - 1) - / dmix->slave_period_size) * dmix->slave_period_size; - else if (dmix->hw_ptr_alignment == SND_PCM_HW_PTR_ALIGNMENT_ROUNDDOWN || - (dmix->hw_ptr_alignment == SND_PCM_HW_PTR_ALIGNMENT_AUTO && - (dmix->slave_period_size * SEC_TO_MS) / pcm->rate < LOW_LATENCY_PERIOD_TIME)) - dmix->slave_appl_ptr = dmix->slave_hw_ptr = - ((dmix->slave_hw_ptr / dmix->slave_period_size) * - dmix->slave_period_size); -} - static int snd_pcm_dmix_reset(snd_pcm_t *pcm) { snd_pcm_direct_t *dmix = pcm->private_data; dmix->hw_ptr %= pcm->period_size; dmix->appl_ptr = dmix->last_appl_ptr = dmix->hw_ptr; - reset_slave_ptr(pcm, dmix); + snd_pcm_direct_reset_slave_ptr(pcm, dmix); return 0; }
@@ -592,7 +571,7 @@ static int snd_pcm_dmix_start_timer(snd_pcm_t *pcm, snd_pcm_direct_t *dmix) int err;
snd_pcm_hwsync(dmix->spcm); - reset_slave_ptr(pcm, dmix); + snd_pcm_direct_reset_slave_ptr(pcm, dmix); err = snd_timer_start(dmix->timer); if (err < 0) return err;
From: Vanitha Channaiah vanitha.channaiah@in.bosch.com
snd_pcm_wait() is waiting for longer time more than two periodic size as avail is less than avail_min by few frames. This is because the hw_ptr read from the kernel during snd_pcm_start() is not period aligned which is ahead of few frames.
These changes are adaptation of same fix as given for dmix commit ("6b058fda9dce8f416774ae54975f5706f3f5a6da") ("pcm-dmix-Add-option-to-allow-alignment-of-slave-poin.patch")
Signed-off-by: Vanitha Channaiah vanitha.channaiah@in.bosch.com --- src/pcm/pcm_dshare.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/src/pcm/pcm_dshare.c b/src/pcm/pcm_dshare.c index f135b5d..45e2597 100644 --- a/src/pcm/pcm_dshare.c +++ b/src/pcm/pcm_dshare.c @@ -333,16 +333,16 @@ static int snd_pcm_dshare_reset(snd_pcm_t *pcm) snd_pcm_direct_t *dshare = pcm->private_data; dshare->hw_ptr %= pcm->period_size; dshare->appl_ptr = dshare->last_appl_ptr = dshare->hw_ptr; - dshare->slave_appl_ptr = dshare->slave_hw_ptr = *dshare->spcm->hw.ptr; + snd_pcm_direct_reset_slave_ptr(pcm, dshare); return 0; }
-static int snd_pcm_dshare_start_timer(snd_pcm_direct_t *dshare) +static int snd_pcm_dshare_start_timer(snd_pcm_t *pcm, snd_pcm_direct_t *dshare) { int err;
snd_pcm_hwsync(dshare->spcm); - dshare->slave_appl_ptr = dshare->slave_hw_ptr = *dshare->spcm->hw.ptr; + snd_pcm_direct_reset_slave_ptr(pcm, dshare); err = snd_timer_start(dshare->timer); if (err < 0) return err; @@ -364,7 +364,8 @@ static int snd_pcm_dshare_start(snd_pcm_t *pcm) else if (avail < 0) return 0; else { - if ((err = snd_pcm_dshare_start_timer(dshare)) < 0) + err = snd_pcm_dshare_start_timer(pcm, dshare); + if (err < 0) return err; snd_pcm_dshare_sync_area(pcm); } @@ -547,7 +548,8 @@ static snd_pcm_sframes_t snd_pcm_dshare_mmap_commit(snd_pcm_t *pcm, return 0; snd_pcm_mmap_appl_forward(pcm, size); if (dshare->state == STATE_RUN_PENDING) { - if ((err = snd_pcm_dshare_start_timer(dshare)) < 0) + err = snd_pcm_dshare_start_timer(pcm, dshare); + if (err < 0) return err; } else if (dshare->state == SND_PCM_STATE_RUNNING || dshare->state == SND_PCM_STATE_DRAINING) { @@ -755,6 +757,7 @@ int snd_pcm_dshare_open(snd_pcm_t **pcmp, const char *name, dshare->slowptr = opts->slowptr; dshare->max_periods = opts->max_periods; dshare->var_periodsize = opts->var_periodsize; + dshare->hw_ptr_alignment = opts->hw_ptr_alignment; dshare->sync_ptr = snd_pcm_dshare_sync_ptr;
retry:
On Tue, 30 Apr 2019 09:38:40 +0200, vanitha.channaiah@in.bosch.com wrote:
From: Vanitha Channaiah vanitha.channaiah@in.bosch.com
snd_pcm_wait() is waiting for longer time more than two periodic size as avail is less than avail_min by few frames. This is because the hw_ptr read from the kernel during snd_pcm_start() is not period aligned which is ahead of few frames.
These changes are adaptation of same fix as given for dmix commit ("6b058fda9dce8f416774ae54975f5706f3f5a6da") ("pcm-dmix-Add-option-to-allow-alignment-of-slave-poin.patch")
Signed-off-by: Vanitha Channaiah vanitha.channaiah@in.bosch.com
Again, this patch description is too ambiguous.
And, if it enables the hw_ptr_alignment option, update the documentation as well.
thanks,
Takashi
src/pcm/pcm_dshare.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/src/pcm/pcm_dshare.c b/src/pcm/pcm_dshare.c index f135b5d..45e2597 100644 --- a/src/pcm/pcm_dshare.c +++ b/src/pcm/pcm_dshare.c @@ -333,16 +333,16 @@ static int snd_pcm_dshare_reset(snd_pcm_t *pcm) snd_pcm_direct_t *dshare = pcm->private_data; dshare->hw_ptr %= pcm->period_size; dshare->appl_ptr = dshare->last_appl_ptr = dshare->hw_ptr;
- dshare->slave_appl_ptr = dshare->slave_hw_ptr = *dshare->spcm->hw.ptr;
- snd_pcm_direct_reset_slave_ptr(pcm, dshare); return 0;
}
-static int snd_pcm_dshare_start_timer(snd_pcm_direct_t *dshare) +static int snd_pcm_dshare_start_timer(snd_pcm_t *pcm, snd_pcm_direct_t *dshare) { int err;
snd_pcm_hwsync(dshare->spcm);
- dshare->slave_appl_ptr = dshare->slave_hw_ptr = *dshare->spcm->hw.ptr;
- snd_pcm_direct_reset_slave_ptr(pcm, dshare); err = snd_timer_start(dshare->timer); if (err < 0) return err;
@@ -364,7 +364,8 @@ static int snd_pcm_dshare_start(snd_pcm_t *pcm) else if (avail < 0) return 0; else {
if ((err = snd_pcm_dshare_start_timer(dshare)) < 0)
err = snd_pcm_dshare_start_timer(pcm, dshare);
snd_pcm_dshare_sync_area(pcm); }if (err < 0) return err;
@@ -547,7 +548,8 @@ static snd_pcm_sframes_t snd_pcm_dshare_mmap_commit(snd_pcm_t *pcm, return 0; snd_pcm_mmap_appl_forward(pcm, size); if (dshare->state == STATE_RUN_PENDING) {
if ((err = snd_pcm_dshare_start_timer(dshare)) < 0)
err = snd_pcm_dshare_start_timer(pcm, dshare);
} else if (dshare->state == SND_PCM_STATE_RUNNING || dshare->state == SND_PCM_STATE_DRAINING) {if (err < 0) return err;
@@ -755,6 +757,7 @@ int snd_pcm_dshare_open(snd_pcm_t **pcmp, const char *name, dshare->slowptr = opts->slowptr; dshare->max_periods = opts->max_periods; dshare->var_periodsize = opts->var_periodsize;
dshare->hw_ptr_alignment = opts->hw_ptr_alignment; dshare->sync_ptr = snd_pcm_dshare_sync_ptr;
retry:
-- 2.7.4
Patch mailing list Patch@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/patch
From: Vanitha Channaiah vanitha.channaiah@in.bosch.com
snd_pcm_wait() is waiting for longer time more than two periodic size as avail is less than avail_min by few frames. This is because the hw_ptr read from the kernel during snd_pcm_start() is not period aligned which is ahead of few (Nperiod + 1)frames.
Signed-off-by: Vanitha Channaiah vanitha.channaiah@in.bosch.com --- src/pcm/pcm_direct.c | 7 ++++--- src/pcm/pcm_dmix.c | 2 ++ src/pcm/pcm_dshare.c | 2 ++ src/pcm/pcm_dsnoop.c | 5 ++++- 4 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/src/pcm/pcm_direct.c b/src/pcm/pcm_direct.c index 411a035..b56da85 100644 --- a/src/pcm/pcm_direct.c +++ b/src/pcm/pcm_direct.c @@ -2043,11 +2043,12 @@ int snd_pcm_direct_parse_open_conf(snd_config_t *root, snd_config_t *conf,
void snd_pcm_direct_reset_slave_ptr(snd_pcm_t *pcm, snd_pcm_direct_t *dmix) { - dmix->slave_appl_ptr = dmix->slave_hw_ptr = *dmix->spcm->hw.ptr; - + /* For buffer size less than two period size, the start position + * of slave app ptr is rounded up in order to avoid xruns + */ if (dmix->hw_ptr_alignment == SND_PCM_HW_PTR_ALIGNMENT_ROUNDUP || (dmix->hw_ptr_alignment == SND_PCM_HW_PTR_ALIGNMENT_AUTO && - pcm->buffer_size <= pcm->period_size * 2)) + pcm->buffer_size < pcm->period_size * 2)) dmix->slave_appl_ptr = ((dmix->slave_appl_ptr + dmix->slave_period_size - 1) / dmix->slave_period_size) * dmix->slave_period_size; diff --git a/src/pcm/pcm_dmix.c b/src/pcm/pcm_dmix.c index dcde40d..274726e 100644 --- a/src/pcm/pcm_dmix.c +++ b/src/pcm/pcm_dmix.c @@ -562,6 +562,7 @@ static int snd_pcm_dmix_reset(snd_pcm_t *pcm) snd_pcm_direct_t *dmix = pcm->private_data; dmix->hw_ptr %= pcm->period_size; dmix->appl_ptr = dmix->last_appl_ptr = dmix->hw_ptr; + dmix->slave_appl_ptr = dmix->slave_hw_ptr = *dmix->spcm->hw.ptr; snd_pcm_direct_reset_slave_ptr(pcm, dmix); return 0; } @@ -571,6 +572,7 @@ static int snd_pcm_dmix_start_timer(snd_pcm_t *pcm, snd_pcm_direct_t *dmix) int err;
snd_pcm_hwsync(dmix->spcm); + dmix->slave_appl_ptr = dmix->slave_hw_ptr = *dmix->spcm->hw.ptr; snd_pcm_direct_reset_slave_ptr(pcm, dmix); err = snd_timer_start(dmix->timer); if (err < 0) diff --git a/src/pcm/pcm_dshare.c b/src/pcm/pcm_dshare.c index 45e2597..3847645 100644 --- a/src/pcm/pcm_dshare.c +++ b/src/pcm/pcm_dshare.c @@ -333,6 +333,7 @@ static int snd_pcm_dshare_reset(snd_pcm_t *pcm) snd_pcm_direct_t *dshare = pcm->private_data; dshare->hw_ptr %= pcm->period_size; dshare->appl_ptr = dshare->last_appl_ptr = dshare->hw_ptr; + dshare->slave_appl_ptr = dshare->slave_hw_ptr = *dshare->spcm->hw.ptr; snd_pcm_direct_reset_slave_ptr(pcm, dshare); return 0; } @@ -342,6 +343,7 @@ static int snd_pcm_dshare_start_timer(snd_pcm_t *pcm, snd_pcm_direct_t *dshare) int err;
snd_pcm_hwsync(dshare->spcm); + dshare->slave_appl_ptr = dshare->slave_hw_ptr = *dshare->spcm->hw.ptr; snd_pcm_direct_reset_slave_ptr(pcm, dshare); err = snd_timer_start(dshare->timer); if (err < 0) diff --git a/src/pcm/pcm_dsnoop.c b/src/pcm/pcm_dsnoop.c index d08b624..2bbbc2d 100644 --- a/src/pcm/pcm_dsnoop.c +++ b/src/pcm/pcm_dsnoop.c @@ -278,6 +278,7 @@ static int snd_pcm_dsnoop_reset(snd_pcm_t *pcm) dsnoop->hw_ptr %= pcm->period_size; dsnoop->appl_ptr = dsnoop->hw_ptr; dsnoop->slave_appl_ptr = dsnoop->slave_hw_ptr; + snd_pcm_direct_reset_slave_ptr(pcm, dsnoop); return 0; }
@@ -285,12 +286,13 @@ static int snd_pcm_dsnoop_start(snd_pcm_t *pcm) { snd_pcm_direct_t *dsnoop = pcm->private_data; int err; - + if (dsnoop->state != SND_PCM_STATE_PREPARED) return -EBADFD; snd_pcm_hwsync(dsnoop->spcm); snoop_timestamp(pcm); dsnoop->slave_appl_ptr = dsnoop->slave_hw_ptr; + snd_pcm_direct_reset_slave_ptr(pcm, dsnoop); err = snd_timer_start(dsnoop->timer); if (err < 0) return err; @@ -627,6 +629,7 @@ int snd_pcm_dsnoop_open(snd_pcm_t **pcmp, const char *name, dsnoop->max_periods = opts->max_periods; dsnoop->var_periodsize = opts->var_periodsize; dsnoop->sync_ptr = snd_pcm_dsnoop_sync_ptr; + dsnoop->hw_ptr_alignment = opts->hw_ptr_alignment;
retry: if (first_instance) {
On Tue, 30 Apr 2019 09:38:41 +0200, vanitha.channaiah@in.bosch.com wrote:
From: Vanitha Channaiah vanitha.channaiah@in.bosch.com
snd_pcm_wait() is waiting for longer time more than two periodic size as avail is less than avail_min by few frames. This is because the hw_ptr read from the kernel during snd_pcm_start() is not period aligned which is ahead of few (Nperiod + 1)frames.
Signed-off-by: Vanitha Channaiah vanitha.channaiah@in.bosch.com
Ditto as patch 2, the description is too ambiguous, and the update of documentation is missing.
And...
--- a/src/pcm/pcm_direct.c +++ b/src/pcm/pcm_direct.c @@ -2043,11 +2043,12 @@ int snd_pcm_direct_parse_open_conf(snd_config_t *root, snd_config_t *conf,
void snd_pcm_direct_reset_slave_ptr(snd_pcm_t *pcm, snd_pcm_direct_t *dmix) {
- dmix->slave_appl_ptr = dmix->slave_hw_ptr = *dmix->spcm->hw.ptr;
- /* For buffer size less than two period size, the start position
* of slave app ptr is rounded up in order to avoid xruns
if (dmix->hw_ptr_alignment == SND_PCM_HW_PTR_ALIGNMENT_ROUNDUP || (dmix->hw_ptr_alignment == SND_PCM_HW_PTR_ALIGNMENT_AUTO &&*/
pcm->buffer_size <= pcm->period_size * 2))
dmix->slave_appl_ptr = ((dmix->slave_appl_ptr + dmix->slave_period_size - 1) / dmix->slave_period_size) * dmix->slave_period_size;pcm->buffer_size < pcm->period_size * 2))
It's not good to change the helper function semantics out of sudden, even without any description.
thanks,
Takashi
From: Vanitha Channaiah vanitha.channaiah@in.bosch.com
snd_pcm_sw_params() is reformatted by using _snd_pcm_sw_params_internal() function
Signed-off-by: Vanitha Channaiah vanitha.channaiah@in.bosch.com --- src/pcm/pcm.c | 12 +----------- src/pcm/pcm_local.h | 1 + src/pcm/pcm_params.c | 21 +++++++++++++++++++++ 3 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/src/pcm/pcm.c b/src/pcm/pcm.c index 3a71d79..f0db545 100644 --- a/src/pcm/pcm.c +++ b/src/pcm/pcm.c @@ -968,21 +968,11 @@ int snd_pcm_sw_params(snd_pcm_t *pcm, snd_pcm_sw_params_t *params) } #endif __snd_pcm_lock(pcm); /* forced lock due to pcm field change */ - err = pcm->ops->sw_params(pcm->op_arg, params); + err = _snd_pcm_sw_params_internal(pcm, params); if (err < 0) { __snd_pcm_unlock(pcm); return err; } - pcm->tstamp_mode = params->tstamp_mode; - pcm->tstamp_type = params->tstamp_type; - pcm->period_step = params->period_step; - pcm->avail_min = params->avail_min; - pcm->period_event = sw_get_period_event(params); - pcm->start_threshold = params->start_threshold; - pcm->stop_threshold = params->stop_threshold; - pcm->silence_threshold = params->silence_threshold; - pcm->silence_size = params->silence_size; - pcm->boundary = params->boundary; __snd_pcm_unlock(pcm); return 0; } diff --git a/src/pcm/pcm_local.h b/src/pcm/pcm_local.h index d52229d..e103f72 100644 --- a/src/pcm/pcm_local.h +++ b/src/pcm/pcm_local.h @@ -661,6 +661,7 @@ static inline int muldiv_near(int a, int b, int c) return n; }
+int _snd_pcm_sw_params_internal(snd_pcm_t *pcm, snd_pcm_sw_params_t *params); int snd_pcm_hw_refine(snd_pcm_t *pcm, snd_pcm_hw_params_t *params); int _snd_pcm_hw_params_internal(snd_pcm_t *pcm, snd_pcm_hw_params_t *params); #undef _snd_pcm_hw_params diff --git a/src/pcm/pcm_params.c b/src/pcm/pcm_params.c index 8826bc3..3ba05fb 100644 --- a/src/pcm/pcm_params.c +++ b/src/pcm/pcm_params.c @@ -2439,3 +2439,24 @@ int _snd_pcm_hw_params_internal(snd_pcm_t *pcm, snd_pcm_hw_params_t *params) return 0; }
+int _snd_pcm_sw_params_internal(snd_pcm_t *pcm, snd_pcm_sw_params_t *params) +{ + int err; + + assert(pcm && params); + assert(pcm->setup); + err = pcm->ops->sw_params(pcm->op_arg, params); + if (err < 0) + return err; + pcm->tstamp_mode = params->tstamp_mode; + pcm->tstamp_type = params->tstamp_type; + pcm->period_step = params->period_step; + pcm->avail_min = params->avail_min; + pcm->period_event = sw_get_period_event(params); + pcm->start_threshold = params->start_threshold; + pcm->stop_threshold = params->stop_threshold; + pcm->silence_threshold = params->silence_threshold; + pcm->silence_size = params->silence_size; + pcm->boundary = params->boundary; + return 0; +}
On Tue, 30 Apr 2019 09:38:42 +0200, vanitha.channaiah@in.bosch.com wrote:
From: Vanitha Channaiah vanitha.channaiah@in.bosch.com
snd_pcm_sw_params() is reformatted by using _snd_pcm_sw_params_internal() function
Signed-off-by: Vanitha Channaiah vanitha.channaiah@in.bosch.com
I see no reason to do that. Please describe.
thanks,
Takashi
From: Vanitha Channaiah vanitha.channaiah@in.bosch.com
Issue: After partial read of unaligned frames, snd_pcm_wait() would block for the pcm->avail_min which would result in blocking of snd_pcm_wait() for longer periods i.e more than one period as specified by avail_min
Fix: After reading unaligned frames, set the pcm->avail_min to the needed available frames so that snd_pcm_wait() blocks till needed available frames. Once needed available frames are read, set back the original pcm->avail_min
Signed-off-by: Vanitha Channaiah vanitha.channaiah@in.bosch.com --- src/pcm/pcm.c | 21 +++++++++++++++++++++ src/pcm/pcm_local.h | 2 ++ 2 files changed, 23 insertions(+)
diff --git a/src/pcm/pcm.c b/src/pcm/pcm.c index f0db545..f361eb1 100644 --- a/src/pcm/pcm.c +++ b/src/pcm/pcm.c @@ -973,6 +973,7 @@ int snd_pcm_sw_params(snd_pcm_t *pcm, snd_pcm_sw_params_t *params) __snd_pcm_unlock(pcm); return err; } + pcm->original_avail_min = pcm->avail_min; __snd_pcm_unlock(pcm); return 0; } @@ -7267,6 +7268,17 @@ void snd_pcm_areas_from_bufs(snd_pcm_t *pcm, snd_pcm_channel_area_t *areas, snd_pcm_unlock(pcm); }
+static void snd_pcm_set_avail_min(snd_pcm_t *pcm, snd_pcm_uframes_t avail) +{ + if (avail != pcm->avail_min) { + snd_pcm_sw_params_t swparams; + + snd_pcm_sw_params_current(pcm, &swparams); + swparams.avail_min = avail; + _snd_pcm_sw_params_internal(pcm, &swparams); + } +} + snd_pcm_sframes_t snd_pcm_read_areas(snd_pcm_t *pcm, const snd_pcm_channel_area_t *areas, snd_pcm_uframes_t offset, snd_pcm_uframes_t size, snd_pcm_xfer_areas_func_t func) @@ -7274,6 +7286,7 @@ snd_pcm_sframes_t snd_pcm_read_areas(snd_pcm_t *pcm, const snd_pcm_channel_area_ snd_pcm_uframes_t xfer = 0; snd_pcm_sframes_t err = 0; snd_pcm_state_t state; + snd_pcm_uframes_t needed_slave_avail_min = 0;
if (size == 0) return 0; @@ -7332,6 +7345,14 @@ snd_pcm_sframes_t snd_pcm_read_areas(snd_pcm_t *pcm, const snd_pcm_channel_area_ if (err < 0) break; frames = err; + pcm->unaligned_frames += frames; + pcm->unaligned_frames %= pcm->period_size; + if (pcm->unaligned_frames) { + needed_slave_avail_min = pcm->period_size - pcm->unaligned_frames; + snd_pcm_set_avail_min(pcm, needed_slave_avail_min); + } else { + snd_pcm_set_avail_min(pcm, pcm->original_avail_min); + } offset += frames; size -= frames; xfer += frames; diff --git a/src/pcm/pcm_local.h b/src/pcm/pcm_local.h index e103f72..3fdffb4 100644 --- a/src/pcm/pcm_local.h +++ b/src/pcm/pcm_local.h @@ -210,6 +210,8 @@ struct _snd_pcm { snd_pcm_tstamp_type_t tstamp_type; /* timestamp type */ unsigned int period_step; snd_pcm_uframes_t avail_min; /* min avail frames for wakeup */ + snd_pcm_uframes_t unaligned_frames; + snd_pcm_uframes_t original_avail_min; int period_event; snd_pcm_uframes_t start_threshold; snd_pcm_uframes_t stop_threshold;
On Tue, 30 Apr 2019 09:38:43 +0200, vanitha.channaiah@in.bosch.com wrote:
From: Vanitha Channaiah vanitha.channaiah@in.bosch.com
Issue: After partial read of unaligned frames, snd_pcm_wait() would block for the pcm->avail_min which would result in blocking of snd_pcm_wait() for longer periods i.e more than one period as specified by avail_min
Fix: After reading unaligned frames, set the pcm->avail_min to the needed available frames so that snd_pcm_wait() blocks till needed available frames. Once needed available frames are read, set back the original pcm->avail_min
Signed-off-by: Vanitha Channaiah vanitha.channaiah@in.bosch.com
This kind of changes in the core code should be avoided as much as possible, especially if it's only relevant with the specific plugins.
Sorry, this isn't convincing enough. If this is a MUST, please clarify better.
thanks,
Takashi
On Tue, 30 Apr 2019 09:38:39 +0200, vanitha.channaiah@in.bosch.com wrote:
From: Vanitha Channaiah vanitha.channaiah@in.bosch.com
This patch is same as the fix in below commit commit 6b058fda9dce ("pcm: dmix: Add option to allow alignment of slave pointers")
The patch description needs rephrasing. What actually this does is to move the code from pcm_dmix.c to pcm_direct.c and its header so that the helper code can be re-used by other direct-PCM plugins.
thanks,
Takashi
participants (2)
-
Takashi Iwai
-
vanitha.channaiah@in.bosch.com