GNOME Bugzilla – Bug 353475
[API] GstURIHandler: add uri_is_supported vfunc
Last modified: 2012-08-18 19:37:18 UTC
While working with adding daap/dpap support on gstreamer trought gnomevfs modules, i found that the playbin gst element could not use the new gnomevfs modules. After some investigation, i found that the gnomevfs plugin was implementing the GstURIHandlerInterface in order to inform what protocols were supported by the plugin. The problem is that gnomevfs does not have a method to query for all supported protocols, and this way the list of supported protocols was hardcoded in the gnomevfs gst plugin. Attached there is a patch to add a new virtual method to GstURIHandlerInterface that instead of querying all supported protocols, it receives a uri and returns if this uri is supported or not, allowing gnomevfs gst plugin to work with more gnomevfs modules than the hardcoded list.
Created attachment 71865 [details] [review] Implement the support on gstreamer core
Created attachment 71866 [details] [review] Added support on gnomevfs plugin (requires the gstreamer core patch)
Created attachment 71871 [details] [review] Fixed typo, Updated doc, do not touch gstelementfactory* This patch moves all the code to gsturi.[hc] and fix a typo.
The first question is whether we would want to support arbitrary protocols in gnomevfssrc in the first place, or rather only support known-to-be-good-and-stable protocols like we do now. If we add new API for something like this, we might want to consider things like per-protocol ranks as well (and whether we want to cover that with the same API or a separate piece of API). In your patch in gst_uri_handler_uri_is_supported() you do: + g_return_val_if_fail (iface->uri_is_supported != NULL, FALSE); That doesn't look quite right to me. g_return_val_if_fail() should only be used to catch programming errors and invalid input and such. If non-implementation of the uri_is_supported method is acceptable (and it is, since it is an optional addition), the function must not throw a warning and just return FALSE as the documentation suggests (or ideally fall back on going through the protocols array and decide based on that).
(In reply to comment #4) > The first question is whether we would want to support arbitrary protocols in > gnomevfssrc in the first place, or rather only support > known-to-be-good-and-stable protocols like we do now. This is up to you guys. I personally think this is an interesting adition, but i am biased :). > If we add new API for something like this, we might want to consider things > like per-protocol ranks as well I quite don't understand this, could you elaborate a little more. > (and whether we want to cover that with the same API > or a separate piece of API). I believe the same API for now is enough, but we can think of a new API later if needed. > In your patch in gst_uri_handler_uri_is_supported() you do: > > + g_return_val_if_fail (iface->uri_is_supported != NULL, FALSE); > > That doesn't look quite right to me. g_return_val_if_fail() should only be used > to catch programming errors and invalid input and such. If non-implementation > of the uri_is_supported method is acceptable (and it is, since it is an > optional addition), the function must not throw a warning and just return FALSE > as the documentation suggests (or ideally fall back on going through the > protocols array and decide based on that). Agreed, i will fix this. I just don't know what is best, to fallback on the protocols array or return FALSE. I will just return FALSE for now, but i can change it later if needed. BR Andrunko
Created attachment 71973 [details] [review] Fixed gst_uri_handler_uri_is_supported Do not use g_return_val_if_fail to check if the optional callback is implemented.
Is this still something we want? I don't quite see the reason why we need a vfunc like this, the list of protocols/schemes should include everything that is supported and the only reason why such URI fails later is if it is an invalid URI. In that case it will either be catched by the GStreamer URI handling code (for really broken URIs) or shortly afterwards after passing it to set_uri(). If an element claims to support some URI scheme it should always handle valid URIs with that scheme.
Andre, would you please respond to comment#7 ?
Sebastian: well, one problem is that there are a bunch of artificial URIs for which no clear standard exists really, e.g. cdda:// URIs or dvd:// URIs or so - the way the device is passed, the way the title information is added and all that is pretty much non-standard, so it's well possible that one element rejects a cdda:// URI not because it is invalid but because it's not made up the way it expects, while another element will accept that URI. However, since this refered to gnomevfs, and gio seems to have a g_vfs_get_supported_uri_schemes(), this may just be obsolete now?
Ok, understood and makes sense. But why would this be obsolete because of g_vfs_get_supported_uri_schemes()? The situation is the same as with gnomevfs, you only additionally get all URI schemes that are supported (which still doesn't say anything about how they should be constructed for e.g. cdda).
What to do with this? gst_element_make_from_uri() and gst_uri_handler_set_uri() both return GErrors now, so the caller can tell the difference between 'unsupported scheme' and 'bad URI'.
Closing as obsolete, please re-open if you think we still need or want this.