GNOME Bugzilla – Bug 402141
gst_element_factory_can_{sink,src}_caps seems to be broken
Last modified: 2011-03-10 18:02:21 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.
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);
Your example is flawed; what you describe as workaround is the correct solution for the problem you face.
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?
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?
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)
That sounds like the right thing to do. Not a bug, then?
*** Bug 622272 has been marked as a duplicate of this bug. ***
Created attachment 164212 [details] [review] mpeg4videoparse: add unit test for element factory bug
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.
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); }
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?
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?
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.
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.
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...
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.
You should use gst_caps_can_intersect() here. IMHO we should deprecate the old functions and replace them with all/any variants
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
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.
Created attachment 182799 [details] [review] gstutils: replace gst_element_factory_can_{sink,src}_caps Update the patch, will work on tests now.
Created attachment 182818 [details] [review] gstutils: replace gst_element_factory_can_{sink,src}_caps Also include .defs update
Created attachment 182819 [details] [review] add tests for new api
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.
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