On 03/14/2011 02:48 PM, Max Kellermann wrote:
On 2011/03/14 01:52, Philipp Schafft<l...@lion.leolix.org> wrote:
Please comment the patch and tell us what needs to be changed/updated if
any.
Some pedantic comments:
You don't need to check for g_mutex_new() failures.
When roar_init() returns NULL, the error must be set, or MPD
segfaults (roar_mm_calloc() error handler).
roar_finish() does not need to call roar_close(), because MPD
guarantees that an output is closed before it is destructed.
No need to check for NULL before g_free() (roar_finish()).
Similar with g_mutex_free(): at this point in roar_finish(),
self->lock cannot be NULL. You could add an assert(self->lock!=NULL)
if you want to ensure that.
Break long lines (80 columns), some of the error messages are too
long.
I get three compiler warnings (use --enable-werror!):
src/output/roar_plugin.c:284:14: error: assignment discards qualifiers from
pointer target type
src/output/roar_plugin.c:296:19: error: assignment discards qualifiers from
pointer target type
src/output/roar_plugin.c:301:19: error: assignment discards qualifiers from
pointer target type
Looks good overall. If you fix the error bug and the warnings, I'll
merge it.
Max
Hi, I updated the patch to fix the issues mentioned in the comments.
>From 84f1df4ad9a89b3d21650e9708c6ded0dec35df5 Mon Sep 17 00:00:00 2001
From: Themaister <mais...@archlinux.us>
Date: Tue, 8 Feb 2011 00:17:58 +0100
Subject: [PATCH] RoarAudio plugin
---
Makefile.am | 7 +
configure.ac | 17 ++
src/mixer/roar_mixer_plugin.c | 137 ++++++++++++++++
src/mixer_list.h | 1 +
src/output/roar_output_plugin.h | 41 +++++
src/output/roar_plugin.c | 331 +++++++++++++++++++++++++++++++++++++++
src/output_list.c | 4 +
7 files changed, 538 insertions(+), 0 deletions(-)
create mode 100644 src/mixer/roar_mixer_plugin.c
create mode 100644 src/output/roar_output_plugin.h
create mode 100644 src/output/roar_plugin.c
diff --git a/Makefile.am b/Makefile.am
index d4fec31..7b41d1c 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -142,6 +142,7 @@ mpd_headers = \
src/output/httpd_client.h \
src/output/httpd_internal.h \
src/output/pulse_output_plugin.h \
+ src/output/roar_output_plugin.h \
src/output/winmm_output_plugin.h \
src/page.h \
src/pcm_buffer.h \
@@ -673,6 +674,7 @@ OUTPUT_LIBS = \
$(LIBWRAP_LDFLAGS) \
$(AO_LIBS) \
$(ALSA_LIBS) \
+ $(ROAR_LIBS) \
$(FFADO_LIBS) \
$(JACK_LIBS) \
$(OPENAL_LIBS) \
@@ -706,6 +708,11 @@ OUTPUT_SRC += src/output/alsa_plugin.c
MIXER_SRC += src/mixer/alsa_mixer_plugin.c
endif
+if HAVE_ROAR
+OUTPUT_SRC += src/output/roar_plugin.c
+MIXER_SRC += src/mixer/roar_mixer_plugin.c
+endif
+
if ENABLE_FFADO_OUTPUT
OUTPUT_SRC += src/output/ffado_output_plugin.c
endif
diff --git a/configure.ac b/configure.ac
index 89b5ce3..0d07555 100644
--- a/configure.ac
+++ b/configure.ac
@@ -121,6 +121,11 @@ AC_ARG_ENABLE(alsa,
AS_HELP_STRING([--enable-alsa], [enable ALSA support]),,
[enable_alsa=auto])
+AC_ARG_ENABLE(roar,
+ AS_HELP_STRING([--enable-roar],
+ [enable support for RoarAudio]),,
+ [enable_roar=auto])
+
AC_ARG_ENABLE(ao,
AS_HELP_STRING([--enable-ao],
[enable support for libao]),,
@@ -1221,6 +1226,16 @@ fi
AM_CONDITIONAL(HAVE_ALSA, test x$enable_alsa = xyes)
+dnl ----------------------------------- ROAR ----------------------------------
+MPD_AUTO_PKG(roar, ROAR, [libroar >= 0.4.0],
+ [ROAR output plugin], [libroar not found])
+
+if test x$enable_roar = xyes; then
+ AC_DEFINE(HAVE_ROAR, 1, [Define to enable ROAR support])
+fi
+
+AM_CONDITIONAL(HAVE_ROAR, test x$enable_roar = xyes)
+
dnl ----------------------------------- FFADO ---------------------------------
MPD_AUTO_PKG(ffado, FFADO, [libffado],
@@ -1430,6 +1445,7 @@ AM_CONDITIONAL(ENABLE_WINMM_OUTPUT, test x$enable_winmm_output = xyes)
dnl --------------------- Post Audio Output Plugins Tests ---------------------
if
test x$enable_alsa = xno &&
+ test x$enable_roar = xno &&
test x$enable_ao = xno &&
test x$enable_ffado = xno &&
test x$enable_fifo = xno &&
@@ -1566,6 +1582,7 @@ results(id3,[ID3])
printf '\nPlayback support:\n\t'
results(alsa,ALSA)
+results(roar,ROAR)
results(ffado,FFADO)
results(fifo,FIFO)
results(recorder_output,[File Recorder])
diff --git a/src/mixer/roar_mixer_plugin.c b/src/mixer/roar_mixer_plugin.c
new file mode 100644
index 0000000..85a4ceb
--- /dev/null
+++ b/src/mixer/roar_mixer_plugin.c
@@ -0,0 +1,137 @@
+/*
+ * Copyright (C) 2003-2010 The Music Player Daemon Project
+ * Copyright (C) 2010-2011 Philipp 'ph3-der-loewe' Schafft
+ * Copyright (C) 2010-2011 Hans-Kristian 'maister' Arntzen
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+
+#include "config.h"
+#include "mixer_api.h"
+#include "output_api.h"
+#include "output/roar_output_plugin.h"
+
+#include <glib.h>
+
+#include <assert.h>
+#include <sys/stat.h>
+#include <sys/ioctl.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+typedef struct roar_mpd_mixer
+{
+ /** the base mixer class */
+ struct mixer base;
+ roar_t *self;
+} roar_mixer_t;
+
+/**
+ * The quark used for GError.domain.
+ */
+static inline GQuark
+roar_mixer_quark(void)
+{
+ return g_quark_from_static_string("roar_mixer");
+}
+
+static struct mixer *
+roar_mixer_init(void *ao, G_GNUC_UNUSED const struct config_param *param,
+ G_GNUC_UNUSED GError **error_r)
+{
+ roar_mixer_t *self = g_new(roar_mixer_t, 1);
+ self->self = ao;
+
+ mixer_init(&self->base, &roar_mixer_plugin);
+
+ return &self->base;
+}
+
+static void
+roar_mixer_finish(struct mixer *data)
+{
+ roar_mixer_t *self = (roar_mixer_t *) data;
+
+ g_free(self);
+}
+
+static void
+roar_mixer_close(G_GNUC_UNUSED struct mixer *data)
+{
+}
+
+static bool
+roar_mixer_open(G_GNUC_UNUSED struct mixer *data,
+ G_GNUC_UNUSED GError **error_r)
+{
+ return true;
+}
+
+static int
+roar_mixer_get_volume(struct mixer *mixer, G_GNUC_UNUSED GError **error_r)
+{
+ roar_mixer_t *self = (roar_mixer_t *)mixer;
+ g_mutex_lock(self->self->lock);
+ if (self->self->vss && self->self->alive)
+ {
+ float l, r;
+ int error;
+ roar_vs_volume_get(self->self->vss, &l, &r, &error);
+ g_mutex_unlock(self->self->lock);
+ return (l + r) * 50;
+ }
+ else
+ {
+ g_mutex_unlock(self->self->lock);
+ return 0;
+ }
+}
+
+static bool
+roar_mixer_set_volume(struct mixer *mixer, unsigned volume,
+ G_GNUC_UNUSED GError **error_r)
+{
+ roar_mixer_t *self = (roar_mixer_t *)mixer;
+ g_mutex_lock(self->self->lock);
+ if (self->self->vss && self->self->alive)
+ {
+ assert(volume <= 100);
+
+ int error;
+ float level = volume / 100.0;
+
+ roar_vs_volume_mono(self->self->vss, level, &error);
+ g_mutex_unlock(self->self->lock);
+ return true;
+ }
+ else
+ {
+ g_mutex_unlock(self->self->lock);
+ return false;
+ }
+}
+
+const struct mixer_plugin roar_mixer_plugin = {
+ .init = roar_mixer_init,
+ .finish = roar_mixer_finish,
+ .open = roar_mixer_open,
+ .close = roar_mixer_close,
+ .get_volume = roar_mixer_get_volume,
+ .set_volume = roar_mixer_set_volume,
+ .global = false,
+};
diff --git a/src/mixer_list.h b/src/mixer_list.h
index f284c2d..95ded5c 100644
--- a/src/mixer_list.h
+++ b/src/mixer_list.h
@@ -28,6 +28,7 @@
extern const struct mixer_plugin software_mixer_plugin;
extern const struct mixer_plugin alsa_mixer_plugin;
extern const struct mixer_plugin oss_mixer_plugin;
+extern const struct mixer_plugin roar_mixer_plugin;
extern const struct mixer_plugin pulse_mixer_plugin;
extern const struct mixer_plugin raop_mixer_plugin;
extern const struct mixer_plugin winmm_mixer_plugin;
diff --git a/src/output/roar_output_plugin.h b/src/output/roar_output_plugin.h
new file mode 100644
index 0000000..1db7bff
--- /dev/null
+++ b/src/output/roar_output_plugin.h
@@ -0,0 +1,41 @@
+/*
+ * Copyright (C) 2003-2010 The Music Player Daemon Project
+ * Copyright (C) 2010-2011 Philipp 'ph3-der-loewe' Schafft
+ * Copyright (C) 2010-2011 Hans-Kristian 'maister' Arntzen
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+
+#ifndef __ROAR_OUTPUT_H
+#define __ROAR_OUTPUT_H
+
+#include <roaraudio.h>
+#include <glib.h>
+
+typedef struct roar
+{
+ roar_vs_t * vss;
+ int err;
+ char *host;
+ char *name;
+ int role;
+ struct roar_connection con;
+ struct roar_audio_info info;
+ GMutex *lock;
+ volatile bool alive;
+} roar_t;
+
+#endif
diff --git a/src/output/roar_plugin.c b/src/output/roar_plugin.c
new file mode 100644
index 0000000..7bb878f
--- /dev/null
+++ b/src/output/roar_plugin.c
@@ -0,0 +1,331 @@
+/*
+ * Copyright (C) 2003-2010 The Music Player Daemon Project
+ * Copyright (C) 2010-2011 Philipp 'ph3-der-loewe' Schafft
+ * Copyright (C) 2010-2011 Hans-Kristian 'maister' Arntzen
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include "config.h"
+#include "output_api.h"
+#include "mixer_list.h"
+#include "roar_output_plugin.h"
+
+#include <glib.h>
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <arpa/inet.h>
+#include <netdb.h>
+#include <stdint.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <string.h>
+#include <stdint.h>
+
+
+#undef G_LOG_DOMAIN
+#define G_LOG_DOMAIN "roaraudio"
+
+static inline GQuark
+roar_output_quark(void)
+{
+ return g_quark_from_static_string("roar_output");
+}
+
+static void
+roar_configure(struct roar * self, const struct config_param *param)
+{
+ self->host = config_dup_block_string(param, "server", NULL);
+ self->name = config_dup_block_string(param, "name", "MPD");
+ char *role = config_dup_block_string(param, "role", "music");
+ if (role != NULL)
+ {
+ self->role = roar_str2role(role);
+ g_free(role);
+ }
+ else
+ self->role = ROAR_ROLE_MUSIC;
+}
+
+static void *
+roar_init(G_GNUC_UNUSED const struct audio_format *audio_format,
+ const struct config_param *param,
+ G_GNUC_UNUSED GError **error)
+{
+ GMutex *lock = g_mutex_new();
+
+ roar_t * self = roar_mm_calloc(1, sizeof(*self));
+ if (self == NULL)
+ {
+ g_set_error(error, roar_output_quark(), 0, "Failed to allocate memory");
+ return NULL;
+ }
+
+ self->lock = lock;
+ self->err = ROAR_ERROR_NONE;
+ roar_configure(self, param);
+ return self;
+}
+
+static void
+roar_close(void *data)
+{
+ roar_t * self = data;
+ g_mutex_lock(self->lock);
+ self->alive = false;
+
+ if (self->vss != NULL)
+ roar_vs_close(self->vss, ROAR_VS_TRUE, &(self->err));
+ self->vss = NULL;
+ roar_disconnect(&(self->con));
+ g_mutex_unlock(self->lock);
+}
+
+static void
+roar_finish(void *data)
+{
+ roar_t * self = data;
+
+ g_free(self->host);
+ g_free(self->name);
+ g_mutex_free(self->lock);
+
+ roar_mm_free(data);
+}
+
+static bool
+roar_open(void *data, struct audio_format *audio_format, GError **error)
+{
+ roar_t * self = data;
+ g_mutex_lock(self->lock);
+
+ if (roar_simple_connect(&(self->con), self->host, self->name) < 0)
+ {
+ g_set_error(error, roar_output_quark(), 0,
+ "Failed to connect to Roar server");
+ g_mutex_unlock(self->lock);
+ return false;
+ }
+
+ self->vss = roar_vs_new_from_con(&(self->con), &(self->err));
+
+ if (self->vss == NULL || self->err != ROAR_ERROR_NONE)
+ {
+ g_set_error(error, roar_output_quark(), 0,
+ "Failed to connect to server");
+ g_mutex_unlock(self->lock);
+ return false;
+ }
+
+ self->info.rate = audio_format->sample_rate;
+ self->info.channels = audio_format->channels;
+ self->info.codec = ROAR_CODEC_PCM_S;
+
+ switch (audio_format->format)
+ {
+ case SAMPLE_FORMAT_S8:
+ self->info.bits = 8;
+ break;
+ case SAMPLE_FORMAT_S16:
+ self->info.bits = 16;
+ break;
+ case SAMPLE_FORMAT_S24:
+ self->info.bits = 24;
+ break;
+ case SAMPLE_FORMAT_S24_P32:
+ self->info.bits = 32;
+ audio_format->format = SAMPLE_FORMAT_S32;
+ break;
+ case SAMPLE_FORMAT_S32:
+ self->info.bits = 32;
+ break;
+ default:
+ self->info.bits = 16;
+ audio_format->format = SAMPLE_FORMAT_S16;
+ }
+ audio_format->reverse_endian = 0;
+
+ if (roar_vs_stream(self->vss, &(self->info), ROAR_DIR_PLAY,
+ &(self->err)) < 0)
+ {
+ g_set_error(error, roar_output_quark(), 0, "Failed to start stream");
+ g_mutex_unlock(self->lock);
+ return false;
+ }
+ roar_vs_role(self->vss, self->role, &(self->err));
+ self->alive = true;
+
+ g_mutex_unlock(self->lock);
+ return true;
+}
+
+static void
+roar_cancel(void *data)
+{
+ roar_t * self = data;
+
+ g_mutex_lock(self->lock);
+ if (self->vss != NULL)
+ {
+ roar_vs_t *vss = self->vss;
+ self->vss = NULL;
+ roar_vs_close(vss, ROAR_VS_TRUE, &(self->err));
+ self->alive = false;
+
+ vss = roar_vs_new_from_con(&(self->con), &(self->err));
+ if (vss)
+ {
+ roar_vs_stream(vss, &(self->info), ROAR_DIR_PLAY, &(self->err));
+ roar_vs_role(vss, self->role, &(self->err));
+ self->vss = vss;
+ self->alive = true;
+ }
+ }
+ g_mutex_unlock(self->lock);
+}
+
+static size_t
+roar_play(void *data, const void *chunk, size_t size, GError **error)
+{
+ struct roar * self = data;
+ ssize_t rc;
+
+ if (self->vss == NULL)
+ {
+ g_set_error(error, roar_output_quark(), 0, "Connection is invalid");
+ return 0;
+ }
+
+ rc = roar_vs_write(self->vss, chunk, size, &(self->err));
+ if ( rc <= 0 )
+ {
+ g_set_error(error, roar_output_quark(), 0, "Failed to play data");
+ return 0;
+ }
+
+ return rc;
+}
+
+static const char*
+roar_tag_convert(enum tag_type type, bool *is_uuid)
+{
+ *is_uuid = false;
+ switch (type)
+ {
+ case TAG_ARTIST:
+ case TAG_ALBUM_ARTIST:
+ return "AUTHOR";
+ case TAG_ALBUM:
+ return "ALBUM";
+ case TAG_TITLE:
+ return "TITLE";
+ case TAG_TRACK:
+ return "TRACK";
+ case TAG_NAME:
+ return "NAME";
+ case TAG_GENRE:
+ return "GENRE";
+ case TAG_DATE:
+ return "DATE";
+ case TAG_PERFORMER:
+ return "PERFORMER";
+ case TAG_COMMENT:
+ return "COMMENT";
+ case TAG_DISC:
+ return "DISCID";
+ case TAG_COMPOSER:
+#ifdef ROAR_META_TYPE_COMPOSER
+ return "COMPOSER";
+#else
+ return "AUTHOR";
+#endif
+ case TAG_MUSICBRAINZ_ARTISTID:
+ case TAG_MUSICBRAINZ_ALBUMID:
+ case TAG_MUSICBRAINZ_ALBUMARTISTID:
+ case TAG_MUSICBRAINZ_TRACKID:
+ *is_uuid = true;
+ return "HASH";
+
+ default:
+ return NULL;
+ }
+}
+
+static void
+roar_send_tag(void *data, const struct tag *meta)
+{
+ struct roar * self = data;
+
+ if (self->vss == NULL)
+ return;
+
+ g_mutex_lock(self->lock);
+ size_t cnt = 1;
+ struct roar_keyval vals[32];
+ memset(vals, 0, sizeof(vals));
+ char uuid_buf[32][64];
+
+ char timebuf[16];
+ snprintf(timebuf, sizeof(timebuf), "%02d:%02d:%02d",
+ meta->time / 3600, (meta->time % 3600) / 60, meta->time % 60);
+
+ vals[0].key = g_strdup("LENGTH");
+ vals[0].value = timebuf;
+
+ for (unsigned i = 0; i < meta->num_items && cnt < 32; i++)
+ {
+ bool is_uuid = false;
+ const char *key = roar_tag_convert(meta->items[i]->type, &is_uuid);
+ if (key != NULL)
+ {
+ if (is_uuid)
+ {
+ snprintf(uuid_buf[cnt], sizeof(uuid_buf[0]), "{UUID}%s",
+ meta->items[i]->value);
+ vals[cnt].key = g_strdup(key);
+ vals[cnt].value = uuid_buf[cnt];
+ }
+ else
+ {
+ vals[cnt].key = g_strdup(key);
+ vals[cnt].value = meta->items[i]->value;
+ }
+ cnt++;
+ }
+ }
+
+ roar_vs_meta(self->vss, vals, cnt, &(self->err));
+
+ for (unsigned i = 0; i < 32; i++)
+ g_free(vals[i].key);
+
+ g_mutex_unlock(self->lock);
+}
+
+const struct audio_output_plugin roar_output_plugin = {
+ .name = "roar",
+ .init = roar_init,
+ .finish = roar_finish,
+ .open = roar_open,
+ .play = roar_play,
+ .cancel = roar_cancel,
+ .close = roar_close,
+ .send_tag = roar_send_tag,
+
+ .mixer_plugin = &roar_mixer_plugin
+};
+
+
diff --git a/src/output_list.c b/src/output_list.c
index 24b089e..75d24d6 100644
--- a/src/output_list.c
+++ b/src/output_list.c
@@ -26,6 +26,7 @@ extern const struct audio_output_plugin null_output_plugin;
extern const struct audio_output_plugin fifo_output_plugin;
extern const struct audio_output_plugin pipe_output_plugin;
extern const struct audio_output_plugin alsaPlugin;
+extern const struct audio_output_plugin roar_output_plugin;
extern const struct audio_output_plugin ao_output_plugin;
extern const struct audio_output_plugin oss_output_plugin;
extern const struct audio_output_plugin openal_output_plugin;
@@ -54,6 +55,9 @@ const struct audio_output_plugin *audio_output_plugins[] = {
#ifdef HAVE_ALSA
&alsaPlugin,
#endif
+#ifdef HAVE_ROAR
+ &roar_output_plugin,
+#endif
#ifdef HAVE_AO
&ao_output_plugin,
#endif
--
1.7.4.1
------------------------------------------------------------------------------
Colocation vs. Managed Hosting
A question and answer guide to determining the best fit
for your organization - today and in the future.
http://p.sf.net/sfu/internap-sfd2d
_______________________________________________
Musicpd-dev-team mailing list
Musicpd-dev-team@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
