From: Mark Wooding <mdw@distorted.org.uk>
Date: Sun, 19 May 2013 02:48:25 +0000 (+0100)
Subject: server/speaker.c: Only copy whole frames into collection buffers.
X-Git-Tag: 5.1.1~7
X-Git-Url: http://www.chiark.greenend.org.uk/ucgi/~mdw/git/disorder/commitdiff_plain/05577e6c12009f025c1a0e8a4aec956e6fa914f6

server/speaker.c: Only copy whole frames into collection buffers.

At least the ALSA playback function gets into a real mess if a buffer
contains a partial frame: it leaves the partial frame behind, but is
called again until the buffer is empty, resulting in an infinite loop.

Thanks for Joe Birr-Pixton for helping diagnose this bug and coming up
with the right fix.
---

diff --git a/server/speaker.c b/server/speaker.c
index 212ffb9..1d26c6c 100644
--- a/server/speaker.c
+++ b/server/speaker.c
@@ -418,9 +418,13 @@ static int addfd(int fd, int events) {
 static size_t speaker_callback(void *buffer,
                                size_t max_samples,
                                void attribute((unused)) *userdata) {
-  const size_t max_bytes = max_samples * uaudio_sample_size;
+  size_t max_bytes = max_samples * uaudio_sample_size;
   size_t provided_samples = 0;
 
+  /* Be sure to keep the amount of data in a buffer a whole number of frames:
+   * otherwise the playing threads can become stuck. */
+  max_bytes -= max_bytes % (uaudio_sample_size * uaudio_channels);
+
   pthread_mutex_lock(&lock);
   /* TODO perhaps we should immediately go silent if we've been asked to pause
    * or cancel the playing track (maybe block in the cancel case and see what
@@ -437,6 +441,8 @@ static size_t speaker_callback(void *buffer,
       /* Limit to what we were asked for */
       if(bytes > max_bytes)
         bytes = max_bytes;
+      /* And truncate to a whole number of frames. */
+      bytes -= bytes % (uaudio_sample_size * uaudio_channels);
       /* Provide it */
       memcpy(buffer, playing->buffer + playing->start, bytes);
       playing->start += bytes;