[Spice-devel,spice-gtk,v1] build-sys: requires gstreamer 1.9 or above

Submitted by Victor Toso on Aug. 2, 2017, noon

Details

Message ID 20170802120002.32112-1-victortoso@redhat.com
State New
Headers show
Series "build-sys: requires gstreamer 1.9 or above" ( rev: 1 ) in Spice

Not browsing as part of any series.

Commit Message

Victor Toso Aug. 2, 2017, noon
From: Victor Toso <me@victortoso.com>

The recent work on spice-gtk with Gstreamer for video stream is
targeting hardware video acceleration. One of our main targets is
intel-based hardware and for that the VAAPI elements from
gstreamer-vaapi are necessary.

The gstreamer-vaapi project is maintained and released as part of
GStreamer since 1.8 [0] and the auto detection for VAAPI elements
based on client's hardware since 1.9 [1].

This patch removes checks for old GStreamer versions and request the
1.9.0 which is new enough for VAAPI. Note that we don't require the
elements itself to be present on build time.

Also, at this moment:
- Fedora 26 has 1.12
- Fedora 25 has 1.10
- Fedora 24 has 1.8 (EOL in August 8th)
- Debian 9 has 1.10
- RHEL 7 has 1.10

[0] See release notes from 1.8.0 (NEWS) 1abf889dddc75b4e4 (gstreamer)

[1] IRC chat on #gstreamer with Víctor M. Jáques
   toso | ceyusa: hey, is there a way to verify if we can hw
          decode using vaapi? some sort of api that translate
          what vainfo and validate with gstreamer-vaapi maybe?
   toso | s/what vainfo/the output of vainfo/
 ceyusa | toso: using gstreamer-vaapi 1.9, only the available
          decoder entries are registered
 ceyusa | so, using a gst-inspect-1.0 you'll see only the
          available decoders
   toso | ceyusa: and the available decoders takes in
          consideration my actual hw then?
 ceyusa | toso: yes
   toso | ceyusa: awesome

Signed-off-by: Victor Toso <victortoso@redhat.com>
---
 configure.ac              |  2 +-
 src/channel-display-gst.c | 36 ------------------------------------
 2 files changed, 1 insertion(+), 37 deletions(-)

Patch hide | download patch | download mbox

diff --git a/configure.ac b/configure.ac
index 8fd525b..2899217 100644
--- a/configure.ac
+++ b/configure.ac
@@ -271,7 +271,7 @@  AC_ARG_ENABLE([gstvideo],
   [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],
+         [gstreamer-1.0 > 1.9 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])
diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
index d7f47d1..0bfae0b 100644
--- a/src/channel-display-gst.c
+++ b/src/channel-display-gst.c
@@ -299,9 +299,7 @@  static gboolean handle_pipeline_message(GstBus *bus, GstMessage *msg, gpointer v
                                           gst_opts[decoder->base.codec_type].name);
         GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(decoder->pipeline),
                                   GST_DEBUG_GRAPH_SHOW_ALL
-#if GST_CHECK_VERSION(1,5,1)
                                     | GST_DEBUG_GRAPH_SHOW_FULL_PARAMS
-#endif
                                     | GST_DEBUG_GRAPH_SHOW_STATES,
                                     filename);
         g_free(filename);
@@ -314,7 +312,6 @@  static gboolean handle_pipeline_message(GstBus *bus, GstMessage *msg, gpointer v
     return TRUE;
 }
 
-#if GST_CHECK_VERSION(1,9,0)
 static void app_source_setup(GstElement *pipeline G_GNUC_UNUSED,
                              GstElement *source,
                              SpiceGstDecoder *decoder)
@@ -339,13 +336,11 @@  static void app_source_setup(GstElement *pipeline G_GNUC_UNUSED,
     gst_caps_unref(caps);
     decoder->appsrc = GST_APP_SRC(gst_object_ref(source));
 }
-#endif
 
 static gboolean create_pipeline(SpiceGstDecoder *decoder)
 {
     GstAppSinkCallbacks appsink_cbs = { NULL };
     GstBus *bus;
-#if GST_CHECK_VERSION(1,9,0)
     GstElement *playbin, *sink;
     SpiceGstPlayFlags flags;
     GstCaps *caps;
@@ -386,35 +381,6 @@  static gboolean create_pipeline(SpiceGstDecoder *decoder)
     g_warn_if_fail(decoder->appsrc == NULL);
     decoder->appsink = GST_APP_SINK(sink);
     decoder->pipeline = playbin;
-#else
-    gchar *desc;
-    GError *err = NULL;
-
-    /* - We schedule the frame display ourselves so set sync=false on appsink
-     *   so the pipeline decodes them as fast as possible. This will also
-     *   minimize the risk of frames getting lost when we rebuild the
-     *   pipeline.
-     * - Set max-bytes=0 on appsrc so it does not drop frames that may be
-     *   needed by those that follow.
-     */
-    desc = g_strdup_printf("appsrc name=src is-live=true format=time max-bytes=0 block=true "
-                           "caps=%s ! %s ! videoconvert ! appsink name=sink "
-                           "caps=video/x-raw,format=BGRx sync=false drop=false",
-                           gst_opts[decoder->base.codec_type].dec_caps,
-                           gst_opts[decoder->base.codec_type].dec_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"));
-#endif
 
     appsink_cbs.new_sample = new_sample;
     gst_app_sink_set_callbacks(decoder->appsink, &appsink_cbs, decoder, NULL);
@@ -542,13 +508,11 @@  static gboolean spice_gst_decoder_queue_frame(VideoDecoder *video_decoder,
         return FALSE;
     }
 
-#if GST_CHECK_VERSION(1,9,0)
     if (decoder->appsrc == NULL) {
         spice_warning("Error: Playbin has not yet initialized the Appsrc element");
         stream_dropped_frame_on_playback(decoder->base.stream);
         return TRUE;
     }
-#endif
 
     /* ref() the frame data for the buffer */
     frame->ref_data(frame->data_opaque);

Comments

----- Original Message -----
> From: Victor Toso <me@victortoso.com>
> 
> The recent work on spice-gtk with Gstreamer for video stream is
> targeting hardware video acceleration. One of our main targets is
> intel-based hardware and for that the VAAPI elements from
> gstreamer-vaapi are necessary.
> 
> The gstreamer-vaapi project is maintained and released as part of
> GStreamer since 1.8 [0] and the auto detection for VAAPI elements
> based on client's hardware since 1.9 [1].
> 
> This patch removes checks for old GStreamer versions and request the
> 1.9.0 which is new enough for VAAPI. Note that we don't require the
> elements itself to be present on build time.
> 
> Also, at this moment:
> - Fedora 26 has 1.12
> - Fedora 25 has 1.10
> - Fedora 24 has 1.8 (EOL in August 8th)
> - Debian 9 has 1.10
> - RHEL 7 has 1.10
> 
> [0] See release notes from 1.8.0 (NEWS) 1abf889dddc75b4e4 (gstreamer)
> 
> [1] IRC chat on #gstreamer with Víctor M. Jáques
>    toso | ceyusa: hey, is there a way to verify if we can hw
>           decode using vaapi? some sort of api that translate
>           what vainfo and validate with gstreamer-vaapi maybe?
>    toso | s/what vainfo/the output of vainfo/
>  ceyusa | toso: using gstreamer-vaapi 1.9, only the available
>           decoder entries are registered
>  ceyusa | so, using a gst-inspect-1.0 you'll see only the
>           available decoders
>    toso | ceyusa: and the available decoders takes in
>           consideration my actual hw then?
>  ceyusa | toso: yes
>    toso | ceyusa: awesome
> 
> Signed-off-by: Victor Toso <victortoso@redhat.com>

ack

> ---
>  configure.ac              |  2 +-
>  src/channel-display-gst.c | 36 ------------------------------------
>  2 files changed, 1 insertion(+), 37 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 8fd525b..2899217 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -271,7 +271,7 @@ AC_ARG_ENABLE([gstvideo],
>    [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],
> +         [gstreamer-1.0 > 1.9 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])
> diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
> index d7f47d1..0bfae0b 100644
> --- a/src/channel-display-gst.c
> +++ b/src/channel-display-gst.c
> @@ -299,9 +299,7 @@ static gboolean handle_pipeline_message(GstBus *bus,
> GstMessage *msg, gpointer v
>                                            gst_opts[decoder->base.codec_type].name);
>          GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(decoder->pipeline),
>                                    GST_DEBUG_GRAPH_SHOW_ALL
> -#if GST_CHECK_VERSION(1,5,1)
>                                      | GST_DEBUG_GRAPH_SHOW_FULL_PARAMS
> -#endif
>                                      | GST_DEBUG_GRAPH_SHOW_STATES,
>                                      filename);
>          g_free(filename);
> @@ -314,7 +312,6 @@ static gboolean handle_pipeline_message(GstBus *bus,
> GstMessage *msg, gpointer v
>      return TRUE;
>  }
>  
> -#if GST_CHECK_VERSION(1,9,0)
>  static void app_source_setup(GstElement *pipeline G_GNUC_UNUSED,
>                               GstElement *source,
>                               SpiceGstDecoder *decoder)
> @@ -339,13 +336,11 @@ static void app_source_setup(GstElement *pipeline
> G_GNUC_UNUSED,
>      gst_caps_unref(caps);
>      decoder->appsrc = GST_APP_SRC(gst_object_ref(source));
>  }
> -#endif
>  
>  static gboolean create_pipeline(SpiceGstDecoder *decoder)
>  {
>      GstAppSinkCallbacks appsink_cbs = { NULL };
>      GstBus *bus;
> -#if GST_CHECK_VERSION(1,9,0)
>      GstElement *playbin, *sink;
>      SpiceGstPlayFlags flags;
>      GstCaps *caps;
> @@ -386,35 +381,6 @@ static gboolean create_pipeline(SpiceGstDecoder
> *decoder)
>      g_warn_if_fail(decoder->appsrc == NULL);
>      decoder->appsink = GST_APP_SINK(sink);
>      decoder->pipeline = playbin;
> -#else
> -    gchar *desc;
> -    GError *err = NULL;
> -
> -    /* - We schedule the frame display ourselves so set sync=false on
> appsink
> -     *   so the pipeline decodes them as fast as possible. This will also
> -     *   minimize the risk of frames getting lost when we rebuild the
> -     *   pipeline.
> -     * - Set max-bytes=0 on appsrc so it does not drop frames that may be
> -     *   needed by those that follow.
> -     */
> -    desc = g_strdup_printf("appsrc name=src is-live=true format=time
> max-bytes=0 block=true "
> -                           "caps=%s ! %s ! videoconvert ! appsink name=sink
> "
> -                           "caps=video/x-raw,format=BGRx sync=false
> drop=false",
> -                           gst_opts[decoder->base.codec_type].dec_caps,
> -                           gst_opts[decoder->base.codec_type].dec_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"));
> -#endif
>  
>      appsink_cbs.new_sample = new_sample;
>      gst_app_sink_set_callbacks(decoder->appsink, &appsink_cbs, decoder,
>      NULL);
> @@ -542,13 +508,11 @@ static gboolean
> spice_gst_decoder_queue_frame(VideoDecoder *video_decoder,
>          return FALSE;
>      }
>  
> -#if GST_CHECK_VERSION(1,9,0)
>      if (decoder->appsrc == NULL) {
>          spice_warning("Error: Playbin has not yet initialized the Appsrc
>          element");
>          stream_dropped_frame_on_playback(decoder->base.stream);
>          return TRUE;
>      }
> -#endif
>  
>      /* ref() the frame data for the buffer */
>      frame->ref_data(frame->data_opaque);
> --
> 2.13.0
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
>
On Wed, Aug 02, 2017 at 02:00:02PM +0200, Victor Toso wrote:
> From: Victor Toso <me@victortoso.com>
> 
> The recent work on spice-gtk with Gstreamer for video stream is
> targeting hardware video acceleration. One of our main targets is
> intel-based hardware and for that the VAAPI elements from
> gstreamer-vaapi are necessary.
> 
> The gstreamer-vaapi project is maintained and released as part of
> GStreamer since 1.8 [0] and the auto detection for VAAPI elements
> based on client's hardware since 1.9 [1].
> 
> This patch removes checks for old GStreamer versions and request the
> 1.9.0 which is new enough for VAAPI. Note that we don't require the
> elements itself to be present on build time.
> 
> Also, at this moment:
> - Fedora 26 has 1.12
> - Fedora 25 has 1.10
> - Fedora 24 has 1.8 (EOL in August 8th)
> - Debian 9 has 1.10
> - RHEL 7 has 1.10

RHEL 7.4 which was released yesterday has 1.10, RHEL 7.3 has GStreamer
1.4 if I'm not mistaken. I'm wondering if we should follow the same policy
as libosinfo, compatibility with 2 minor releases of long term support
distros, compatibility with 2 major releases of distros with short
support times.

Christophe
On Wed, Aug 02, 2017 at 06:29:11PM +0200, Christophe Fergeau wrote:
> On Wed, Aug 02, 2017 at 02:00:02PM +0200, Victor Toso wrote:
> > From: Victor Toso <me@victortoso.com>
> > 
> > The recent work on spice-gtk with Gstreamer for video stream is
> > targeting hardware video acceleration. One of our main targets is
> > intel-based hardware and for that the VAAPI elements from
> > gstreamer-vaapi are necessary.
> > 
> > The gstreamer-vaapi project is maintained and released as part of
> > GStreamer since 1.8 [0] and the auto detection for VAAPI elements
> > based on client's hardware since 1.9 [1].
> > 
> > This patch removes checks for old GStreamer versions and request the
> > 1.9.0 which is new enough for VAAPI. Note that we don't require the
> > elements itself to be present on build time.
> > 
> > Also, at this moment:
> > - Fedora 26 has 1.12
> > - Fedora 25 has 1.10
> > - Fedora 24 has 1.8 (EOL in August 8th)
> > - Debian 9 has 1.10
> > - RHEL 7 has 1.10
> 
> RHEL 7.4 which was released yesterday has 1.10, RHEL 7.3 has GStreamer
> 1.4 if I'm not mistaken. I'm wondering if we should follow the same policy
> as libosinfo, compatibility with 2 minor releases of long term support

s/libosinfo/libvirt-glib/

Christophe
> 
> On Wed, Aug 02, 2017 at 02:00:02PM +0200, Victor Toso wrote:
> > From: Victor Toso <me@victortoso.com>
> > 
> > The recent work on spice-gtk with Gstreamer for video stream is
> > targeting hardware video acceleration. One of our main targets is
> > intel-based hardware and for that the VAAPI elements from
> > gstreamer-vaapi are necessary.
> > 
> > The gstreamer-vaapi project is maintained and released as part of
> > GStreamer since 1.8 [0] and the auto detection for VAAPI elements
> > based on client's hardware since 1.9 [1].
> > 
> > This patch removes checks for old GStreamer versions and request the
> > 1.9.0 which is new enough for VAAPI. Note that we don't require the
> > elements itself to be present on build time.
> > 
> > Also, at this moment:
> > - Fedora 26 has 1.12
> > - Fedora 25 has 1.10
> > - Fedora 24 has 1.8 (EOL in August 8th)
> > - Debian 9 has 1.10
> > - RHEL 7 has 1.10
> 
> RHEL 7.4 which was released yesterday has 1.10, RHEL 7.3 has GStreamer
> 1.4 if I'm not mistaken. I'm wondering if we should follow the same policy
> as libosinfo, compatibility with 2 minor releases of long term support
> distros, compatibility with 2 major releases of distros with short
> support times.
> 
> Christophe
> 

https://access.redhat.com/support/policy/updates/errata.
If we stick to EOS we are screwed :-)

So you are suggesting to wait 7.5 to remove 7.3 support, right?

Frediano
On Wed, Aug 02, 2017 at 02:19:36PM -0400, Frediano Ziglio wrote:
> >
> > On Wed, Aug 02, 2017 at 02:00:02PM +0200, Victor Toso wrote:
> > > From: Victor Toso <me@victortoso.com>
> > >
> > > The recent work on spice-gtk with Gstreamer for video stream is
> > > targeting hardware video acceleration. One of our main targets is
> > > intel-based hardware and for that the VAAPI elements from
> > > gstreamer-vaapi are necessary.
> > >
> > > The gstreamer-vaapi project is maintained and released as part of
> > > GStreamer since 1.8 [0] and the auto detection for VAAPI elements
> > > based on client's hardware since 1.9 [1].
> > >
> > > This patch removes checks for old GStreamer versions and request
> > > the 1.9.0 which is new enough for VAAPI. Note that we don't
> > > require the elements itself to be present on build time.
> > >
> > > Also, at this moment:
> > > - Fedora 26 has 1.12
> > > - Fedora 25 has 1.10
> > > - Fedora 24 has 1.8 (EOL in August 8th)
> > > - Debian 9 has 1.10
> > > - RHEL 7 has 1.10
> >
> > RHEL 7.4 which was released yesterday has 1.10, RHEL 7.3 has
> > GStreamer 1.4 if I'm not mistaken. I'm wondering if we should follow
> > the same policy as libvirt-glib, compatibility with 2 minor releases
> > of long term support distros, compatibility with 2 major releases of
> > distros with short support times.
> >
> > Christophe
> >
>
> https://access.redhat.com/support/policy/updates/errata.
> If we stick to EOS we are screwed :-)
>
> So you are suggesting to wait 7.5 to remove 7.3 support, right?
>
> Frediano

Hmm, might be a good approach indeed.

RHEL 7.5 release should be around March/2018 but hopefully we can have a
spice-gtk release before that. So if we agree that upstream code to be
compatible with 7.4 and 7.3 till 7.5 is released, this patch should be
postponed indeed.

(I'm glad that I doubled checked for 7.3 glib before bumping it!)
On Wed, Aug 02, 2017 at 02:19:36PM -0400, Frediano Ziglio wrote:
> > 
> > On Wed, Aug 02, 2017 at 02:00:02PM +0200, Victor Toso wrote:
> > > From: Victor Toso <me@victortoso.com>
> > > 
> > > The recent work on spice-gtk with Gstreamer for video stream is
> > > targeting hardware video acceleration. One of our main targets is
> > > intel-based hardware and for that the VAAPI elements from
> > > gstreamer-vaapi are necessary.
> > > 
> > > The gstreamer-vaapi project is maintained and released as part of
> > > GStreamer since 1.8 [0] and the auto detection for VAAPI elements
> > > based on client's hardware since 1.9 [1].
> > > 
> > > This patch removes checks for old GStreamer versions and request the
> > > 1.9.0 which is new enough for VAAPI. Note that we don't require the
> > > elements itself to be present on build time.
> > > 
> > > Also, at this moment:
> > > - Fedora 26 has 1.12
> > > - Fedora 25 has 1.10
> > > - Fedora 24 has 1.8 (EOL in August 8th)
> > > - Debian 9 has 1.10
> > > - RHEL 7 has 1.10
> > 
> > RHEL 7.4 which was released yesterday has 1.10, RHEL 7.3 has GStreamer
> > 1.4 if I'm not mistaken. I'm wondering if we should follow the same policy
> > as libosinfo, compatibility with 2 minor releases of long term support
> > distros, compatibility with 2 major releases of distros with short
> > support times.
> > 
> > Christophe
> > 
> 
> https://access.redhat.com/support/policy/updates/errata.
> If we stick to EOS we are screwed :-)
> 
> So you are suggesting to wait 7.5 to remove 7.3 support, right?

Yes. Or rather, I'd prefer we avoid dropping 7.3 support the day after
7.4 is released, and this is one way of achieving that.

Christophe