[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#247905: Arts has bad interactivity with alsa + dmix in kde 3.2.2



Package: libarts1
Version: 1.2.2
Severity: important
Tags: patch

When playing audio trough arts the reaction time of artsd can
increase to around 1s if the alsa output plugin is used together with
dmix.

Attached is a fix from Allen Sandfeld. This fix is already in kde cvs.
The patch must be applied in the arts-1.2.2/flow directory and does not
interfere with arts_1.2.2-1.diff.gz.

Full information on the bug is available on
http://bugs.kde.org/show_bug.cgi?id=81095

-- 
lg, Chris
Index: audioioalsa9.cc
===================================================================
RCS file: /home/kde/arts/flow/audioioalsa9.cc,v
retrieving revision 1.4.2.3
diff -u -3 -p -r1.4.2.3 audioioalsa9.cc
--- audioioalsa9.cc	2 Apr 2004 11:21:49 -0000	1.4.2.3
+++ audioioalsa9.cc	7 May 2004 19:29:35 -0000
@@ -63,20 +63,18 @@ namespace Arts {
 
 class AudioIOALSA : public AudioIO, public IONotify  {
 protected:
-	int audio_read_fd;
-	int audio_write_fd;
+	struct pollfd m_audio_read_pfd, m_audio_write_pfd;
 
         snd_pcm_t *m_pcm_playback;
 	snd_pcm_t *m_pcm_capture;
 	snd_pcm_format_t m_format;
 	int m_period_size, m_periods;
-        bool inProgress;
-	bool restartIOHandling;
 
         void startIO();
-        int poll2iomanager(int pollTypes);
+        static int poll2iomanager(int pollTypes);
+        static int iomanager2poll(int pollTypes);
 	int setPcmParams(snd_pcm_t *pcm);
-	int watchDescriptor(snd_pcm_t *pcm);
+	void watchDescriptor(snd_pcm_t *pcm, struct pollfd *pfd);
 
         void notifyIO(int fd, int types);
 
@@ -118,9 +116,6 @@ AudioIOALSA::AudioIOALSA()
 	m_format = SND_PCM_FORMAT_S16_LE;
         m_pcm_playback = NULL;
 	m_pcm_capture = NULL;
-        inProgress = false;
-        restartIOHandling = false;
-	audio_read_fd = audio_write_fd = -1;
 }
 
 bool AudioIOALSA::open()
@@ -309,10 +304,10 @@ void AudioIOALSA::startIO()
 {
         /* watch PCM file descriptor(s) */
 	if (m_pcm_playback) {
-		audio_write_fd = watchDescriptor(m_pcm_playback);
+		watchDescriptor(m_pcm_playback, &m_audio_write_pfd);
         }
 	if (m_pcm_capture) {
-		audio_read_fd = watchDescriptor(m_pcm_capture);
+		watchDescriptor(m_pcm_capture, &m_audio_read_pfd);
         }
 
 }
@@ -328,28 +323,40 @@ int AudioIOALSA::poll2iomanager(int poll
 	if(pollTypes & POLLERR)
 		types |= IOType::except;
 
-	return types | IOType::reentrant;
+	return types;
 }
 
-int AudioIOALSA::watchDescriptor(snd_pcm_t *pcm)
+int AudioIOALSA::iomanager2poll(int pollTypes)
+{
+	int types = 0;
+
+	if(pollTypes & IOType::read)
+		types |= POLLIN;
+	if(pollTypes & IOType::write)
+		types |= POLLOUT;
+	if(pollTypes & IOType::except)
+		types |= POLLERR;
+
+	return types;
+}
+
+void AudioIOALSA::watchDescriptor(snd_pcm_t *pcm, struct pollfd *pfd)
 {
-        struct pollfd pfds;
         if (snd_pcm_poll_descriptors_count(pcm) != 1) {
-		arts_info("Can't handle more than one poll descriptor\n");
-		return -1;
+		arts_fatal("Can't handle more than one poll descriptor\n");
+		return;
         }
-        if (snd_pcm_poll_descriptors(pcm, &pfds, 1) != 1) {
-		arts_info("Cannot get poll descriptor\n");
-		return -1;
+        if (snd_pcm_poll_descriptors(pcm, pfd, 1) != 1) {
+		arts_fatal("Cannot get poll descriptor\n");
+		return;
 	}
 
         // See though the crack-fumes from the ALSA-developers and try to
         // figure out which way this handle is supposed to be polled.
-        int types = poll2iomanager(pfds.events);
+        int types = poll2iomanager(pfd->events);
 
-        Dispatcher::the()->ioManager()->watchFD(pfds.fd, types, this);
+        Dispatcher::the()->ioManager()->watchFD(pfd->fd, types, this);
 
-	return pfds.fd;
 }
 
 int AudioIOALSA::xrun(snd_pcm_t *pcm)
@@ -399,9 +406,6 @@ int AudioIOALSA::read(void *buffer, int 
 
 int AudioIOALSA::write(void *buffer, int size)
 {
-        // DMix has an annoying habit of returning instantantly on the returned
-        // poll-descriptor. So we block here to avoid an infinity loop.
-        snd_pcm_wait(m_pcm_playback, 5);
 
         int frames = snd_pcm_bytes_to_frames(m_pcm_playback, size);
 	int length;
@@ -428,29 +432,28 @@ void AudioIOALSA::notifyIO(int fd, int t
 {
         int todo = 0;
 
-	if(inProgress)
-	{
-		if(!restartIOHandling)
-		{
-			Dispatcher::the()->ioManager()->remove(this,IOType::all);
-			restartIOHandling = true;
-		}
-		return;
+	// Call pcm_poll_desciptors_revents to translate the event safely for us.
+	// This is necassary even if we know what the event is, because _revents
+	// might have side-effects (dmix have)
+	unsigned short revents;
+        if(fd == m_audio_write_pfd.fd) {
+		m_audio_write_pfd.revents = iomanager2poll(type);
+		snd_pcm_poll_descriptors_revents(m_pcm_playback, &m_audio_write_pfd, 1, &revents);
+		if (revents & POLLOUT)
+ 			todo |= AudioSubSystem::ioWrite;
+
+	}
+        if(fd == m_audio_read_pfd.fd) {
+		m_audio_read_pfd.revents = iomanager2poll(type);
+		snd_pcm_poll_descriptors_revents(m_pcm_capture, &m_audio_read_pfd, 1, &revents);
+		if (revents & POLLIN)
+ 			todo |= AudioSubSystem::ioRead;
 	}
 
-	// We can't trust the type as ALSA might have read-type events,
-	// that are really meant to be write-type event!
-        if(fd == audio_write_fd) todo |= AudioSubSystem::ioWrite;
-        if(fd == audio_read_fd) todo |= AudioSubSystem::ioRead;
-
         if (type & IOType::except) todo |= AudioSubSystem::ioExcept;
 
-        restartIOHandling = false;
-        inProgress = true;
-        AudioSubSystem::the()->handleIO(todo);
-        inProgress = false;
+        if (todo) AudioSubSystem::the()->handleIO(todo);
 
-        if (restartIOHandling) startIO();
 }
 
 int AudioIOALSA::setPcmParams(snd_pcm_t *pcm)
Index: audiosubsys.cc
===================================================================
RCS file: /home/kde/arts/flow/audiosubsys.cc,v
retrieving revision 1.46
diff -u -3 -p -r1.46 audiosubsys.cc
--- audiosubsys.cc	29 Aug 2003 21:02:06 -0000	1.46
+++ audiosubsys.cc	7 May 2004 19:29:36 -0000
@@ -7,12 +7,12 @@
     modify it under the terms of the GNU Library General Public
     License as published by the Free Software Foundation; either
     version 2 of the License, or (at your option) any later version.
-  
+
     This library is distributed in the hope that it will be useful,
     but WITHOUT ANY WARRANTY; without even the implied warranty of
     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
     Library General Public License for more details.
-   
+
     You should have received a copy of the GNU Library General Public License
     along with this library; see the file COPYING.LIB.  If not, write to
     the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
@@ -100,7 +100,7 @@ AudioSubSystem::AudioSubSystem()
 	d = new AudioSubSystemPrivate;
 	d->audioIO = 0;
 	d->audioIOInit = false;
-	
+
 	_running = false;
 	consumer = 0;
 	producer = 0;
@@ -338,7 +338,7 @@ int AudioSubSystem::selectWriteFD()
 
 bool AudioSubSystem::check()
 {
-	bool ok = open();
+        bool ok = open();
 
 	if(ok) close();
 	return ok;
@@ -353,7 +353,7 @@ bool AudioSubSystem::open()
 	{
 		if(d->audioIOName == "")
 			_error = "couldn't auto detect which audio I/O method to use";
-		else	
+		else
 			_error = "unable to select '"+d->audioIOName+"' style audio I/O";
 		return false;
 	}
@@ -422,6 +422,7 @@ void AudioSubSystem::handleIO(int type)
 		/*
 		 * make sure that we have a fragment full of data at least
 		 */
+Rewrite:
 		while(wBuffer.size() < _fragmentSize)
 		{
 			long wbsz = wBuffer.size();
@@ -477,6 +478,9 @@ void AudioSubSystem::handleIO(int type)
 				}
 			}
 		}
+
+                // If we can write a fragment more, then do so right now:
+                if (space >= _fragmentSize*2) goto Rewrite;
 	}
 
 	assert((type & ioExcept) == 0);

Reply to: