On 03/14/2011 02:48 PM, Max Kellermann wrote:
On 2011/03/14 01:52, Philipp Schafft 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
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 000..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 wi
