From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=49540 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1P0o8X-0004Na-F0 for qemu-devel@nongnu.org; Wed, 29 Sep 2010 00:19:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1P0o86-0003ad-Mc for qemu-devel@nongnu.org; Wed, 29 Sep 2010 00:19:21 -0400 Received: from mga01.intel.com ([192.55.52.88]:57821) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1P0o86-0003aI-EF for qemu-devel@nongnu.org; Wed, 29 Sep 2010 00:18:54 -0400 Date: Wed, 29 Sep 2010 12:18:41 +0800 From: Wu Fengguang Message-ID: <20100929041841.GA16868@localhost> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Subject: [Qemu-devel] [PATCH] pulse-audio: fix bug on updating rpos List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: malc Cc: "Anvin, H Peter" , qemu-devel@nongnu.org Fix a rpos coordination bug between qpa_run_out() and qpa_thread_out(), which shows up as playback noises. qpa_run_out() qpa_thread_out loop N critical section 1 qpa_run_out() qpa_thread_out loop N doing pa_simple_write() qpa_run_out() qpa_thread_out loop N doing pa_simple_write() qpa_thread_out loop N critical section 2 qpa_thread_out loop N+1 critical section 1 qpa_run_out() qpa_thread_out loop N+1 doing pa_simple_write() In the above scheme, "qpa_thread_out loop N+1 critical section 1" will get the same rpos as the one used by "qpa_thread_out loop N critical section 1". So it will be reading dead samples from the old rpos. The rpos can only be updated back to qpa_thread_out when there is a qpa_run_out() run between two qpa_thread_out loops. normal sequence: qpa_thread_out: hw->rpos (X0) => local rpos => pa->rpos (X1) qpa_run_out: pa->rpos (X1) => hw->rpos (X1) qpa_thread_out: hw->rpos (X1) => local rpos => pa->rpos (X2) buggy sequence: qpa_thread_out: hw->rpos (X0) => local rpos => pa->rpos (X1) qpa_thread_out: hw->rpos (X0) => local rpos => pa->rpos (X1') Obviously qpa_run_out() shall be called at least once between any two qpa_thread_out loops (after pa->rpos is set), in order for the new qpa_thread_out loop to see the updated rpos. Setting pa->live to 0 does the trick. The next loop will have to wait for one qpa_run_out() invocation in order to get a non-zero pa->live and proceed. Signed-off-by: malc Signed-off-by: Wu Fengguang --- audio/paaudio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- qemu-kvm.orig/audio/paaudio.c 2010-09-29 10:28:25.000000000 +0800 +++ qemu-kvm/audio/paaudio.c 2010-09-29 12:11:45.000000000 +0800 @@ -111,8 +111,8 @@ static void *qpa_thread_out (void *arg) return NULL; } + pa->live = 0; pa->rpos = rpos; - pa->live -= decr; pa->decr += decr; }