GNOME Bugzilla – Bug 125739
GstPropertyProbe interface
Last modified: 2004-12-22 21:47:04 UTC
patch for a GstProperty probe interface. An application may call gst_property_probe_get_list(element) to get a list of properties that can be probed. Probeable properties have the type (char *). At a time and in a thread appropriate to the application, gst_property_probe_probe_property(element,property_name) is called. This function may potentially take a long time to discover possible property values. After a sucessful probe_property() call, the application can call gst_property_probe_get_property_into() to get the list of probed property values. The list of probed property values is not exhaustive. For example, probing for an X display may return :0 and :1, but won't (necessarily) return condor.ucsf.edu:0 -- however, this could still be a valid value for the property. This patch is slightly premature. The prototypes for the functions are wrong, and the property info should be a structure with { property value, translatable string } pairs.
Created attachment 21021 [details] [review] patch
Created attachment 21028 [details] [review] new patch
Several comments: + if (!gst_library_load ("gstmixer")) + return FALSE; + + if (!gst_library_load ("gstpropertyprobe")) + return FALSE; That will not work. Applications need to link to the API, too (e.g. the _get_type () function), so you can't gst_library_load () in the application. I'd suggest to revert this. It will only work uninstalled. gst_ossprobe_probe_property (GstElement *element, const char *property) How about making that a GParamSpec? In that way, you can do a switch (spec->param_id) instead of a series of strcmp()s, just like in _set_property() and _get_property(). It looks a bit more gobject'ish... You could even consider making the probe_property functions that the app calls and the virtual functions not-the-same, just like in GObject. this means that you get the param_id and the GParamSpec in the plugins, but the app can choose to call a string-based or a GParamSpec-based property_probe(). How are get_property_info() and probe_property() different? It seems like one probes actively, and the other returns the cached probe result. Imo, the app shouldn't care, it's up to the plugin to decide. The plugin knows better than the app when to free its cache. + for (i=0;i<4;i++) { + char *devname = g_strdup_printf ("/dev/mixer%d", i); + int fd; + + fd = open (devname, O_RDONLY); + + if (fd >= 0) { + g_ptr_array_add(ptr_array, devname); + close (fd); + } else { + g_free(devname); + } + } Will result in issues. See gstosselement.c on how to open a device non-blocking. Imo, we should use gst_osselement_open() instead of open() even for probing. I'd personally suggest to create the probe-list at _class_init() time, not in the probe function, since the element can then be in another state than NULL.
We need to stop building interfaces as plugins, then. Yes, it makes more sense as a GParamSpec. The interface wrapper code should support both. probe_property() may take a long time, and should be called specifically when the application wants the element to probe, and in the given thread. get_property_info() merely returns an already-probed list. It also makes sense to only allow probe_property() calls when the element is in NULL. The entire point of this interface is that probing can (or is allowed to) take a long time. Probing (in this sense) in a class init function is not reasonable.
Did we build interfaces as plugins? :). Probing can indeed take long... I'm not sure how reasonable it is to require the application to know this, though... Hm... Interesting issue. I think your idea makes sense... However, the result of the probe must be stored in the class, not in the instance. That way, you only need to probe once for all instances together.
Yes, most of the interfaces are currently plugins. Not for long, though. Where the probed information is stored is up to the implementation. Certain properties (e.g., "rate" in osssink) depend on the instance, not the class.
Rate isn't a property, the possible samplerate values are supposed to be available through _get_caps(), right?
Checked in, closing.