The initial implementation used a separate thread to drive the audio
playback channel. But if you have adaptive streaming turned on,
you will eventually get a update_client_playback_latency message on the
display channel (which in the Xspice case is being driven by the main,
Xorg, thread).
After enough time you would get a thread collision and bad things
would result. I saw this manifest as infinite spin loops in snd_send_data.
This patch eliminates the use of threading altogether, making everything
run in the main Xorg thread using watches and timers, eliminating the
possibility of thread collision.
Signed-off-by: Jeremy White <jwhite@xxxxxxxxxxxxxxx>
---
src/qxl.h | 3 +-
src/spiceqxl_audio.c | 564 +++++++++++++++++++++++++++++++-------------------
2 files changed, 352 insertions(+), 215 deletions(-)
diff --git a/src/qxl.h b/src/qxl.h
index fa9b13f..603faca 100644
--- a/src/qxl.h
+++ b/src/qxl.h
@@ -334,8 +334,6 @@ struct _qxl_screen_t
/* XSpice specific, dragged from the Device */
QXLReleaseInfo *last_release;
- pthread_t audio_thread;
-
uint32_t cmdflags;
uint32_t oom_running;
uint32_t num_free_res; /* is having a release ring effective
@@ -353,6 +351,7 @@ struct _qxl_screen_t
} guest_primary;
char playback_fifo_dir[PATH_MAX];
+ void *playback_opaque;
#endif /* XSPICE */
uint32_t deferred_fps;
diff --git a/src/spiceqxl_audio.c b/src/spiceqxl_audio.c
index 02859ee..9012f27 100644
--- a/src/spiceqxl_audio.c
+++ b/src/spiceqxl_audio.c
@@ -20,6 +20,10 @@
* CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
*/
+/* XSpice based audio feed; reads from files (presumably fifos) in a configured directory,
+ and mixes their raw data on to the spice playback channel. */
+
+
#ifdef HAVE_CONFIG_H
#include "config.h"
#endif
@@ -32,283 +36,375 @@
#include <sys/time.h>
#include <unistd.h>
#include <dirent.h>
+#include <sys/inotify.h>
+
+/* mplayer + pulse will write data to the fifo as fast as we can read it.
+ So we need to pace both how quickly we consume the data and how quickly
+ we feed the data in to Spice. We will read ahead (up to READ_BUFFER_PERIODS),
+ and feed ahead into the Spice server (up to FEED_BUFFER_PERIODS).
+*/
+
+#define PERIOD_MS 10
+#define READ_BUFFER_PERIODS 2
+#define FEED_BUFFER_PERIODS 8
-#define BUFFER_PERIODS 10
-#define PERIOD_MS 10
#define MAX_FIFOS 16
+struct fifo_data {
+ char *buffer;
+ int size;
+ int len;
+ int add_to;
+ int fd;
+ SpiceWatch *watch;
+};
+
struct audio_data {
- int fifo_fds[MAX_FIFOS];
- ino_t inodes[MAX_FIFOS];
- uint32_t valid_bytes, write_offs;
- char *buffer, *spice_buffer;
- int period_frames;
- uint32_t spice_write_offs, spice_buffer_bytes;
- uint32_t frame_bytes, period_bytes, fed, buffer_bytes;
- struct timeval last_read_time;
+ struct fifo_data fifos[MAX_FIFOS];
+ uint32_t *spice_buffer;
+ int spice_buffer_bytes;
+ int period_bytes;
+ struct timeval fed_through_time;
+ int remainder;
+ int fifo_count;
+ SpiceTimer *wall_timer;
+ int wall_timer_live;
+ int dir_watch;
+ int fifo_dir_watch;
+ SpiceWatch *fifo_dir_qxl_watch;
};
-static ssize_t
-read_from_fifos (struct audio_data *data)
+/* We maintain a ring buffer for each file we are reading from;
+ these helper functions faciliate adding data to the buffer,
+ and removing it. */
+static inline void fifo_data_added(struct fifo_data *f, int n)
{
- size_t to_read_bytes = min(data->period_bytes, data->buffer_bytes - data->write_offs);
- int i;
- ssize_t max_readed = 0;
- int16_t *out_buf = (int16_t*)(data->buffer + data->write_offs), *buf;
+ f->add_to = (f->add_to + n) % f->size;
+ f->len += n;
+}
- buf = malloc(to_read_bytes);
- if (!buf)
- {
- ErrorF("playback: malloc failed: %s\n", strerror(errno));
- return 0;
+static inline int fifo_read(struct fifo_data *f)
+{
+ int rc;
+ int len = min(f->size - f->len, f->size - f->add_to);
+ rc = read(f->fd, f->buffer + f->add_to, len);
+ if (rc > 0)
+ fifo_data_added(f, rc);
+
+ if (rc > 0 && rc == len && f->size - f->len > 0) {
+ rc = read(f->fd, f->buffer + f->add_to, f->size - f->len);
+ if (rc > 0)
+ fifo_data_added(f, rc);
}
- memset(out_buf, 0, to_read_bytes);
-
- for (i = 0; i < MAX_FIFOS; ++i)
- {
- unsigned int s;
- ssize_t readed;
-
- if (data->fifo_fds[i] < 0)
- continue;
+ return rc;
+}
- readed = read(data->fifo_fds[i], buf, to_read_bytes);
- if (readed < 0)
- {
- if (errno != EAGAIN && errno != EINTR)
- ErrorF("playback: read from FIFO %d failed: %s\n", data->fifo_fds[i], strerror(errno));
- continue;
- }
+static inline void fifo_remove_data(struct fifo_data *f, unsigned char *dest, int len)
+{
+ int remove_from = f->add_to >= f->len ? f->add_to - f->len : f->add_to + f->size - f->len;
+ int remain = f->size - remove_from;
+
+ if (remain < len) {
+ memcpy(dest, f->buffer + remove_from, remain);
+ dest += remain;
+ len -= remain;
+ f->len -= remain;
+ remove_from = 0;
+ }
- if (readed == 0)
- {
- ErrorF("playback: FIFO %d gave EOF\n", data->fifo_fds[i]);
- close(data->fifo_fds[i]);
- data->fifo_fds[i] = -1;
- data->inodes[i] = 0;
- continue;
- }
+ memcpy(dest, f->buffer + remove_from, len);
+ f->len -= len;
+}
- if (readed > max_readed)
- max_readed = readed;
-
- for (s = 0; s < readed / sizeof(int16_t); ++s)
- {
- /* FIXME: Ehhh, this'd be better as floats. With this algorithm,
- * samples mixed after being clipped will have undue weight. But
- * if we're clipping, then we're distorted anyway, so whatever. */
- if (out_buf[s] + buf[s] > INT16_MAX)
- out_buf[s] = INT16_MAX;
- else if (out_buf[s] + buf[s] < -INT16_MAX)
- out_buf[s] = -INT16_MAX;
- else
- out_buf[s] += buf[s];
- }
+static void mix_in_one_fifo(struct fifo_data *f, int16_t *out, int len)
+{
+ int s;
+ int16_t *in;
+
+ if (len > f->len)
+ len = f->len;
+
+ in = calloc(1, len);
+
+ fifo_remove_data(f, (unsigned char *) in, len);
+
+ for (s = 0; s < (len / sizeof(int16_t)); s++) {
+ /* FIXME: Ehhh, this'd be better as floats. With this algorithm,
+ * samples mixed after being clipped will have undue weight. But
+ * if we're clipping, then we're distorted anyway, so whatever. */
+ if (out[s] + in[s] > INT16_MAX)
+ out[s] = INT16_MAX;
+ else if (out[s] + in[s] < -INT16_MAX)
+ out[s] = -INT16_MAX;
+ else
+ out[s] += in[s];
}
- free(buf);
+ free(in);
+}
- if (!max_readed)
- return 0;
+static void mix_in_fifos(qxl_screen_t *qxl)
+{
+ int i;
+ struct audio_data *data = qxl->playback_opaque;
+ struct fifo_data *f;
- data->valid_bytes = min(data->valid_bytes + max_readed,
- data->buffer_bytes);
+ memset(data->spice_buffer, 0, data->spice_buffer_bytes);
- data->write_offs += max_readed;
- data->write_offs %= data->buffer_bytes;
+ if (data->fifo_count == 0)
+ return;
- ++data->fed;
+ /* First fifo can just be copied */
+ f = &data->fifos[0];
+ fifo_remove_data(f, (unsigned char *) data->spice_buffer, min(data->spice_buffer_bytes, f->len));
- return max_readed;
+ /* Extra fifos need to be mixed in */
+ for (i = 1; i < data->fifo_count; i++) {
+ f = &data->fifos[i];
+ if (f->len > 0)
+ mix_in_one_fifo(f, (int16_t *) data->spice_buffer, data->spice_buffer_bytes);
+ }
}
-static int
-scan_fifos (struct audio_data *data, const char *dirname)
+static int can_feed(struct audio_data *data)
{
- DIR *dir;
- struct dirent *ent;
- int i;
+ struct timeval end, diff;
- dir = opendir(dirname);
- if (!dir)
- {
- ErrorF("playback: failed to open FIFO directory '%s': %s\n", dirname, strerror(errno));
+ gettimeofday(&end, NULL);
+
+ if (end.tv_sec > data->fed_through_time.tv_sec ||
+ (end.tv_sec == data->fed_through_time.tv_sec &&
+ end.tv_usec >= data->fed_through_time.tv_usec)) {
+ data->fed_through_time.tv_sec = data->fed_through_time.tv_usec = 0;
+ data->remainder = 0;
return 1;
}
- while ((ent = readdir(dir)))
- {
- char path[PATH_MAX];
+ timersub(&data->fed_through_time, &end, &diff);
+ if (diff.tv_sec == 0 && diff.tv_usec < PERIOD_MS * 1000 * FEED_BUFFER_PERIODS)
+ return 1;
- if (ent->d_name[0] == '.')
- /* skip dot-files */
- continue;
+ return 0;
+}
- for (i = 0; i < MAX_FIFOS; ++i)
- if (ent->d_ino == data->inodes[i])
- break;
- if (i < MAX_FIFOS)
- /* file already open */
- continue;
+static void did_feed(struct audio_data *data, int len)
+{
+ struct timeval diff;
- for (i = 0; i < MAX_FIFOS; ++i)
- if (data->fifo_fds[i] < 0)
- break;
- if (i == MAX_FIFOS)
- {
- static int once = 0;
- if (!once)
- {
- ErrorF("playback: Too many FIFOs already open\n");
- ++once;
+ if (data->fed_through_time.tv_sec == 0 && data->fed_through_time.tv_usec == 0)
+ gettimeofday(&data->fed_through_time, NULL);
+
+ diff.tv_sec = 0;
+ diff.tv_usec = (data->remainder + (len * PERIOD_MS * 1000)) / data->period_bytes;
+ data->remainder = (data->remainder + (len * PERIOD_MS * 1000)) % data->period_bytes;
+
+ timeradd(&data->fed_through_time, &diff, &data->fed_through_time);
+}
+
+static void condense_fifos(struct audio_data *data)
+{
+ int i;
+ struct fifo_data tmp;
+
+ for (i = 0; i < data->fifo_count; i++) {
+ struct fifo_data *f = &data->fifos[i];
+ if (f->fd == -1 && f->len == 0) {
+ if ((i + 1) < data->fifo_count) {
+ tmp = *f;
+ *f = data->fifos[data->fifo_count - 1];
+ data->fifos[data->fifo_count - 1] = tmp;
}
- closedir(dir);
- return 0;
+ data->fifo_count--;
+ i--;
}
+ }
+}
- if (snprintf(path, sizeof(path), "%s/%s", dirname, ent->d_name) >= sizeof(path)) {
- ErrorF("playback: FIFO filename is too long - truncated into %s", path);
+static void watch_or_wait(qxl_screen_t *qxl);
+static void process_fifos(qxl_screen_t *qxl, struct audio_data *data, int maxlen)
+{
+ while (maxlen > 0) {
+ if (! data->spice_buffer) {
+ uint32_t chunk_frames;
+ spice_server_playback_get_buffer(&qxl->playback_sin, &data->spice_buffer, &chunk_frames);
+ data->spice_buffer_bytes = chunk_frames * sizeof(int16_t) * SPICE_INTERFACE_PLAYBACK_CHAN;
}
- data->fifo_fds[i] = open(path, O_RDONLY | O_RSYNC | O_NONBLOCK);
- if (data->fifo_fds[i] < 0)
- {
- ErrorF("playback: open FIFO '%s' failed: %s\n", path, strerror(errno));
- continue;
- }
- ErrorF("playback: opened FIFO '%s' as %d\n", path, data->fifo_fds[i]);
+ if (! data->spice_buffer)
+ break;
- data->inodes[i] = ent->d_ino;
- }
+ if (! can_feed(data))
+ break;
- closedir(dir);
+ mix_in_fifos(qxl);
- return 0;
+ did_feed(data, data->spice_buffer_bytes);
+ maxlen -= data->spice_buffer_bytes;
+
+ spice_server_playback_put_samples(&qxl->playback_sin, data->spice_buffer);
+ data->spice_buffer = NULL;
+ }
+
+ watch_or_wait(qxl);
}
-static void *
-audio_thread_main (void *p)
+static void read_from_fifos(int fd, int event, void *opaque)
{
- qxl_screen_t *qxl = p;
+ qxl_screen_t *qxl = opaque;
+ struct audio_data *data = qxl->playback_opaque;
int i;
- struct audio_data data;
- int freq = SPICE_INTERFACE_PLAYBACK_FREQ;
+ int maxlen = 0;
+ for (i = 0; i < data->fifo_count; i++) {
+ struct fifo_data *f = &data->fifos[i];
+
+ if (f->size - f->len > 0 && f->fd >= 0) {
+ int rc;
+
+ rc = fifo_read(f);
+ if (rc == -1 && (errno == EAGAIN || errno == EINTR))
+ /* no new data to read */;
+ else if (rc <= 0) {
+ if (rc == 0)
+ ErrorF("fifo %d closed\n", f->fd);
+ else
+ ErrorF("fifo %d error %d: %s\n", f->fd, errno, strerror(errno));
- memset(&data, 0, sizeof(data));
- for (i = 0; i < MAX_FIFOS; ++i)
- data.fifo_fds[i] = -1;
+ if (f->watch)
+ qxl->core->watch_remove(f->watch);
+ f->watch = NULL;
+ close(f->fd);
+ f->fd = -1;
+ }
-#if SPICE_INTERFACE_PLAYBACK_MAJOR > 1 || SPICE_INTERFACE_PLAYBACK_MINOR >= 3
- freq = spice_server_get_best_playback_rate(&qxl->playback_sin);
-#endif
- data.period_frames = freq * PERIOD_MS / 1000;
+ if (f->size == f->len) {
+ if (f->watch)
+ qxl->core->watch_remove(f->watch);
+ f->watch = NULL;
+ }
+ }
+ if (f->len > maxlen)
+ maxlen = f->len;
+ }
- data.frame_bytes = sizeof(int16_t) * SPICE_INTERFACE_PLAYBACK_CHAN;
+ process_fifos(qxl, data, maxlen);
+}
- data.period_bytes = data.period_frames * data.frame_bytes;
- data.buffer_bytes = data.period_bytes * BUFFER_PERIODS;
- data.buffer = malloc(data.buffer_bytes);
- memset(data.buffer, 0, data.buffer_bytes);
+static void start_watching(qxl_screen_t *qxl)
+{
+ struct audio_data *data = qxl->playback_opaque;
+ int i;
- spice_server_playback_start(&qxl->playback_sin);
+ for (i = 0; i < data->fifo_count; i++) {
+ struct fifo_data *f = &data->fifos[i];
+ if (f->watch || f->size == f->len || f->fd == -1)
+ continue;
- gettimeofday(&data.last_read_time, NULL);
+ f->watch = qxl->core->watch_add(f->fd, SPICE_WATCH_EVENT_READ, read_from_fifos, qxl);
+ }
+}
- while (1)
- {
- struct timeval end, diff, period_tv;
-
- if (scan_fifos(&data, qxl->playback_fifo_dir))
- goto cleanup;
-
- while (data.fed < BUFFER_PERIODS)
- {
- if (!read_from_fifos(&data))
- break;
-
- while (data.valid_bytes)
- {
- int to_copy_bytes;
- uint32_t read_offs;
-
- if (!data.spice_buffer)
- {
- uint32_t chunk_frames;
- spice_server_playback_get_buffer(&qxl->playback_sin, (uint32_t**)&data.spice_buffer, &chunk_frames);
- data.spice_buffer_bytes = chunk_frames * data.frame_bytes;
- }
- if (!data.spice_buffer)
- break;
-
- if (data.valid_bytes > data.write_offs)
- {
- read_offs = data.buffer_bytes + data.write_offs - data.valid_bytes;
- to_copy_bytes = min(data.buffer_bytes - read_offs,
- data.spice_buffer_bytes - data.spice_write_offs);
- }
- else
- {
- read_offs = data.write_offs - data.valid_bytes;
- to_copy_bytes = min(data.valid_bytes,
- data.spice_buffer_bytes - data.spice_write_offs);
- }
+static void watch_or_wait(qxl_screen_t *qxl)
+{
+ struct audio_data *data = qxl->playback_opaque;
- memcpy(data.spice_buffer + data.spice_write_offs,
- data.buffer + read_offs, to_copy_bytes);
+ if (! can_feed(data)) {
+ if (! data->wall_timer_live) {
+ qxl->core->timer_start(data->wall_timer, PERIOD_MS);
+ data->wall_timer_live++;
+ }
+ }
+ else {
+ start_watching(qxl);
+ if (data->wall_timer_live)
+ qxl->core->timer_cancel(data->wall_timer);
+ data->wall_timer_live = 0;
+ }
+}
- data.valid_bytes -= to_copy_bytes;
+static void wall_ticker(void *opaque)
+{
+ qxl_screen_t *qxl = opaque;
+ struct audio_data *data = qxl->playback_opaque;
- data.spice_write_offs += to_copy_bytes;
+ data->wall_timer_live = 0;
- if (data.spice_write_offs >= data.spice_buffer_bytes)
- {
- spice_server_playback_put_samples(&qxl->playback_sin, (uint32_t*)data.spice_buffer);
- data.spice_buffer = NULL;
- data.spice_buffer_bytes = data.spice_write_offs = 0;
- }
- }
- }
+ condense_fifos(data);
+
+ read_from_fifos(-1, 0, qxl);
+}
- period_tv.tv_sec = 0;
- period_tv.tv_usec = PERIOD_MS * 1000;
+static void handle_one_change(qxl_screen_t *qxl, struct inotify_event *e)
+{
- usleep(period_tv.tv_usec);
+ if (e->mask & (IN_CREATE | IN_MOVED_TO)) {
+ struct audio_data *data = qxl->playback_opaque;
+ struct fifo_data *f;
+ char *fname;
- gettimeofday(&end, NULL);
+ condense_fifos(data);
- timersub(&end, &data.last_read_time, &diff);
+ f = &data->fifos[data->fifo_count];
- while (data.fed &&
- (diff.tv_sec > 0 || diff.tv_usec >= period_tv.tv_usec))
- {
- timersub(&diff, &period_tv, &diff);
+ if (data->fifo_count == MAX_FIFOS) {
+ static int once = 0;
+ if (!once) {
+ ErrorF("playback: Too many FIFOs already open\n");
+ ++once;
+ }
+ return;
+ }
- --data.fed;
+ fname = malloc(strlen(e->name) + strlen(qxl->playback_fifo_dir) + 1 + 1);
+ strcpy(fname, qxl->playback_fifo_dir);
+ strcat(fname, "/");
+ strcat(fname, e->name);
- timeradd(&data.last_read_time, &period_tv, &data.last_read_time);
+ f->fd = open(fname, O_RDONLY | O_RSYNC | O_NONBLOCK);
+ free(fname);
+ if (f->fd < 0) {
+ ErrorF("playback: open FIFO '%s' failed: %s\n", e->name, strerror(errno));
+ return;
}
- if (!data.fed)
- data.last_read_time = end;
- }
+ ErrorF("playback: opened FIFO '%s' as %d:%d\n", e->name, data->fifo_count, f->fd);
-cleanup:
- if (data.spice_buffer)
- {
- memset(data.spice_buffer, 0, data.spice_buffer_bytes - data.spice_write_offs);
- spice_server_playback_put_samples(&qxl->playback_sin, (uint32_t*)data.spice_buffer);
- data.spice_buffer = NULL;
- data.spice_buffer_bytes = data.spice_write_offs = 0;
+ data->fifo_count++;
+
+ f->watch = qxl->core->watch_add(f->fd, SPICE_WATCH_EVENT_READ, read_from_fifos, qxl);
}
+}
- free(data.buffer);
+static void playback_dir_changed(int fd, int event, void *opaque)
+{
+ qxl_screen_t *qxl = opaque;
+ static unsigned char buf[sizeof(struct inotify_event) + NAME_MAX + 1];
+ static int index = 0;
+ struct inotify_event *e;
+ int rc;
+
+ do {
+ rc = read(fd, buf + index, sizeof(buf) - index);
+ if (rc > 0) {
+ index += rc;
+ if (index >= sizeof(*e)) {
+ int len;
+ e = (struct inotify_event *) buf;
+ len = sizeof(*e) + e->len;
+ if (index >= len) {
+ handle_one_change(qxl, e);
+ if (index > len)
+ memmove(buf, buf + index, index - len);
+ index -= len;
+ }
+ }
+ }
+ }
+ while (rc > 0);
+}
- spice_server_playback_stop(&qxl->playback_sin);
- return NULL;
-}
static const SpicePlaybackInterface playback_sif = {
{
@@ -319,14 +415,41 @@ static const SpicePlaybackInterface playback_sif = {
}
};
+static void audio_initialize (qxl_screen_t *qxl)
+{
+ int i;
+ struct audio_data *data = qxl->playback_opaque;
+ int freq = SPICE_INTERFACE_PLAYBACK_FREQ;
+ int period_frames;
+ int frame_bytes;
+
+#if SPICE_INTERFACE_PLAYBACK_MAJOR > 1 || SPICE_INTERFACE_PLAYBACK_MINOR >= 3
+ freq = spice_server_get_best_playback_rate(&qxl->playback_sin);
+#endif
+
+ period_frames = freq * PERIOD_MS / 1000;
+ frame_bytes = sizeof(int16_t) * SPICE_INTERFACE_PLAYBACK_CHAN;
+ data->period_bytes = period_frames * frame_bytes;
+
+ for (i = 0; i < MAX_FIFOS; ++i) {
+ data->fifos[i].fd = -1;
+ data->fifos[i].size = data->period_bytes * READ_BUFFER_PERIODS;
+ data->fifos[i].buffer = calloc(1, data->fifos[i].size);
+ }
+
+ spice_server_playback_start(&qxl->playback_sin);
+}
+
+
int
qxl_add_spice_playback_interface (qxl_screen_t *qxl)
{
int ret;
+ struct audio_data *data = calloc(1, sizeof(*data));
- if (qxl->playback_fifo_dir[0] == 0)
- {
+ if (qxl->playback_fifo_dir[0] == 0) {
ErrorF("playback: no audio FIFO directory, audio is disabled\n");
+ free(data);
return 0;
}
@@ -346,9 +469,24 @@ qxl_add_spice_playback_interface (qxl_screen_t *qxl)
#endif
- ret = pthread_create(&qxl->audio_thread, NULL, &audio_thread_main, qxl);
- if (ret < 0)
+
+ qxl->playback_opaque = data;
+ audio_initialize(qxl);
+
+ data->wall_timer = qxl->core->timer_add(wall_ticker, qxl);
+
+ data->dir_watch = inotify_init1(IN_NONBLOCK);
+ data->fifo_dir_watch = -1;
+ if (data->dir_watch >= 0)
+ data->fifo_dir_watch = inotify_add_watch(data->dir_watch, qxl->playback_fifo_dir, IN_CREATE | IN_MOVE);
+
+ if (data->fifo_dir_watch == -1) {
+ ErrorF("Error %s(%d) watching the fifo dir\n", strerror(errno), errno);
return errno;
+ }
+
+ data->fifo_dir_qxl_watch = qxl->core->watch_add(data->dir_watch,
+ SPICE_WATCH_EVENT_READ, playback_dir_changed, qxl);
return 0;
}
--
1.7.10.4
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel