Message ID | HE1P191MB0153D35FC0B82F7F6D2E409AE2BD0@HE1P191MB0153.EURP191.PROD.OUTLOOK.COM (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | pcm: dmix: fix incorrect boundary handling in snd_pcm_dmix_sync_area | expand |
On Fri, 15 May 2020 18:24:08 +0200, Maarten Baert wrote: > > The available size calculation did not handle wraparound correctly. Also, the > underrun handling code confused slave_buffer_size with slave_boundary, causing > another integer overflow that results in too many samples being skipped. > > Signed-off-by: Maarten Baert <maarten-baert@hotmail.com> Thanks, the code change looks correct but the embedded patch isn't applicable. Indeed the patch seems somehow malformed. Could you repost the proper one, at best with git-send-email? thanks, Takashi > --- > src/pcm/pcm_dmix.c | 16 +++++++++------- > 1 file changed, 9 insertions(+), 7 deletions(-) > > diff --git a/src/pcm/pcm_dmix.c b/src/pcm/pcm_dmix.c > index 843fa316..b5fdb240 100644 > --- a/src/pcm/pcm_dmix.c > +++ b/src/pcm/pcm_dmix.c > @@ -315,11 +315,12 @@ static void snd_pcm_dmix_sync_area(snd_pcm_t *pcm) > /* check the available size in the local buffer > * last_appl_ptr keeps the last updated position > */ > - size = dmix->appl_ptr - dmix->last_appl_ptr; > + if (dmix->appl_ptr >= dmix->last_appl_ptr) > + size = dmix->appl_ptr - dmix->last_appl_ptr; > + else > + size = dmix->appl_ptr + (pcm->boundary - dmix->last_appl_ptr); > if (! size) > return; > - if (size >= pcm->boundary / 2) > - size = pcm->boundary - size; > /* the slave_app_ptr can be far behind the slave_hw_ptr */ > /* reduce mixing and errors here - just skip not catched writes */ > @@ -328,18 +329,19 @@ static void snd_pcm_dmix_sync_area(snd_pcm_t *pcm) > else > slave_size = dmix->slave_appl_ptr + (dmix->slave_boundary - dmix->slave_hw_ptr); > if (slave_size > dmix->slave_buffer_size) { > - transfer = dmix->slave_buffer_size - slave_size; > + transfer = dmix->slave_boundary - slave_size; > if (transfer > size) > transfer = size; > dmix->last_appl_ptr += transfer; > dmix->last_appl_ptr %= pcm->boundary; > dmix->slave_appl_ptr += transfer; > dmix->slave_appl_ptr %= dmix->slave_boundary; > - size = dmix->appl_ptr - dmix->last_appl_ptr; > + if (dmix->appl_ptr >= dmix->last_appl_ptr) > + size = dmix->appl_ptr - dmix->last_appl_ptr; > + else > + size = dmix->appl_ptr + (pcm->boundary - dmix->last_appl_ptr); > if (! size) > return; > - if (size >= pcm->boundary / 2) > - size = pcm->boundary - size; > } > /* check the available size in the slave PCM buffer */ > -- > 2.26.2 >
diff --git a/src/pcm/pcm_dmix.c b/src/pcm/pcm_dmix.c index 843fa316..b5fdb240 100644 --- a/src/pcm/pcm_dmix.c +++ b/src/pcm/pcm_dmix.c @@ -315,11 +315,12 @@ static void snd_pcm_dmix_sync_area(snd_pcm_t *pcm) /* check the available size in the local buffer * last_appl_ptr keeps the last updated position */ - size = dmix->appl_ptr - dmix->last_appl_ptr; + if (dmix->appl_ptr >= dmix->last_appl_ptr) + size = dmix->appl_ptr - dmix->last_appl_ptr; + else + size = dmix->appl_ptr + (pcm->boundary - dmix->last_appl_ptr); if (! size) return; - if (size >= pcm->boundary / 2) - size = pcm->boundary - size; /* the slave_app_ptr can be far behind the slave_hw_ptr */ /* reduce mixing and errors here - just skip not catched writes */ @@ -328,18 +329,19 @@ static void snd_pcm_dmix_sync_area(snd_pcm_t *pcm) else slave_size = dmix->slave_appl_ptr + (dmix->slave_boundary - dmix->slave_hw_ptr); if (slave_size > dmix->slave_buffer_size) { - transfer = dmix->slave_buffer_size - slave_size; + transfer = dmix->slave_boundary - slave_size; if (transfer > size) transfer = size; dmix->last_appl_ptr += transfer; dmix->last_appl_ptr %= pcm->boundary; dmix->slave_appl_ptr += transfer; dmix->slave_appl_ptr %= dmix->slave_boundary; - size = dmix->appl_ptr - dmix->last_appl_ptr; + if (dmix->appl_ptr >= dmix->last_appl_ptr) + size = dmix->appl_ptr - dmix->last_appl_ptr; + else + size = dmix->appl_ptr + (pcm->boundary - dmix->last_appl_ptr); if (! size) return; - if (size >= pcm->boundary / 2) - size = pcm->boundary - size; } /* check the available size in the slave PCM buffer */
The available size calculation did not handle wraparound correctly. Also, the underrun handling code confused slave_buffer_size with slave_boundary, causing another integer overflow that results in too many samples being skipped. Signed-off-by: Maarten Baert <maarten-baert@hotmail.com> --- src/pcm/pcm_dmix.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-)