On Fri, 16 Oct 2015, Victor Toso wrote:
[...]
> > @@ -321,9 +321,9 @@ AS_IF([test "x$with_audio" = "xauto"],
> > AS_IF([test "x$with_audio" = "xpulse"],
> > [AS_IF([test "x$have_pulse" = "xyes"],
> > [AC_DEFINE([WITH_PULSE], 1, [Have pulseaudio?])],
> > - [AC_MSG_ERROR([PulseAudio requested but not found])
> > - ])
> > -])
> > + [AC_MSG_ERROR([PulseAudio requested but not found])]
> > + )]
> > +)
>
> Order is inverted )] ?
>
> checking for GSTVIDEO... yes
> ./configure: line 17978: ]: command not found
Ah. I missed that one in the middle of the configure output.
I had also meant to fix the patch to follow the existing practice
regarding the parentheses and square brackets though I don't like it
because it's really really misleading.
Take for instance the following gst-audio check:
1: AS_IF([test "x$with_audio" = "xgstreamer"],
2: [AS_IF([test "x$have_gst_audio" = "xyes"],
3: ...
4: ...
5: ])
6: ])
Clearly the square bracket on line 5 closes '[AS_IF' since it's at the
same level. So then the parenthesis closes the 'AS_IF' from line 1. But
then what's this on line 6???
In fact the square bracket on line 5 closes a one-liner expression on
line 4. And the square bracket in column 1 on line 6 closes the bracket
one indentation level down on line 2.
So I think it makes much more sense to write this as:
1: AS_IF([test "x$with_audio" = "xgstreamer"],
2: [AS_IF([test "x$have_gst_audio" = "xyes"],
3: ...
4: ...
5: )]
6: )
The ')]' on line 5 are correctly aligned with the corresponding
'[AS_IF(' on line 2, and the lone ')' on line 6 is still aligned with
the 'AS_IF(' on line 1.
And line 4 would be self contained:
[AC_MSG_ERROR([GStreamer 1.0 audio requested but not found])]
Anyway, I changed the patch to follow existing convention, added a
GStreamer Video line in the summary, and removed the extraneous square
bracket which was one line down. See the updated attached patch.
--
Francois Gouget <fgouget@xxxxxxxxxxxxxxx>commit bf4e9313c30502af315628293a957c054e78073d
Author: Francois Gouget <fgouget@xxxxxxxxxxxxxxx>
Date: Wed Jul 29 12:49:13 2015 +0200
spice-gtk: Add a GStreamer video decoder for MJPEG, VP8 and h264
Based on a patch by Jeremy White.
Signed-off-by: Francois Gouget <fgouget@xxxxxxxxxxxxxxx>
diff --git a/configure.ac b/configure.ac
index 85da9a4..a198de3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -338,6 +338,27 @@ AM_CONDITIONAL([WITH_GSTAUDIO], [test "x$have_gst_audio" = "xyes"])
AC_SUBST(GSTAUDIO_CFLAGS)
AC_SUBST(GSTAUDIO_LIBS)
+AC_ARG_ENABLE([gst-video],
+ AS_HELP_STRING([--enable-gst-video=@<:@auto/yes/no@:>@],
+ [Enable GStreamer video support @<:@default=auto@:>@]),
+ [],
+ [enable_gst_video="auto"])
+
+AS_IF([test "x$enable_gst_video" != "xno"],
+ [PKG_CHECK_MODULES(GSTVIDEO, gstreamer-1.0 gstreamer-base-1.0 gstreamer-app-1.0 gstreamer-video-1.0, [have_gst_video="yes"], [have_gst_video="no"])
+ AS_IF([test "x$have_gst_video" = "xyes"],
+ [AC_DEFINE([HAVE_GSTVIDEO], 1, [Have GStreamer 1.0 video?])
+ enable_gst_video="yes"],
+ [AS_IF([test "x$enable_gst_video" = "xyes"],
+ [AC_MSG_ERROR([GStreamer 1.0 video requested but not found])
+ ])
+ ])
+])
+AS_IF([test "x$enable_gst_video" = "xauto"], [enable_gst_video="no"])
+AM_CONDITIONAL([HAVE_GSTVIDEO], [test "x$have_gst_video" = "xyes"])
+AC_SUBST(GSTVIDEO_CFLAGS)
+AC_SUBST(GSTVIDEO_LIBS)
+
AC_CHECK_LIB(jpeg, jpeg_destroy_decompress,
AC_MSG_CHECKING([for jpeglib.h])
AC_TRY_CPP(
@@ -702,7 +723,7 @@ SPICE_CFLAGS="$SPICE_CFLAGS $WARN_CFLAGS"
AC_SUBST(SPICE_CFLAGS)
-SPICE_GLIB_CFLAGS="$PIXMAN_CFLAGS $PULSE_CFLAGS $GSTAUDIO_CFLAGS $GLIB2_CFLAGS $GIO_CFLAGS $GOBJECT2_CFLAGS $SSL_CFLAGS $SASL_CFLAGS"
+SPICE_GLIB_CFLAGS="$PIXMAN_CFLAGS $PULSE_CFLAGS $GSTVIDEO_CFLAGS $GSTAUDIO_CFLAGS $GLIB2_CFLAGS $GIO_CFLAGS $GOBJECT2_CFLAGS $SSL_CFLAGS $SASL_CFLAGS"
SPICE_GTK_CFLAGS="$SPICE_GLIB_CFLAGS $GTK_CFLAGS "
AC_SUBST(SPICE_GLIB_CFLAGS)
@@ -747,6 +768,7 @@ AC_MSG_NOTICE([
Gtk: ${with_gtk}
Coroutine: ${with_coroutine}
Audio: ${with_audio}
+ GStreamer Video: ${enable_gst_video}
SASL support: ${enable_sasl}
Smartcard support: ${have_smartcard}
USB redirection support: ${have_usbredir} ${with_usbredir_hotplug}
diff --git a/src/Makefile.am b/src/Makefile.am
index 99a8a1f..ae43b13 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -98,6 +98,7 @@ SPICE_COMMON_CPPFLAGS = \
$(SSL_CFLAGS) \
$(SASL_CFLAGS) \
$(GSTAUDIO_CFLAGS) \
+ $(GSTVIDEO_CFLAGS) \
$(SMARTCARD_CFLAGS) \
$(USBREDIR_CFLAGS) \
$(GUDEV_CFLAGS) \
@@ -211,6 +212,7 @@ libspice_client_glib_2_0_la_LIBADD = \
$(SSL_LIBS) \
$(PULSE_LIBS) \
$(GSTAUDIO_LIBS) \
+ $(GSTVIDEO_LIBS) \
$(SASL_LIBS) \
$(SMARTCARD_LIBS) \
$(USBREDIR_LIBS) \
@@ -344,6 +346,12 @@ libspice_client_glib_2_0_la_SOURCES += \
$(NULL)
endif
+if HAVE_GSTVIDEO
+libspice_client_glib_2_0_la_SOURCES += \
+ channel-display-gst.c \
+ $(NULL)
+endif
+
if WITH_PHODAV
libspice_client_glib_2_0_la_SOURCES += \
giopipe.c \
diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
new file mode 100644
index 0000000..c5ad2e9
--- /dev/null
+++ b/src/channel-display-gst.c
@@ -0,0 +1,238 @@
+/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
+/*
+ Copyright (C) 2015 CodeWeavers, Inc
+
+ This library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ This library 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
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with this library; if not, see <http://www.gnu.org/licenses/>.
+*/
+#include "config.h"
+
+#include "spice-client.h"
+#include "spice-common.h"
+#include "spice-channel-priv.h"
+
+#include "channel-display-priv.h"
+
+#include <gst/gst.h>
+#include <gst/app/gstappsrc.h>
+#include <gst/app/gstappsink.h>
+
+
+/* GStreamer decoder implementation */
+
+typedef struct SpiceGstDecoder {
+ VideoDecoder base;
+
+ /* ---------- Video characteristics ---------- */
+
+ int width;
+ int height;
+
+ /* ---------- GStreamer pipeline ---------- */
+
+ GstElement *pipeline;
+ GstAppSrc *appsrc;
+ GstAppSink *appsink;
+
+ /* ---------- Output frame data ---------- */
+
+ GstBuffer *buffer;
+ GstMapInfo mapinfo;
+} SpiceGstDecoder;
+
+
+/* ---------- GStreamer pipeline ---------- */
+
+static void reset_pipeline(SpiceGstDecoder *decoder)
+{
+ if (!decoder->pipeline) {
+ return;
+ }
+
+ gst_element_set_state(decoder->pipeline, GST_STATE_NULL);
+ gst_object_unref(decoder->appsrc);
+ gst_object_unref(decoder->appsink);
+ gst_object_unref(decoder->pipeline);
+ decoder->pipeline = NULL;
+}
+
+static gboolean construct_pipeline(SpiceGstDecoder *decoder)
+{
+ const gchar *src_caps, *gstdec_name;
+ switch (decoder->base.codec_type) {
+ case SPICE_VIDEO_CODEC_TYPE_MJPEG:
+ src_caps = "caps=image/jpeg";
+ gstdec_name = "jpegdec";
+ break;
+ case SPICE_VIDEO_CODEC_TYPE_VP8:
+ src_caps = "caps=video/x-vp8";
+ gstdec_name = "vp8dec";
+ break;
+ case SPICE_VIDEO_CODEC_TYPE_H264:
+ src_caps = "caps=video/x-h264";
+ gstdec_name = "h264parse ! avdec_h264";
+ break;
+ default:
+ spice_warning("Unknown codec type %d", decoder->base.codec_type);
+ return -1;
+ }
+
+ GError *err = NULL;
+ gchar *desc = g_strdup_printf("appsrc name=src format=2 do-timestamp=1 %s ! %s ! videoconvert ! appsink name=sink caps=video/x-raw,format=BGRx", src_caps, gstdec_name);
+ SPICE_DEBUG("GStreamer pipeline: %s", desc);
+ decoder->pipeline = gst_parse_launch_full(desc, NULL, GST_PARSE_FLAG_FATAL_ERRORS, &err);
+ g_free(desc);
+ if (!decoder->pipeline) {
+ spice_warning("GStreamer error: %s", err->message);
+ g_clear_error(&err);
+ return FALSE;
+ }
+
+ decoder->appsrc = GST_APP_SRC(gst_bin_get_by_name(GST_BIN(decoder->pipeline), "src"));
+ decoder->appsink = GST_APP_SINK(gst_bin_get_by_name(GST_BIN(decoder->pipeline), "sink"));
+
+ if (gst_element_set_state(decoder->pipeline, GST_STATE_PLAYING) == GST_STATE_CHANGE_FAILURE) {
+ SPICE_DEBUG("GStreamer error: Unable to set the pipeline to the playing state.");
+ reset_pipeline(decoder);
+ return FALSE;
+ }
+
+ return TRUE;
+}
+
+static gboolean push_compressed_buffer(SpiceGstDecoder *decoder,
+ SpiceMsgIn *frame_msg)
+{
+ uint8_t *data;
+ uint32_t size = spice_msg_in_frame_data(frame_msg, &data);
+ if (size == 0) {
+ SPICE_DEBUG("got an empty frame buffer!");
+ return FALSE;
+ }
+
+ GstBuffer *buffer = gst_buffer_new_allocate(NULL, size, NULL);
+ gst_buffer_fill(buffer, 0, data, size);
+ if (gst_app_src_push_buffer(decoder->appsrc, buffer) != GST_FLOW_OK) {
+ SPICE_DEBUG("GStreamer error: unable to push frame of size %d", size);
+ return FALSE;
+ }
+
+ return TRUE;
+}
+
+static void release_last_frame(SpiceGstDecoder *decoder)
+{
+ if (decoder->buffer) {
+ if (decoder->mapinfo.memory) {
+ gst_buffer_unmap(decoder->buffer, &decoder->mapinfo);
+ decoder->mapinfo.memory = NULL;
+ }
+ gst_buffer_unref(decoder->buffer);
+ decoder->buffer = NULL;
+ }
+}
+
+static uint8_t* pull_raw_frame(SpiceGstDecoder *decoder)
+{
+ GstSample *sample = gst_app_sink_pull_sample(decoder->appsink);
+ if (!sample) {
+ SPICE_DEBUG("GStreamer error: could not pull sample");
+ return NULL;
+ }
+ decoder->buffer = gst_sample_get_buffer(sample);
+ gst_buffer_ref(decoder->buffer);
+ gst_sample_unref(sample);
+
+ if (gst_buffer_map(decoder->buffer, &decoder->mapinfo, GST_MAP_READ)) {
+ return decoder->mapinfo.data;
+ }
+
+ SPICE_DEBUG("GStreamer error: could not map the buffer");
+ gst_buffer_unref(decoder->buffer);
+ decoder->buffer = NULL;
+ decoder->mapinfo.memory = NULL;
+ return NULL;
+}
+
+
+/* ---------- VideoDecoder's public API ---------- */
+
+static void gst_decoder_destroy(VideoDecoder *video_decoder)
+{
+ SpiceGstDecoder *decoder = (SpiceGstDecoder*)video_decoder;
+ release_last_frame(decoder);
+ reset_pipeline(decoder);
+ g_free(decoder);
+ /* Don't call gst_deinit() as other parts may still be using GStreamer */
+}
+
+static uint8_t* gst_decoder_decode_frame(VideoDecoder *video_decoder,
+ SpiceMsgIn *frame_msg)
+{
+ SpiceGstDecoder *decoder = (SpiceGstDecoder*)video_decoder;
+ int width, height;
+
+ stream_get_dimensions(decoder->base.stream, frame_msg, &width, &height);
+ if (width != decoder->width || height != decoder->height) {
+ SPICE_DEBUG("video format change: width %d -> %d, height %d -> %d", decoder->width, width, decoder->height, height);
+ decoder->width = width;
+ decoder->height = height;
+ reset_pipeline(decoder);
+ }
+ if (!decoder->pipeline && !construct_pipeline(decoder)) {
+ return NULL;
+ }
+
+ /* Release the output frame buffer early so the pipeline can reuse it.
+ * This also simplifies error handling.
+ */
+ release_last_frame(decoder);
+
+ if (push_compressed_buffer(decoder, frame_msg)) {
+ return pull_raw_frame(decoder);
+ }
+ return NULL;
+}
+
+G_GNUC_INTERNAL
+gboolean gstvideo_init(void)
+{
+ static int success = 0;
+ if (!success) {
+ GError *err = NULL;
+ if (gst_init_check(NULL, NULL, &err)) {
+ success = 1;
+ } else {
+ spice_warning("Disabling GStreamer video support: %s", err->message);
+ g_clear_error(&err);
+ success = -1;
+ }
+ }
+ return success > 0;
+}
+
+G_GNUC_INTERNAL
+VideoDecoder* create_gstreamer_decoder(int codec_type, display_stream *stream)
+{
+ SpiceGstDecoder *decoder = NULL;
+
+ if (gstvideo_init()) {
+ decoder = spice_new0(SpiceGstDecoder, 1);
+ decoder->base.destroy = &gst_decoder_destroy;
+ decoder->base.decode_frame = &gst_decoder_decode_frame;
+ decoder->base.codec_type = codec_type;
+ decoder->base.stream = stream;
+ }
+
+ return (VideoDecoder*)decoder;
+}
diff --git a/src/channel-display-priv.h b/src/channel-display-priv.h
index 4ca80b6..d8a03e9 100644
--- a/src/channel-display-priv.h
+++ b/src/channel-display-priv.h
@@ -66,6 +66,12 @@ struct VideoDecoder {
* @return: A pointer to a structure implementing the VideoDecoder methods.
*/
VideoDecoder* create_mjpeg_decoder(int codec_type, display_stream *stream);
+#ifdef HAVE_GSTVIDEO
+VideoDecoder* create_gstreamer_decoder(int codec_type, display_stream *stream);
+gboolean gstvideo_init(void);
+#else
+# define gstvideo_init() FALSE
+#endif
typedef struct display_surface {
diff --git a/src/channel-display.c b/src/channel-display.c
index 5c916cc..6d850d6 100644
--- a/src/channel-display.c
+++ b/src/channel-display.c
@@ -596,6 +596,12 @@ static void spice_display_channel_reset_capabilities(SpiceChannel *channel)
if (SPICE_DISPLAY_CHANNEL(channel)->priv->enable_adaptive_streaming) {
spice_channel_set_capability(SPICE_CHANNEL(channel), SPICE_DISPLAY_CAP_STREAM_REPORT);
}
+ spice_channel_set_capability(SPICE_CHANNEL(channel), SPICE_DISPLAY_CAP_MULTI_CODEC);
+ spice_channel_set_capability(SPICE_CHANNEL(channel), SPICE_DISPLAY_CAP_CODEC_MJPEG);
+ if (gstvideo_init()) {
+ spice_channel_set_capability(SPICE_CHANNEL(channel), SPICE_DISPLAY_CAP_CODEC_VP8);
+ spice_channel_set_capability(SPICE_CHANNEL(channel), SPICE_DISPLAY_CAP_CODEC_H264);
+ }
}
static void destroy_surface(gpointer data)
@@ -1009,7 +1015,11 @@ static void display_handle_stream_create(SpiceChannel *channel, SpiceMsgIn *in)
st->video_decoder = create_mjpeg_decoder(op->codec_type, st);
break;
default:
+#ifdef HAVE_GSTVIDEO
+ st->video_decoder = create_gstreamer_decoder(op->codec_type, st);
+#else
st->video_decoder = NULL;
+#endif
}
if (st->video_decoder == NULL) {
spice_printerr("could not create a video decoder for codec %d", op->codec_type);
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel