[Spice-commits] src/channel-display-gst.c src/channel-display-priv.h

[Victor Toso de Carvalho]

 src/channel-display-gst.c  |    2 +-
 src/channel-display-priv.h |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

New commits:
commit c9129ed202b00f4a74ea7a55de19150194257e77
Author: Victor Toso <me at victortoso.com>
Date:   Thu Jul 13 14:33:48 2017 +0200

    display-gst: Improve h264 elements filtering
    
    This patch fixes the avdec_h264 element not being present on
    gstvideo_has_codec() which get all decoder elements from GstRegistry
    and filter them on our GstCaps in order to get the ones for given
    codec.
    
    The issue is around the filtering. The current GstCaps for h264 is not
    consider a subset of avdec_h264's capabilities and that will filter
    this element out of the list.
    
    The proposed solution for that is to set `subsetonly` parameter from
    gst_element_factory_list_filter() to false.
    
    While at it, the cap "stream-format=byte-stream" is less useful now
    because it isn't needed to play the stream - see 6fe88871240c53b8
    
    In my system, our debug shows:
    .. gstvideo_debug_available_decoders: From 228 video decoder elements,
    - 4 can handle caps   image/jpeg: jpegdec, nvdec, avdec_mjpeg, vaapijpegdec
    - 3 can handle caps  video/x-vp8: vaapidecodebin, vp8dec, avdec_vp8
    - 4 can handle caps video/x-h264: vaapidecodebin, avdec_h264, nvdec, vaapih264dec
    - 3 can handle caps  video/x-vp9: vaapidecodebin, vp9dec, avdec_vp9
    
    IRC log on #gstreamer around this issue:
    
    toso  Hi, I'm wondering if I'm getting the documentation wrong about
          the meaning of gst_caps_is_subset()... isn't it "video/x-h264,
          stream-format=3D(string)byte-stream" a subset of "video/x-h264,
          alignment=3D(string)au, stream-format=3D(string){ avc, byte-stream }"=
     ?
    __tim toso, no, because it's missing the alignment field
    __tim which means alignment could be anything
    __tim which means it  might not be au
    toso  that's a bit confusing, no?
    __tim it's not a strict subset :)
    toso  I really thought it would be a subset in this case.. meaning, we
          don't care about the aligment on _is_subset
    __tim you can do can_intersect() if you don't care
    slomo it's an actual subset if you define the abscence of a field as
          "the field can have every possible value"
    toso  __tim: problem is that it isn't a subset on calling
          gst_element_factory_list_filter()
    toso  slomo: yeah, that was my assumption on the meaning of subset
    toso  so, just to confirm, this is the expected behavior and not a
          bug, right? :)
    slomo yes
    __tim slomo, foo, bar=3D{1,2} is not a subset of  foo, bar=3D1  is it?
    slomo but if something does not work as expect for you, that might be
          a bug :)
    slomo __tim: correct
    slomo __tim: but foo, bar=3D1 is a subset of foo, bar=3D{1,2}
    __tim yes
    slomo __tim: and foo, bar=3D1 is a subset of foo
    slomo __tim: and foo is not a subset of foo, bar=3D1
    __tim yes. yes.
    __tim andn the case we're talking about is basically  "foo" + "foo,
          bar=3D1"
    slomo yes, the first is a superset of the second there
    slomo toso: if something does not work for you that might also be a
          bug of course ;)
    __tim ok, so not a subset.
    __tim then we're on the same page I think :)
    toso  slomo: right, my point was that I would like to list the h264
          decoders that are available and I was using `video/x-h264,
          stream-format=3Dbyte-stream` as input for
          gst_element_factory_list_filter() .. and avdec_h264 is the one
          not showing because it needs the aligment
    toso  but as I got it wrong, I'll rethink on how to fix it
    slomo toso: pass FALSE as the last argument
    slomo and you can just use 'video/x-h264'
    slomo and with gst_element_factory_list_get_elements() you can make
          sure to get only a list of video decoders to begin with
    toso  just gst_element_factory_list_get_elements() gives me like 220
          elements
    toso  using _type_decoder | _type_media_video | _type_media_image
    slomo is that a problem?
    toso  trying to reduce that to actual decoding elements
    slomo they should be all actual video decoders
    slomo and with gst_element_factory_list_filter(list, 'video/x-h264',
          GST_PAD_SINK, FALSE) you can then filter out all non-h264 ones
          dv_       is it generally ok if an audio decoder messes with the
          PTS/durations?
    toso  right, let me try that then
    toso  slomo, __tim many thanks :)
    
    Signed-off-by: Victor Toso <victortoso at redhat.com>
    Acked-by: Marc-André Lureau <marcandre.lureau at redhat.com>

diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
index 338e7d2..d7f47d1 100644
--- a/src/channel-display-gst.c
+++ b/src/channel-display-gst.c
@@ -656,7 +656,7 @@ gboolean gstvideo_has_codec(int codec_type)
     }
 
     caps = gst_caps_from_string(gst_opts[codec_type].dec_caps);
-    codec_decoders = gst_element_factory_list_filter(all_decoders, caps, GST_PAD_SINK, TRUE);
+    codec_decoders = gst_element_factory_list_filter(all_decoders, caps, GST_PAD_SINK, FALSE);
     gst_caps_unref(caps);
 
     if (codec_decoders == NULL) {
diff --git a/src/channel-display-priv.h b/src/channel-display-priv.h
index a0a420c..1389868 100644
--- a/src/channel-display-priv.h
+++ b/src/channel-display-priv.h
@@ -181,7 +181,7 @@ static const struct {
      * (hardcoded in spice-server), let's add it here to avoid the warning.
      */
     { SPICE_DISPLAY_CAP_CODEC_H264, "h264",
-      "h264parse ! avdec_h264", "video/x-h264,stream-format=byte-stream" },
+      "h264parse ! avdec_h264", "video/x-h264" },
 
     /* SPICE_VIDEO_CODEC_TYPE_VP9 */
     { SPICE_DISPLAY_CAP_CODEC_VP9, "vp9",