Message ID | 1580976781-6642-1-git-send-email-vanitha.channaiah@in.bosch.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [-,1/1] pcm: direct: Round down of slave_hw_ptr when buffer size is two period size | expand |
Hello Takashi-san, This patch is regarding the fix for rounding down/up of slave pointers. For buffer_size >= 2*period_size, round down of slave pointers and for buffer_size < 2*period_size, round up of slave pointers will avoid xruns. which otherwise causes snd_pcm_wait() to block for more than expected snd_pcm_period_elapsed() which leads to xruns. We had similar discussion for same issue in below link: https://mailman.alsa-project.org/pipermail/alsa-devel/2019-June/151169.html Regards, Vanitha -----Original Message----- From: Channaiah Vanitha (RBEI/ECF3) <Vanitha.Channaiah@in.bosch.com> Sent: Thursday, February 6, 2020 1:43 PM To: Channaiah Vanitha (RBEI/ECF3) <Vanitha.Channaiah@in.bosch.com>; patch@alsa-project.org Cc: alsa-devel@alsa-project.org; Wischer Timo (ADITG/ESS) <twischer@de.adit-jv.com> Subject: [PATCH - 1/1] pcm: direct: Round down of slave_hw_ptr when buffer size is two period size From: Vanitha Channaiah <vanitha.channaiah@in.bosch.com> For buffer size equal to two period size, the start position of slave_hw_ptr is rounded down in order to avoid xruns For e.g.: Considering below parameters and its values: Period size = 96 (0x60) Buffer size = 192 (0xC0) Timer ticks = 1 avail_min = 0x60 slave_hw_ptr = unaligned value during dmix_start() Issue: - Initial, app_ptr = hw_ptr = 0 - Application fills buffer size of data. so app_ptr = 0xC0, hw_ptr = 0 - During dmix_start(), current slave_hw_ptr is not rounded down. current slave_hw_ptr would be 0x66 - slave_hw_ptr is keep on updating at the hardware 0x67, 0x68, 0x69 - The diff calculation between old_slave_hw_ptr(0x66) and new_slave_hw_ptr(0x69) results in avail = 0x6 - After 1 snd_pcm_period_elapsed(), slave_hw_ptr = 0xC0 - The result of avail = 0x5A which is less than avail_min(0x60) - Here, xruns will be observed Fix: - Initial, app_ptr = hw_ptr = 0 - Application fills buffer size of data. so app_ptr = 0xC0, hw_ptr = 0 - Round down of slave_hw_ptr during dmix_start() leads to below calculation: - During dmix_start(), slave_hw_ptr rounded to 0x60 (old slave_hw_ptr) - The diff calculation between old_slave_hw_ptr(0x60) and new_slave_hw_ptr(0x69) results in avail = 0x9 - After 1 snd_pcm_period_elapsed(), slave_hw_ptr = 0xC0 - The result of avail = 0x60 which is avail_min(0x60) - Here, xruns can be avoided Signed-off-by: Vanitha Channaiah <vanitha.channaiah@in.bosch.com> diff --git a/src/pcm/pcm_direct.c b/src/pcm/pcm_direct.c index 54d9900..a201fa3 100644 --- a/src/pcm/pcm_direct.c +++ b/src/pcm/pcm_direct.c @@ -2043,10 +2043,14 @@ 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) { + /* + * For buffer size equal to two period size, slave_hw_ptr is rounded down + * 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; -- 2.7.4
On Thu, 06 Feb 2020 09:14:42 +0100, Channaiah Vanitha (RBEI/ECF3) wrote: > > Hello Takashi-san, > > This patch is regarding the fix for rounding down/up of slave pointers. > For buffer_size >= 2*period_size, round down of slave pointers and > for buffer_size < 2*period_size, round up of slave pointers will avoid xruns. > which otherwise causes snd_pcm_wait() to block for more than expected snd_pcm_period_elapsed() which leads to xruns. > > We had similar discussion for same issue in below link: > https://mailman.alsa-project.org/pipermail/alsa-devel/2019-June/151169.html Yes, and I still don't get why this must be required. Doesn't this imply that you drop the samples instead? Takashi > > Regards, > Vanitha > > > -----Original Message----- > From: Channaiah Vanitha (RBEI/ECF3) <Vanitha.Channaiah@in.bosch.com> > Sent: Thursday, February 6, 2020 1:43 PM > To: Channaiah Vanitha (RBEI/ECF3) <Vanitha.Channaiah@in.bosch.com>; patch@alsa-project.org > Cc: alsa-devel@alsa-project.org; Wischer Timo (ADITG/ESS) <twischer@de.adit-jv.com> > Subject: [PATCH - 1/1] pcm: direct: Round down of slave_hw_ptr when buffer size is two period size > > From: Vanitha Channaiah <vanitha.channaiah@in.bosch.com> > > For buffer size equal to two period size, the start position of slave_hw_ptr is rounded down in order to avoid xruns > > For e.g.: > Considering below parameters and its values: > Period size = 96 (0x60) > Buffer size = 192 (0xC0) > Timer ticks = 1 > avail_min = 0x60 > slave_hw_ptr = unaligned value during dmix_start() > > Issue: > - Initial, app_ptr = hw_ptr = 0 > - Application fills buffer size of data. so app_ptr = 0xC0, hw_ptr = 0 > - During dmix_start(), current slave_hw_ptr is not rounded down. current slave_hw_ptr would be 0x66 > - slave_hw_ptr is keep on updating at the hardware 0x67, 0x68, 0x69 > - The diff calculation between old_slave_hw_ptr(0x66) and new_slave_hw_ptr(0x69) > results in avail = 0x6 > - After 1 snd_pcm_period_elapsed(), slave_hw_ptr = 0xC0 > - The result of avail = 0x5A which is less than avail_min(0x60) > - Here, xruns will be observed > > Fix: > - Initial, app_ptr = hw_ptr = 0 > - Application fills buffer size of data. so app_ptr = 0xC0, hw_ptr = 0 > - Round down of slave_hw_ptr during dmix_start() leads to below calculation: > - During dmix_start(), slave_hw_ptr rounded to 0x60 (old slave_hw_ptr) > - The diff calculation between old_slave_hw_ptr(0x60) and new_slave_hw_ptr(0x69) > results in avail = 0x9 > - After 1 snd_pcm_period_elapsed(), slave_hw_ptr = 0xC0 > - The result of avail = 0x60 which is avail_min(0x60) > - Here, xruns can be avoided > > Signed-off-by: Vanitha Channaiah <vanitha.channaiah@in.bosch.com> > > diff --git a/src/pcm/pcm_direct.c b/src/pcm/pcm_direct.c index 54d9900..a201fa3 100644 > --- a/src/pcm/pcm_direct.c > +++ b/src/pcm/pcm_direct.c > @@ -2043,10 +2043,14 @@ 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) { > + /* > + * For buffer size equal to two period size, slave_hw_ptr is rounded down > + * 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; > -- > 2.7.4 > > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@alsa-project.org > https://mailman.alsa-project.org/mailman/listinfo/alsa-devel >
On Thu, 06 Feb 2020 09:44:58 +0100, Takashi Iwai wrote: > > On Thu, 06 Feb 2020 09:14:42 +0100, > Channaiah Vanitha (RBEI/ECF3) wrote: > > > > Hello Takashi-san, > > > > This patch is regarding the fix for rounding down/up of slave pointers. > > For buffer_size >= 2*period_size, round down of slave pointers and > > for buffer_size < 2*period_size, round up of slave pointers will avoid xruns. > > which otherwise causes snd_pcm_wait() to block for more than expected snd_pcm_period_elapsed() which leads to xruns. > > > > We had similar discussion for same issue in below link: > > https://mailman.alsa-project.org/pipermail/alsa-devel/2019-June/151169.html > > Yes, and I still don't get why this must be required. > Doesn't this imply that you drop the samples instead? Actually, in such an extreme situation, there is no perfect solution with dmix. That's the reason we introduced the explicit hw_ptr_alignment option to specify the behavior. The default value is to keep the current behavior for compatibility; i.e. unless you are 100% sure that this change won't break any existing usage, there is no big reason to change the behavior, too. thanks, Takashi > > > Takashi > > > > > Regards, > > Vanitha > > > > > > -----Original Message----- > > From: Channaiah Vanitha (RBEI/ECF3) <Vanitha.Channaiah@in.bosch.com> > > Sent: Thursday, February 6, 2020 1:43 PM > > To: Channaiah Vanitha (RBEI/ECF3) <Vanitha.Channaiah@in.bosch.com>; patch@alsa-project.org > > Cc: alsa-devel@alsa-project.org; Wischer Timo (ADITG/ESS) <twischer@de.adit-jv.com> > > Subject: [PATCH - 1/1] pcm: direct: Round down of slave_hw_ptr when buffer size is two period size > > > > From: Vanitha Channaiah <vanitha.channaiah@in.bosch.com> > > > > For buffer size equal to two period size, the start position of slave_hw_ptr is rounded down in order to avoid xruns > > > > For e.g.: > > Considering below parameters and its values: > > Period size = 96 (0x60) > > Buffer size = 192 (0xC0) > > Timer ticks = 1 > > avail_min = 0x60 > > slave_hw_ptr = unaligned value during dmix_start() > > > > Issue: > > - Initial, app_ptr = hw_ptr = 0 > > - Application fills buffer size of data. so app_ptr = 0xC0, hw_ptr = 0 > > - During dmix_start(), current slave_hw_ptr is not rounded down. current slave_hw_ptr would be 0x66 > > - slave_hw_ptr is keep on updating at the hardware 0x67, 0x68, 0x69 > > - The diff calculation between old_slave_hw_ptr(0x66) and new_slave_hw_ptr(0x69) > > results in avail = 0x6 > > - After 1 snd_pcm_period_elapsed(), slave_hw_ptr = 0xC0 > > - The result of avail = 0x5A which is less than avail_min(0x60) > > - Here, xruns will be observed > > > > Fix: > > - Initial, app_ptr = hw_ptr = 0 > > - Application fills buffer size of data. so app_ptr = 0xC0, hw_ptr = 0 > > - Round down of slave_hw_ptr during dmix_start() leads to below calculation: > > - During dmix_start(), slave_hw_ptr rounded to 0x60 (old slave_hw_ptr) > > - The diff calculation between old_slave_hw_ptr(0x60) and new_slave_hw_ptr(0x69) > > results in avail = 0x9 > > - After 1 snd_pcm_period_elapsed(), slave_hw_ptr = 0xC0 > > - The result of avail = 0x60 which is avail_min(0x60) > > - Here, xruns can be avoided > > > > Signed-off-by: Vanitha Channaiah <vanitha.channaiah@in.bosch.com> > > > > diff --git a/src/pcm/pcm_direct.c b/src/pcm/pcm_direct.c index 54d9900..a201fa3 100644 > > --- a/src/pcm/pcm_direct.c > > +++ b/src/pcm/pcm_direct.c > > @@ -2043,10 +2043,14 @@ 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) { > > + /* > > + * For buffer size equal to two period size, slave_hw_ptr is rounded down > > + * 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; > > -- > > 2.7.4 > > > > _______________________________________________ > > Alsa-devel mailing list > > Alsa-devel@alsa-project.org > > https://mailman.alsa-project.org/mailman/listinfo/alsa-devel > > > _______________________________________________ > Patch mailing list > Patch@alsa-project.org > https://mailman.alsa-project.org/mailman/listinfo/patch >
diff --git a/src/pcm/pcm_direct.c b/src/pcm/pcm_direct.c index 54d9900..a201fa3 100644 --- a/src/pcm/pcm_direct.c +++ b/src/pcm/pcm_direct.c @@ -2043,10 +2043,14 @@ 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) { + /* + * For buffer size equal to two period size, slave_hw_ptr is rounded down + * 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;