Hey,
On Fri, Oct 31, 2014 at 10:59:20AM +0100, Victor Toso wrote:
> From: Christophe Fergeau <cfergeau@xxxxxxxxxx>
You should be the author of the patch, and mention in the commit log
that this is based on a patch that I wrote as you made non-trivial
changes to it.
As gstreamer 0.10 is no longer maintained upstream, I think we should
drop support for it. However, this means we need to test gstreamer 1.x
works well enough on Windows before dropping 0.10 support. So we can
go with this solution for now while we do the windows tests, and then
drop gstreamer 0.10 once this is working satisfactorily.
Apart from these comments, I've tested this patch on a linux client,
including in valgrind, and this works as expected. Patch looks fine too,
so ACK.
Christophe
>
>
> Changes from v1:
> - specifying that it is audio support in commit message
> - rebased with last changes
> ---
>
> To enable audio with GStreamer 1.0: --with-audio=gstreamer1
>
> There is only a few changes between those versions, worth mentioning:
> - audio capabilities "audio/x-raw,format=..." instead of
> "audio/x-raw-int,..."
> - appsink signal for new data changed from "new-buffer" to "new-sample"
> ---
> configure.ac | 17 +++++++++++---
> gtk/spice-audio.c | 4 ++--
> gtk/spice-gstaudio.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++-
> 3 files changed, 79 insertions(+), 6 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index ce36d93..71830e2 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -288,7 +288,7 @@ AS_IF([test "x$have_phodav" = "xyes"],
> AC_DEFINE([USE_PHODAV], [1], [Define if supporting phodav]))
>
> AC_ARG_WITH([audio],
> - AS_HELP_STRING([--with-audio=@<:@gstreamer/pulse/auto/no@:>@], [Select audio backend @<:@default=auto@:>@]),
> + AS_HELP_STRING([--with-audio=@<:@gstreamer/gstreamer1/pulse/auto/no@:>@], [Select audio backend @<:@default=auto@:>@]),
> [],
> [with_audio="auto"])
>
> @@ -297,7 +297,7 @@ AS_IF([test "x$with_audio" = "xauto"], [
> ])
>
> case "$with_audio" in
> - gstreamer|pulse|no*)
> + gstreamer|gstreamer1|pulse|no*)
> ;;
> *) AC_MSG_ERROR(Unsupported audio backend)
> esac
> @@ -326,7 +326,18 @@ AS_IF([test "x$have_gst" = "xyes"],
> [AC_MSG_ERROR([GStreamer requested but not found])
> ])
> ])
> -AM_CONDITIONAL([WITH_GSTAUDIO], [test "x$have_gst" = "xyes"])
> +
> +AS_IF([test "x$with_audio" = "xgstreamer1"],
> + [PKG_CHECK_MODULES(GST, gstreamer-1.0 gstreamer-base-1.0 gstreamer-app-1.0 gstreamer-audio-1.0, [have_gst1=yes], [have_gst1=no])],
> + [have_gst1=no])
> +
> +AS_IF([test "x$have_gst1" = "xyes"],
> + [AC_DEFINE([WITH_GST1AUDIO], 1, [Have GStreamer 1.0?])],
> + [AS_IF([test "x$with_audio" = "xgstreamer1"],
> + [AC_MSG_ERROR([GStreamer 1.0 requested but not found])
> + ])
> +])
> +AM_CONDITIONAL([WITH_GSTAUDIO], [test "x$have_gst" = "xyes" -o "x$have_gst1" = "xyes"])
> AC_SUBST(GST_CFLAGS)
> AC_SUBST(GST_LIBS)
>
> diff --git a/gtk/spice-audio.c b/gtk/spice-audio.c
> index e7af7d5..638f667 100644
> --- a/gtk/spice-audio.c
> +++ b/gtk/spice-audio.c
> @@ -45,7 +45,7 @@
> #ifdef WITH_PULSE
> #include "spice-pulse.h"
> #endif
> -#ifdef WITH_GSTAUDIO
> +#if defined(WITH_GSTAUDIO) || defined(WITH_GST1AUDIO)
> #include "spice-gstaudio.h"
> #endif
>
> @@ -217,7 +217,7 @@ SpiceAudio *spice_audio_new(SpiceSession *session, GMainContext *context,
> #ifdef WITH_PULSE
> self = SPICE_AUDIO(spice_pulse_new(session, context, name));
> #endif
> -#ifdef WITH_GSTAUDIO
> +#if defined(WITH_GSTAUDIO) || defined(WITH_GST1AUDIO)
> self = SPICE_AUDIO(spice_gstaudio_new(session, context, name));
> #endif
> if (!self)
> diff --git a/gtk/spice-gstaudio.c b/gtk/spice-gstaudio.c
> index 69bfa78..e6cf6a9 100644
> --- a/gtk/spice-gstaudio.c
> +++ b/gtk/spice-gstaudio.c
> @@ -19,9 +19,13 @@
>
> #include <gst/gst.h>
> #include <gst/app/gstappsrc.h>
> -#include <gst/app/gstappbuffer.h>
> #include <gst/app/gstappsink.h>
> +#ifdef WITH_GST1AUDIO
> +#include <gst/audio/streamvolume.h>
> +#else
> +#include <gst/app/gstappbuffer.h>
> #include <gst/interfaces/streamvolume.h>
> +#endif
>
> #include "spice-gstaudio.h"
> #include "spice-common.h"
> @@ -131,7 +135,12 @@ static void record_new_buffer(GstAppSink *appsink, gpointer data)
>
> g_return_if_fail(p != NULL);
>
> +#ifdef WITH_GST1AUDIO
> + msg = gst_message_new_application(GST_OBJECT(p->record.pipe),
> + gst_structure_new_empty ("new-sample"));
> +#else
> msg = gst_message_new_application(GST_OBJECT(p->record.pipe), NULL);
> +#endif
> gst_element_post_message(p->record.pipe, msg);
> }
>
> @@ -153,6 +162,38 @@ static gboolean record_bus_cb(GstBus *bus, GstMessage *msg, gpointer data)
> g_return_val_if_fail(p != NULL, FALSE);
>
> switch (GST_MESSAGE_TYPE(msg)) {
> +#ifdef WITH_GST1AUDIO
> + case GST_MESSAGE_APPLICATION: {
> + GstSample *s;
> + GstBuffer *buffer;
> + GstMapInfo mapping;
> +
> + s = gst_app_sink_pull_sample(GST_APP_SINK(p->record.sink));
> + if (!s) {
> + if (!gst_app_sink_is_eos(GST_APP_SINK(p->record.sink)))
> + g_warning("eos not reached, but can't pull new sample");
> + return TRUE;
> + }
> +
> + buffer = gst_sample_get_buffer(s);
> + if (!buffer) {
> + if (!gst_app_sink_is_eos(GST_APP_SINK(p->record.sink)))
> + g_warning("eos not reached, but can't pull new buffer");
> + return TRUE;
> + }
> + if (!gst_buffer_map(buffer, &mapping, GST_MAP_READ)) {
> + return TRUE;
> + }
> +
> + spice_record_send_data(SPICE_RECORD_CHANNEL(p->rchannel),
> + /* FIXME: server side doesn't care about ts?
> + what is the unit? ms apparently */
> + mapping.data, mapping.size, 0);
> + gst_buffer_unmap(buffer, &mapping);
> + gst_sample_unref(s);
> + break;
> + }
> +#else
> case GST_MESSAGE_APPLICATION: {
> GstBuffer *b;
>
> @@ -169,6 +210,7 @@ static gboolean record_bus_cb(GstBus *bus, GstMessage *msg, gpointer data)
> GST_BUFFER_DATA(b), GST_BUFFER_SIZE(b), 0);
> break;
> }
> +#endif
> default:
> break;
> }
> @@ -196,9 +238,15 @@ static void record_start(SpiceRecordChannel *channel, gint format, gint channels
> if (!p->record.pipe) {
> GError *error = NULL;
> GstBus *bus;
> +#ifdef WITH_GST1AUDIO
> + gchar *audio_caps =
> + g_strdup_printf("audio/x-raw,format=\"S16LE\",channels=%d,rate=%d,"
> + "layout=interleaved", channels, frequency);
> +#else
> gchar *audio_caps =
> g_strdup_printf("audio/x-raw-int,channels=%d,rate=%d,signed=(boolean)true,"
> "width=16,depth=16,endianness=1234", channels, frequency);
> +#endif
> gchar *pipeline =
> g_strdup_printf("autoaudiosrc name=audiosrc ! queue ! audioconvert ! audioresample ! "
> "appsink caps=\"%s\" name=appsink", audio_caps);
> @@ -219,8 +267,13 @@ static void record_start(SpiceRecordChannel *channel, gint format, gint channels
> p->record.channels = channels;
>
> gst_app_sink_set_emit_signals(GST_APP_SINK(p->record.sink), TRUE);
> +#ifdef WITH_GST1AUDIO
> + spice_g_signal_connect_object(p->record.sink, "new-sample",
> + G_CALLBACK(record_new_buffer), gstaudio, 0);
> +#else
> spice_g_signal_connect_object(p->record.sink, "new-buffer",
> G_CALLBACK(record_new_buffer), gstaudio, 0);
> +#endif
>
> lerr:
> g_clear_error(&error);
> @@ -311,8 +364,13 @@ static void playback_start(SpicePlaybackChannel *channel, gint format, gint chan
> if (!p->playback.pipe) {
> GError *error = NULL;
> gchar *audio_caps =
> +#ifdef WITH_GST1AUDIO
> + g_strdup_printf("audio/x-raw,format=\"S16LE\",channels=%d,rate=%d,"
> + "layout=interleaved", channels, frequency);
> +#else
> g_strdup_printf("audio/x-raw-int,channels=%d,rate=%d,signed=(boolean)true,"
> "width=16,depth=16,endianness=1234", channels, frequency);
> +#endif
> gchar *pipeline = g_strdup (g_getenv("SPICE_GST_AUDIOSINK"));
> if (pipeline == NULL)
> pipeline = g_strdup_printf("appsrc is-live=1 do-timestamp=0 caps=\"%s\" name=\"appsrc\" ! queue ! "
> @@ -354,7 +412,11 @@ static void playback_data(SpicePlaybackChannel *channel,
> g_return_if_fail(p != NULL);
>
> audio = g_memdup(audio, size); /* TODO: try to avoid memory copy */
> +#ifdef WITH_GST1AUDIO
> + buf = gst_buffer_new_wrapped(audio, size);
> +#else
> buf = gst_app_buffer_new(audio, size, g_free, audio);
> +#endif
> gst_app_src_push_buffer(GST_APP_SRC(p->playback.src), buf);
> }
>
> --
> 2.1.0
>
Attachment:
pgpVnFrfDpfDz.pgp
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel