Re: [PATCH spice-gtk v2 04/15] build-sys: drop gstvideo option, make it required

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,


On 1/9/19 12:09 PM, Frediano Ziglio wrote:
From: Marc-André Lureau<marcandre.lureau@xxxxxxxxxx>

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@xxxxxxxxxx>
---
  .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 */
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux Virtualization]     [Linux Virtualization]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]