After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 402141 - gst_element_factory_can_{sink,src}_caps seems to be broken
gst_element_factory_can_{sink,src}_caps seems to be broken
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal enhancement
: 0.10.33
Assigned To: GStreamer Maintainers
GStreamer Maintainers
: 622272 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2007-01-29 16:08 UTC by Stefan Sauer (gstreamer, gtkdoc dev)
Modified: 2011-03-10 18:02 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
mpeg4videoparse: add unit test for element factory bug (3.38 KB, patch)
2010-06-21 12:15 UTC, Thijs Vermeir
rejected Details | Review
gstutils: gst_element_factory_can_{sink,src}_caps check for any caps (3.61 KB, patch)
2010-06-22 08:01 UTC, Thijs Vermeir
none Details | Review
gstutils: replace gst_element_factory_can_{sink,src}_caps (6.06 KB, patch)
2010-06-24 07:01 UTC, Thijs Vermeir
needs-work Details | Review
gstutils: replace gst_element_factory_can_{sink,src}_caps (7.26 KB, patch)
2011-03-08 08:23 UTC, Stefan Sauer (gstreamer, gtkdoc dev)
none Details | Review
gstutils: replace gst_element_factory_can_{sink,src}_caps (8.05 KB, patch)
2011-03-08 14:00 UTC, Stefan Sauer (gstreamer, gtkdoc dev)
committed Details | Review
add tests for new api (1.93 KB, patch)
2011-03-08 14:00 UTC, Stefan Sauer (gstreamer, gtkdoc dev)
committed Details | Review

Description Stefan Sauer (gstreamer, gtkdoc dev) 2007-01-29 16:08:24 UTC
this code does not what one would expect:

  GstCaps *int_caps=gst_caps_from_string("audio/x-raw-int");
  GstElementFactory * const factory;

  gst_element_factory_can_sink_caps(factory,int_caps);

with factory e.g. beeing alsasink.
Comment 1 Stefan Sauer (gstreamer, gtkdoc dev) 2007-01-29 19:07:22 UTC
Isn't is more that one wants to check if a element support a specifiy media-type.

gboolean gst_element_factory_can_sink_media_type (factory,media_type);

as a workaround for the above, one can do:

GstCaps *int_caps=gst_caps_from_string(GST_AUDIO_INT_PAD_TEMPLATE_CAPS);
Comment 2 René Stadler 2007-01-29 21:50:52 UTC
Your example is flawed; what you describe as workaround is the correct solution for the problem you face.
Comment 3 Stefan Sauer (gstreamer, gtkdoc dev) 2007-01-30 09:46:53 UTC
but it still doesn't work, as one can never know all parts of the caps for sure. I work on the gst_element_factory_can_sink_media_type() implementation right now and propose that as an alternative. Does this sound like a good idea?
Comment 4 René Stadler 2007-01-30 14:38:02 UTC
If a caps structure contains a field I don't know about, all assumptions about the data the caps are attached to become void.

Is there a better example use case?
Comment 5 Stefan Sauer (gstreamer, gtkdoc dev) 2007-01-30 15:03:02 UTC
my usecase is to get a list of audiosinks from the registry and then filter them by those that can play raw-audio.

As I will stick an audioconvert element infront of them, I might also grab the src-caps from audioconvert and probe:
  gst_element_factory_can_sink_caps(audio_sink_facory, audioconvert_src_caps)
Comment 6 René Stadler 2007-02-01 13:10:13 UTC
That sounds like the right thing to do.  Not a bug, then?
Comment 7 Thijs Vermeir 2010-06-21 12:14:17 UTC
*** Bug 622272 has been marked as a duplicate of this bug. ***
Comment 8 Thijs Vermeir 2010-06-21 12:15:37 UTC
Created attachment 164212 [details] [review]
mpeg4videoparse: add unit test for element factory bug
Comment 9 Benjamin Otte (Company) 2010-06-21 20:01:38 UTC
I suppose the code should check if the element factory and the provided caps have an intersection and not if the provided caps are a subset of the element factory caps, because that's what I'd have thought intuitively.

But I'm pretty sure this will cause subtle bugs in code that uses these functions that expect the current behavior, so I'm tempted to say that this is a WONTFIX - unless all users of the function expect this new behavior and are broken in the same way.
Comment 10 Stefan Sauer (gstreamer, gtkdoc dev) 2010-06-21 20:19:49 UTC
For reference, I have this code in buzztard:

**
 * bt_gst_element_factory_can_sink_media_type:
 * @factory: element factory to check
 * @name: caps type name
 *
 * Check if the sink pads of the given @factory are compatible with the given 
 * @name. The @name can e.g. be "audio/x-raw-int".
 *
 * Returns: %TRUE if the pads are compatible.
 */
gboolean bt_gst_element_factory_can_sink_media_type(GstElementFactory *factory,const gchar *name) {
  GList *node;
  GstStaticPadTemplate *tmpl;
  GstCaps *caps;
  guint i,size;
  const GstStructure *s;

  g_assert(GST_IS_ELEMENT_FACTORY(factory));

  node=(GList *)gst_element_factory_get_static_pad_templates(factory);
  for(;node;node=g_list_next(node)) {
    tmpl=node->data;
    if(tmpl->direction==GST_PAD_SINK) {
      caps=gst_static_caps_get(&tmpl->static_caps);
      size=gst_caps_get_size(caps);
      GST_DEBUG("  testing caps: %" GST_PTR_FORMAT, caps);
      for(i=0;i<size;i++) {
        s=gst_caps_get_structure(caps,i);
        if(gst_structure_has_name(s,name)) {
          gst_caps_unref(caps);
          return(TRUE);
        }
      }
      gst_caps_unref(caps);
    }
    else {
      GST_DEBUG("  skipping template, wrong dir: %d", tmpl->direction);
    }
  }
  return(FALSE);
}
Comment 11 Tim-Philipp Müller 2010-06-21 21:48:50 UTC
I'm not sure any code actually uses these functions, and I bet the code that does expects it to do something differently ;-)

Maybe we should deprecate these functions if they're considered broken/useless?

And add new functions with the desired behaviour, if we're not willing to change the behaviour of the broken functions?
Comment 12 Thijs Vermeir 2010-06-22 07:18:36 UTC
Hmm, I don't see the issue here to fix this function.

I want to use such a function to check if the element_factory can create an element that can accept those caps on the sink/src pads. And the name/doc suggests this function is doing that. But the result is just sometimes invalid, so it should just be fixed, no?
Comment 13 Benjamin Otte (Company) 2010-06-22 07:33:02 UTC
That's what you think it does. The question is if one interprets this function as
gst_element_factory_can_sink_all_possible_caps() or as gst_element_factory_can_sink_any_possible_caps(). If you think the first, it means that the provided caps must be a subset of the factory's caps, if you think the second, you just need a non-empty intersection.

And as noted above, "audio/x-raw-int" and "audio/x-raw-int,width=16,..." do have a possible intersection, but the first is not a subset of the second.
Comment 14 Benjamin Otte (Company) 2010-06-22 07:35:00 UTC
I just did a Google code search and the only code containing this function - apart from lots of bindings and core releases since 0.7.something - was Farsight 0.1.x and I don't think Farsight used it the right way. So I guess it might be reasonably save to adapt this function.
Comment 15 Thijs Vermeir 2010-06-22 07:41:01 UTC
ok, I understand. I'm looking for the *_any_* function with the intersection. So can I write a patch so the function works like that and clarify this in the docs? or does that need the new function name...
Comment 16 Thijs Vermeir 2010-06-22 08:01:48 UTC
Created attachment 164282 [details] [review]
gstutils: gst_element_factory_can_{sink,src}_caps check for any caps

Check for any possible caps the the element factory accepts, and not
anymore for all possible caps.
Comment 17 Sebastian Dröge (slomo) 2010-06-23 19:43:01 UTC
You should use gst_caps_can_intersect() here. IMHO we should deprecate the old functions and replace them with all/any variants
Comment 18 Thijs Vermeir 2010-06-24 07:01:06 UTC
Created attachment 164475 [details] [review]
gstutils: replace gst_element_factory_can_{sink,src}_caps

Add new functions to clarify how the caps are compared to
the template caps of the element factory.

Deprecate: gst_element_factory_can_{src|sink}_caps
API: add gst_element_factory_can_{src|sink}_{any|all}_capps
Comment 19 Stefan Sauer (gstreamer, gtkdoc dev) 2011-03-07 15:06:36 UTC
Review of attachment 164475 [details] [review]:

Looks good. Small comments below. Also some tests would be nice.

::: gst/gstutils.c
@@ +1307,3 @@
  * Returns: true if it can src the capabilities
+ *
+ * Deprecated

The tag is "Deprecated: use gst_element_factory_can_src_all_caps() instead".

@@ +1310,3 @@
  */
 gboolean
 gst_element_factory_can_src_caps (GstElementFactory * factory,

what about calling the "all" version to avoid code duplication.

@@ +1343,3 @@
  * Returns: true if it can sink the capabilities
+ *
+ * Deprecated

The tag is "Deprecated:  use gst_element_factory_can_sink_all_caps() instead".

@@ +1346,2 @@
  */
+

remove :)

@@ +1348,3 @@
 gboolean
 gst_element_factory_can_sink_caps (GstElementFactory * factory,
     const GstCaps * caps)

see above.
Comment 20 Stefan Sauer (gstreamer, gtkdoc dev) 2011-03-08 08:23:16 UTC
Created attachment 182799 [details] [review]
gstutils: replace gst_element_factory_can_{sink,src}_caps

Update the patch, will work on tests now.
Comment 21 Stefan Sauer (gstreamer, gtkdoc dev) 2011-03-08 14:00:13 UTC
Created attachment 182818 [details] [review]
gstutils: replace gst_element_factory_can_{sink,src}_caps

Also include .defs update
Comment 22 Stefan Sauer (gstreamer, gtkdoc dev) 2011-03-08 14:00:41 UTC
Created attachment 182819 [details] [review]
add tests for new api
Comment 23 Stefan Sauer (gstreamer, gtkdoc dev) 2011-03-09 13:51:36 UTC
Comment on attachment 164212 [details] [review]
mpeg4videoparse: add unit test for element factory bug

Thijs, please open a new bug if you like to make tests for the video parsers.
Comment 24 Tim-Philipp Müller 2011-03-10 18:02:21 UTC
commit 90b4dc48696481f31306e16e5ad9d5edfd0062a4
Author: Tim-Philipp Müller <tim.muller@collabora.co.uk>
Date:   Thu Mar 10 17:48:26 2011 +0000

    utils: fix ABI break when compiling gstreamer with -DGST_DISABLE_DEPRECATED
    
    GST_DISABLE_DEPRECATED should only affect visibility of declarations in headers,
    not actually remove symbols. See GitDeveloperGuidelines and DeprecatingAPI
    pages in wiki.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=402141