Re: [alsa-devel] [PATCH - alsa-lib] pcm/ioplug: Add mutex in snd_pcm_ioplug_hw_ptr_update

Hi Takashi,

Thanks for the reply. I saw the race between "delay" and "avail_update" function.

From GStreamer stack these two function gets called from different threads.

Caller is calling two different function here it doesn't know about race condition. And this race condition doesn't happen with HW plug-in, Its only with ioplug plug-in.

Thanks, Vishal Agrawal

On Tue, Oct 29, 2013 at 4:12 PM, Takashi Iwai tiwai@suse.de wrote:

At Tue, 29 Oct 2013 15:52:16 +0530, Vishal Agrawal wrote:

...

Function snd_pcm_ioplug_hw_ptr_update can be called from many threads, we need to protect the hw_ptr. I came across this issue with GStreamer and Jackd. Function will be called from the thread doing snd_pcm_write and another would be to get the playback position for the track bar.

Change in itself is very minimum, but it took me a lot of time to debug.

We'd like to avoid mutex in the alsa-lib code as much as possible. In which code paths did you get the race? In general, the code paths involved with hw_ptr_update() are known to be thread-unsafe, so it's the responsibility of the caller side to protect the race.

thanks,

Takashi

...

Thanks, Vishal Agrawal [1.2 <text/html; ISO-8859-1 (quoted-printable)>]

[2 0001-pcm-ioplug-Add-mutex-in-snd_pcm_ioplug_hw_ptr_update.patch

<text/x-patch; US-ASCII (base64)>]

...

From b826b1f28c50bd4a414cf50751be4ac74b4e4174 Mon Sep 17 00:00:00 2001 From: Vishal Agrawal visagrawal@gmail.com Date: Mon, 28 Oct 2013 18:01:14 +0530 Subject: [PATCH - alsa-lib] pcm/ioplug: Add mutex in snd_pcm_ioplug_hw_ptr_update

snd_pcm_ioplug_hw_ptr_update() can be called from different threads, it needs to be protected by mutex

Signed-off-by: Vishal Agrawal visagrawal@gmail.com

diff --git a/src/pcm/pcm_ioplug.c b/src/pcm/pcm_ioplug.c index a90c844..23dbee3 100644 --- a/src/pcm/pcm_ioplug.c +++ b/src/pcm/pcm_ioplug.c @@ -26,6 +26,7 @@

*/

+#include <pthread.h> #include "pcm_local.h" #include "pcm_ioplug.h" #include "pcm_ext_parm.h" @@ -47,12 +48,15 @@ typedef struct snd_pcm_ioplug_priv { snd_htimestamp_t trigger_tstamp; } ioplug_priv_t;

+static pthread_mutex_t hw_ptr_mutex = PTHREAD_MUTEX_INITIALIZER;

/* update the hw pointer */ static void snd_pcm_ioplug_hw_ptr_update(snd_pcm_t *pcm) { ioplug_priv_t *io = pcm->private_data; snd_pcm_sframes_t hw;

• pthread_mutex_lock(&hw_ptr_mutex);
  hw = io->data->callback->pointer(io->data);
  if (hw >= 0) {
          unsigned int delta;
  

@@ -64,6 +68,7 @@ static void snd_pcm_ioplug_hw_ptr_update(snd_pcm_t

*pcm)

...

          io->last_hw = hw;
  } else
          io->data->state = SNDRV_PCM_STATE_XRUN;

• pthread_mutex_unlock(&hw_ptr_mutex);
  

}

static int snd_pcm_ioplug_info(snd_pcm_t *pcm, snd_pcm_info_t *info)

1.7.9.5