diff mbox series

pcm: dmix: fix incorrect boundary handling in snd_pcm_dmix_sync_area

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

Commit Message

Maarten Baert May 15, 2020, 4:24 p.m. UTC
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(-)

Comments

Takashi Iwai May 19, 2020, 1:32 p.m. UTC | #1
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 mbox series

Patch

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 */