[Spice-devel] [PATCH spice-gtk v2 04/15] build-sys: drop gstvideo option, make it required
Snir Sheriber
ssheribe at redhat.com
Sun Jan 13 15:07:00 UTC 2019
Hi,
On 1/9/19 12:09 PM, Frediano Ziglio wrote:
> From: Marc-André Lureau<marcandre.lureau at redhat.com>
>
> GStreamer is being increasingly used by spice-gtk. Let's make it a
> core requirement.
So if we are making the gst-video stuff required too, won't be worth to
make the builtin mjpeg
option a feature and avoid the mandatory dependency? Or it's too
fragile\early\wrong to assume
gstreamer can always handle mjpeg?
Other than this looks good to me too (also the gstaudio one), maybe
worth squash the
gstvideo+gstaudio? (too big?)
Snir.
> Signed-off-by: Marc-André Lureau<marcandre.lureau at redhat.com>
> ---
> .gitlab-ci.yml | 2 --
> configure.ac | 41 ++++++++++++++------------------------
> meson.build | 17 +---------------
> meson_options.txt | 5 -----
> src/Makefile.am | 7 +------
> src/channel-display-priv.h | 10 +---------
> src/channel-display.c | 7 -------
> src/meson.build | 5 +----
> src/spice-widget-priv.h | 4 ----
> src/spice-widget.c | 13 +++---------
> 10 files changed, 22 insertions(+), 89 deletions(-)
>
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index 78b339e7..c956adda 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -32,7 +32,6 @@ makecheck_simple:
> script:
> - ./autogen.sh --enable-static
> --enable-lz4=no
> - --enable-gstvideo=no
> --enable-webdav=no
> --with-sasl=no
> --with-coroutine=auto
> @@ -46,7 +45,6 @@ makecheck_simple:
> makecheck_simple-meson:
> script:
> - meson build -Dauto_features=disabled
> - -Dgstvideo=false
> -Dusbredir=false
> -Ddbus=false || (cat build/meson-logs/meson-log.txt && exit 1)
> - ninja -C build
> diff --git a/configure.ac b/configure.ac
> index 85827b1d..ea4e8ff2 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -221,28 +221,21 @@ SPICE_CHECK_GSTREAMER(GSTAUDIO, 1.0,
> ],
> [AC_MSG_ERROR([Required GStreamer packages missing])])
>
> -AC_ARG_ENABLE([gstvideo],
> - AS_HELP_STRING([--enable-gstvideo=@<:@auto/yes/no@:>@],
> - [Enable GStreamer video support @<:@default=auto@:>@]),
> - [],
> - [enable_gstvideo="auto"])
> -AS_IF([test "x$enable_gstvideo" != "xno"],
> - [SPICE_CHECK_GSTREAMER(GSTVIDEO, 1.0,
> - [gstreamer-1.0 gstreamer-base-1.0 gstreamer-app-1.0 gstreamer-video-1.0],
> - [missing_gstreamer_elements=""
> - SPICE_CHECK_GSTREAMER_ELEMENTS($GST_INSPECT_1_0, [gst-plugins-base 1.0], [appsrc videoconvert appsink])
> - SPICE_CHECK_GSTREAMER_ELEMENTS($GST_INSPECT_1_0, [gst-plugins-good 1.0], [jpegdec vp8dec vp9dec])
> - SPICE_CHECK_GSTREAMER_ELEMENTS($GST_INSPECT_1_0, [gst-plugins-bad 1.0], [h264parse h265parse])
> - SPICE_CHECK_GSTREAMER_ELEMENTS($GST_INSPECT_1_0, [gstreamer-libav 1.0], [avdec_h264 avdec_h265])
> - AS_IF([test x"$missing_gstreamer_elements" = "xyes"],
> - SPICE_WARNING([The GStreamer video decoder can be built but may not work.]))
> - ],
> - [AS_IF([test "x$enable_gstvideo" = "xyes"],
> - AC_MSG_ERROR([GStreamer 1.0 video requested but not found]))
> - ])
> - ], [have_gstvideo="no"]
> -)
> -AM_CONDITIONAL([HAVE_GSTVIDEO], [test "x$have_gstvideo" = "xyes"])
> +SPICE_CHECK_GSTREAMER(GSTVIDEO, 1.0,
> + [gstreamer-1.0 gstreamer-base-1.0 gstreamer-app-1.0 gstreamer-video-1.0],
> + [missing_gstreamer_elements=""
> + SPICE_CHECK_GSTREAMER_ELEMENTS($GST_INSPECT_1_0,
> + [gst-plugins-base 1.0], [appsrc videoconvert appsink])
> + SPICE_CHECK_GSTREAMER_ELEMENTS($GST_INSPECT_1_0,
> + [gst-plugins-good 1.0], [jpegdec vp8dec vp9dec])
> + SPICE_CHECK_GSTREAMER_ELEMENTS($GST_INSPECT_1_0,
> + [gst-plugins-bad 1.0], [h264parse h265parse])
> + SPICE_CHECK_GSTREAMER_ELEMENTS($GST_INSPECT_1_0,
> + [gstreamer-libav 1.0], [avdec_h264 avdec_h265])
> + AS_IF([test x"$missing_gstreamer_elements" = "xyes"],
> + SPICE_WARNING([The GStreamer video decoder can be built but may not work.]))
> + ],
> + [AC_MSG_ERROR([Required GStreamer packages missing])])
>
> AC_ARG_ENABLE([builtin-mjpeg],
> AS_HELP_STRING([--enable-builtin-mjpeg], [Enable the builtin mjpeg video decoder @<:@default=yes@:>@]),
> @@ -252,9 +245,6 @@ AS_IF([test "x$enable_builtin_mjpeg" = "xyes"],
> [AC_DEFINE([HAVE_BUILTIN_MJPEG], 1, [Use the builtin mjpeg decoder?])])
> AM_CONDITIONAL(HAVE_BUILTIN_MJPEG, [test "x$enable_builtin_mjpeg" != "xno"])
>
> -AS_IF([test "x$enable_builtin_mjpeg$enable_gstvideo" = "xnono"],
> - [SPICE_WARNING([No builtin MJPEG or GStreamer decoder, video will not be streamed])])
> -
> AC_CHECK_LIB(jpeg, jpeg_destroy_decompress,
> AC_MSG_CHECKING([for jpeglib.h])
> AC_TRY_CPP(
> @@ -547,7 +537,6 @@ AC_MSG_NOTICE([
> Gtk: ${with_gtk}
> Coroutine: ${with_coroutine}
> PulseAudio: ${enable_pulse}
> - GStreamer Video: ${have_gstvideo}
> SASL support: ${have_sasl}
> Smartcard support: ${have_smartcard}
> USB redirection support: ${have_usbredir} ${with_usbredir_hotplug}
> diff --git a/meson.build b/meson.build
> index 4ade991e..e1e5e919 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -161,22 +161,11 @@ if d.found()
> spice_gtk_has_pulse = true
> endif
>
> -deps = ['gstreamer-1.0', 'gstreamer-base-1.0', 'gstreamer-app-1.0', 'gstreamer-audio-1.0']
> +deps = ['gstreamer-1.0', 'gstreamer-base-1.0', 'gstreamer-app-1.0', 'gstreamer-audio-1.0', 'gstreamer-video-1.0']
> foreach dep : deps
> spice_glib_deps += dependency(dep)
> endforeach
>
> -# gstvideo
> -spice_gtk_has_gstvideo = false
> -if get_option('gstvideo')
> - deps = ['gstreamer-video-1.0']
> - foreach dep : deps
> - spice_glib_deps += dependency(dep)
> - endforeach
> - spice_gtk_config_data.set('HAVE_GSTVIDEO', '1')
> - spice_gtk_has_gstvideo = true
> -endif
> -
> # builtin-mjpeg
> spice_gtk_has_builtin_mjpeg = false
> if get_option('builtin-mjpeg')
> @@ -184,10 +173,6 @@ if get_option('builtin-mjpeg')
> spice_gtk_has_builtin_mjpeg = true
> endif
>
> -if not spice_gtk_has_gstvideo and not spice_gtk_has_builtin_mjpeg
> - warning('No builtin MJPEG or GStreamer decoder, video will not be streamed')
> -endif
> -
> # usbredir
> spice_gtk_has_usbredir = false
> if get_option('usbredir')
> diff --git a/meson_options.txt b/meson_options.txt
> index 08efaceb..8ac6c684 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -10,11 +10,6 @@ option('pulse',
> type : 'feature',
> description: 'Enable the PulseAudio backend')
>
> -option('gstvideo',
> - type : 'boolean',
> - value : true,
> - description : 'Enable GStreamer video support')
> -
> option('builtin-mjpeg',
> type : 'boolean',
> value : true,
> diff --git a/src/Makefile.am b/src/Makefile.am
> index f8335ca7..b50c4262 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -241,6 +241,7 @@ libspice_client_glib_impl_la_SOURCES = \
> channel-webdav.c \
> channel-cursor.c \
> channel-display.c \
> + channel-display-gst.c \
> channel-display-priv.h \
> channel-inputs.c \
> channel-main.c \
> @@ -326,12 +327,6 @@ libspice_client_glib_impl_la_SOURCES += \
> $(NULL)
> endif
>
> -if HAVE_GSTVIDEO
> -libspice_client_glib_impl_la_SOURCES += \
> - channel-display-gst.c \
> - $(NULL)
> -endif
> -
> if WITH_PHODAV
> libspice_client_glib_impl_la_SOURCES += \
> giopipe.c \
> diff --git a/src/channel-display-priv.h b/src/channel-display-priv.h
> index 72ad90ff..6c6b6c40 100644
> --- a/src/channel-display-priv.h
> +++ b/src/channel-display-priv.h
> @@ -31,9 +31,7 @@
> #include "common/quic.h"
> #include "common/rop3.h"
>
> -#ifdef HAVE_GSTVIDEO
> -#include "gst/gst.h"
> -#endif
> +#include <gst/gst.h>
>
> G_BEGIN_DECLS
>
> @@ -90,12 +88,8 @@ struct VideoDecoder {
> #ifdef HAVE_BUILTIN_MJPEG
> VideoDecoder* create_mjpeg_decoder(int codec_type, display_stream *stream);
> #endif
> -#ifdef HAVE_GSTVIDEO
> VideoDecoder* create_gstreamer_decoder(int codec_type, display_stream *stream);
> gboolean gstvideo_has_codec(int codec_type);
> -#else
> -# define gstvideo_has_codec(codec_type) FALSE
> -#endif
>
>
> typedef struct display_surface {
> @@ -205,9 +199,7 @@ void stream_dropped_frame_on_playback(display_stream *st);
> #define SPICE_UNKNOWN_STRIDE 0
> void stream_display_frame(display_stream *st, SpiceFrame *frame, uint32_t width, uint32_t height, int stride, uint8_t* data);
> guintptr get_window_handle(display_stream *st);
> -#ifdef HAVE_GSTVIDEO
> gboolean hand_pipeline_to_widget(display_stream *st, GstPipeline *pipeline);
> -#endif
>
>
> G_END_DECLS
> diff --git a/src/channel-display.c b/src/channel-display.c
> index 56799c04..3dfd721e 100644
> --- a/src/channel-display.c
> +++ b/src/channel-display.c
> @@ -29,9 +29,6 @@
> #include "spice-session-priv.h"
> #include "channel-display-priv.h"
> #include "decode.h"
> -#ifdef HAVE_GSTVIDEO
> -#include "gst/gst.h"
> -#endif
>
> /**
> * SECTION:channel-display
> @@ -1295,9 +1292,7 @@ static display_stream *display_stream_create(SpiceChannel *channel,
> break;
> #endif
> default:
> -#ifdef HAVE_GSTVIDEO
> st->video_decoder = create_gstreamer_decoder(codec_type, st);
> -#endif
> break;
> }
> if (st->video_decoder == NULL) {
> @@ -1420,7 +1415,6 @@ void stream_display_frame(display_stream *st, SpiceFrame *frame,
> }
> }
>
> -#ifdef HAVE_GSTVIDEO
> G_GNUC_INTERNAL
> gboolean hand_pipeline_to_widget(display_stream *st, GstPipeline *pipeline)
> {
> @@ -1432,7 +1426,6 @@ gboolean hand_pipeline_to_widget(display_stream *st, GstPipeline *pipeline)
> }
> return res;
> }
> -#endif
>
> /* after a sequence of 3 drops, push a report to the server, even
> * if the report window is bigger */
> diff --git a/src/meson.build b/src/meson.build
> index 5754f1fc..d47d58dc 100644
> --- a/src/meson.build
> +++ b/src/meson.build
> @@ -88,6 +88,7 @@ spice_client_glib_sources = [
> 'bio-gio.c',
> 'bio-gio.h',
> 'channel-base.c',
> + 'channel-display-gst.c',
> 'channel-display-priv.h',
> 'channel-playback-priv.h',
> 'channel-usbredir-priv.h',
> @@ -128,10 +129,6 @@ if spice_gtk_has_builtin_mjpeg
> spice_client_glib_sources += 'channel-display-mjpeg.c'
> endif
>
> -if spice_gtk_has_gstvideo
> - spice_client_glib_sources += 'channel-display-gst.c'
> -endif
> -
> if spice_gtk_has_polkit
> spice_client_glib_sources += ['usb-acl-helper.c',
> 'usb-acl-helper.h']
> diff --git a/src/spice-widget-priv.h b/src/spice-widget-priv.h
> index 54fe177a..65eb4047 100644
> --- a/src/spice-widget-priv.h
> +++ b/src/spice-widget-priv.h
> @@ -32,9 +32,7 @@
> #include "spice-common.h"
> #include "spice-gtk-session.h"
>
> -#ifdef HAVE_GSTVIDEO
> #include <gst/video/videooverlay.h>
> -#endif
>
> G_BEGIN_DECLS
>
> @@ -154,9 +152,7 @@ struct _SpiceDisplayPrivate {
> } egl;
> #endif // HAVE_EGL
> double scroll_delta_y;
> -#ifdef HAVE_GSTVIDEO
> GWeakRef overlay_weak_ref;
> -#endif
> };
>
> int spice_cairo_image_create (SpiceDisplay *display);
> diff --git a/src/spice-widget.c b/src/spice-widget.c
> index af0301c1..c71cc534 100644
> --- a/src/spice-widget.c
> +++ b/src/spice-widget.c
> @@ -121,10 +121,8 @@ static void size_allocate(GtkWidget *widget, GtkAllocation *conf, gpointer data)
> static gboolean draw_event(GtkWidget *widget, cairo_t *cr, gpointer data);
> static void update_size_request(SpiceDisplay *display);
> static GdkDevice *spice_gdk_window_get_pointing_device(GdkWindow *window);
> -#ifdef HAVE_GSTVIDEO
> static void gst_size_allocate(GtkWidget *widget, GdkRectangle *a, gpointer data);
> static gboolean gst_draw_event(GtkWidget *widget, cairo_t *cr, gpointer data);
> -#endif
>
> /* ---------------------------------------------------------------- */
>
> @@ -657,14 +655,13 @@ static void spice_display_init(SpiceDisplay *display)
> NULL);
> gtk_stack_add_named(d->stack, area, "gl-area");
> #endif
> -#ifdef HAVE_GSTVIDEO
> area = gtk_drawing_area_new();
> gtk_stack_add_named(d->stack, area, "gst-area");
> g_object_connect(area,
> "signal::draw", gst_draw_event, display,
> "signal::size-allocate", gst_size_allocate, display,
> NULL);
> -#endif
> +
> d->label = gtk_label_new(NULL);
> gtk_label_set_selectable(GTK_LABEL(d->label), true);
> gtk_stack_add_named(d->stack, d->label, "label");
> @@ -2140,9 +2137,7 @@ static void unrealize(GtkWidget *widget)
> spice_egl_unrealize_display(display);
> }
> #endif
> -#ifdef HAVE_GSTVIDEO
> g_weak_ref_set(&display->priv->overlay_weak_ref, NULL);
> -#endif
>
> GTK_WIDGET_CLASS(spice_display_parent_class)->unrealize(widget);
> }
> @@ -2570,7 +2565,7 @@ static void queue_draw_area(SpiceDisplay *display, gint x, gint y,
> x, y, width, height);
> }
>
> -#if defined(HAVE_GSTVIDEO) && defined(GDK_WINDOWING_X11)
> +#if defined(GDK_WINDOWING_X11)
> static void gst_sync_bus_call(GstBus *bus, GstMessage *msg, SpiceDisplay *display)
> {
> switch(GST_MESSAGE_TYPE(msg)) {
> @@ -2599,7 +2594,6 @@ static void gst_sync_bus_call(GstBus *bus, GstMessage *msg, SpiceDisplay *displa
> }
> #endif
>
> -#ifdef HAVE_GSTVIDEO
> static gboolean gst_draw_event(GtkWidget *widget, cairo_t *cr, gpointer data)
> {
> SpiceDisplay *display = SPICE_DISPLAY(data);
> @@ -2629,14 +2623,13 @@ static void gst_size_allocate(GtkWidget *widget, GdkRectangle *a, gpointer data)
> gst_object_unref(overlay);
> }
> }
> -#endif
>
> /* This callback should pass to the widget a pointer of the pipeline
> * so that we can set pipeline and overlay related calls from here.
> */
> static gboolean set_overlay(SpiceChannel *channel, void* pipeline_ptr, SpiceDisplay *display)
> {
> -#if defined(HAVE_GSTVIDEO) && defined(GDK_WINDOWING_X11)
> +#if defined(GDK_WINDOWING_X11)
> SpiceDisplayPrivate *d = display->priv;
>
> /* GstVideoOverlay is currently used only under x */
More information about the Spice-devel
mailing list