GNOME Bugzilla – Bug 392393
[API] add libgstbaseutils library for missing plugins messages and unified descriptions
Last modified: 2007-01-09 14:22:12 UTC
For the automatic plugin/codec installation stuff (see bug #161922), I'd like to add a libgstbaseutils library which, for now, provides the following: - creating and parsing of missing-plugin messages as defined in gstreamer/docs/random/draft-missing-plugins.txt and hiding the details from applications and from playbin. - a unified media_type/caps/protocol <=> Human-readable description mapping. This is required and/or useful for the following reasons: a) so applications can show half-way intelligible messages to the user about what it is exactly that is missing. b) provide consistent (and translated) descriptions for one and the same format, at least once demuxers are changed to use this for their codec tag stuff as well. c) saves duplication of all those descriptions in the various demuxers; not to mention that there would be a central place where to disable them for GStreamer users that have strict library size constraints. This also allows applications to provide much better error messages in the absence of automatic codec installation support: "You need a Theora decoder to play this file" is much more informative than "You don't have the necessary decoders installed to play this file".
Created attachment 79304 [details] [review] Adds libgstbaseutils with basic unit tests and documentation See patch for proposed API (header files are all at the beginning).
The main bits of API this adds are: gboolean gst_element_post_missing_uri_source_message (GstElement * e, const gchar * protocol); gboolean gst_element_post_missing_decoder_message (GstElement * e, const GstCaps * decode_caps); These functions will be used by playbin to post missing-plugin messages. There are similar functions for uri_sink, encoder, and element for completeness. Then some API for applications to easily parse and process these messages: gchar * gst_missing_plugin_message_get_installer_detail (GstMessage * m); gchar * gst_missing_plugin_message_get_description (GstMessage * m); gboolean gst_is_missing_plugin_message (GstMessage * m); Then some API to get descriptions from format caps or protocols: gchar * gst_base_utils_get_codec_description (const GstCaps * caps); gchar * gst_base_utils_get_source_description (const gchar * protocol); gchar * gst_base_utils_get_decoder_description (const GstCaps * caps); (and similar ones for sink/encoder/element for completeness). These are used internally only so far to populate the description field in the missing-plugin messages, but are useful for applications in general and would also be useful for demuxers for the codec tag stuff in future.
If I understand the API correctly I could also use the API in buzztard. When loading a song, I might dicover that elemnts are missing. I could use gst_element_post_missing_element_message() from my loader object. The UI front-end catches them and uses gst_missing_plugin_message_get_XXX() to extract the info for libgimmi-codec. If I understood right, it means +1 from my side to add this :) The patch looks good. Some comments still * copy_and_clean_caps() I attach a collected caps list, I've generated from a test, there might be more elements we could filter out.
Created attachment 79711 [details] caps list (grabed from gst-media-test suite run)
In general, this looks pretty promising. A few notes: * base-utils.h should include descriptions.h and missing-plugins.h as <gst/utils/base-utils.h>, because they're all installed headers * Why force the user to include descriptions and missing-plugins via base-utils? * The descriptions.h functions, look good... but is it a good idea to drop the base_utils bit and use the 'GstElement' namespace for the gst_element_post_missing_ functions? I don't know if this has any implications for autogenerated bindings. They're not GstElement member methods, so I don't think they should use the namespace, but... gst_base_utils_element_post_missing_* is just getting silly. Leaving out bad too thought. Perhaps we can come up with shorter function names for those methods, or alternatively, remove the post_element part of the functionality to put the methods into the gst_missing_plugin_message namespace, and make the caller then do gst_element_post () after. Bah, I dunno :) It's mostly the bindings part that bugs me - these functions shouldn't/can't end up as methods of GstElements in Python/C#/Perl/C++ and friends, so they shouldn't go into tat namespace. * I'm not sure about 'utils/' as an include path, but I can't think of any real objection except that it's more generic than our existing gst/ subdir examples: audio, base, cdda, check, controller, dataprotocol, floatcast, interface, net, netbuffer, riff, rtp, tag, video * Still needs docs hooks in the sections.txt file.
Thanks for the review. > * base-utils.h should include descriptions.h and missing-plugins.h as > <gst/utils/base-utils.h>, because they're all installed headers True, will fix (odd that it didn't cause problems). > * Why force the user to include descriptions and missing-plugins via > base-utils? No particular reason, will fix. > * The descriptions.h functions, look good... but is it a > good idea to drop the base_utils bit and use the > 'GstElement' namespace for the > gst_element_post_missing_ > functions? I don't know if this has any implications for > autogenerated bindings. They're not GstElement member > methods, so I don't think they should use the namespace, > but... > gst_base_utils_element_post_missing_* > is just getting silly. Leaving out bad too though. > Perhaps we can come up with shorter function names for > those methods, or alternatively, remove the post_element > part of the functionality to put the methods into the > gst_missing_plugin_message namespace, and make the caller > then do gst_element_post () after. Bah, I dunno :) It's > mostly the bindings part that bugs me - these functions > shouldn't/can't end up as methods of GstElements in > Python/C#/Perl/C++ and friends, so they shouldn't go into > that namespace. I agree, but couldn't really think of better names either. Didn't know it might be an issue for bindings. I thought about just providing functions to create the messages as well, so let's do that then. Just need to find good function names now... > * I'm not sure about 'utils/' as an include path, but I > can't thinkof any real objection except that it's more > generic than our existing gst/ subdir examples: > audio, base, cdda, check, controller, dataprotocol, > floatcast, interface, net, netbuffer, riff, rtp, tag, video I chose the generic name for the library on purpose. IMHO there's lots of stuff that we're replicating all over the place in plugins that we might want to put into a general utility library like this, but that shouldn't end up in core (e.g. base64, misc. hash functions, basic abstraction for win32/unix sockets, etc.). Would you prefer base-utils/ or baseutils/ as an include path instead? > * Still needs docs hooks in the sections.txt file. Yes, left that out on purpose while the API isn't final yet. I will add that once the API is aggreed upon.
Suggestions to replace the gst_element_post() stuff in missing-plugins.h with: 1) GstMessage * gst_missing_plugin_message_new_uri_source (const gchar * proto) GstMessage * gst_missing_plugin_message_new_uri_sink (const gchar * proto) GstMessage * gst_missing_plugin_message_new_element (const gchar * factory_name) GstMessage * gst_missing_plugin_message_new_decoder (const GstCaps * caps) GstMessage * gst_missing_plugin_message_new_encoder (const GstCaps * caps) 2) GstMessage * gst_missing_uri_source_message_new (const gchar * proto) GstMessage * gst_missing_uri_sink_message_new (const gchar * proto) GstMessage * gst_missing_element_message_new (const gchar * factory_name) GstMessage * gst_missing_decoder_message_new (const GstCaps * caps) GstMessage * gst_missing_encoder_message_new (const GstCaps * caps) 1) seems more consistent with the API in gst/gstmessage.h to me. Opinions?
looks ok. Please rename the message constructors (gst_missing_..._message_new)
Created attachment 79847 [details] [review] updated patch with the suggested changes Changes: - no more gst_element_post_missing_*_message() API in order to avoid problems with auto-generated bindings. - instead add gst_missing_*_message_new() API, the plugin then has to post the message itself (note that contrary to comment #7 the message creation functions take two arguments, with the GstElement posting the message as first argument, just like gst_message_new_element(). - fixed includes to be <gst/utils/foo.h> instead of "gst/utils/foo.h" - fixed includes not to force inclusion of <gst/utils/base-utils.h> - includes all the bits to generate a section in the documentation for this - include path stays <gst/utils/*h>, library name stays libgstbaseutils-0.10
Committed: 2007-01-09 Tim-Philipp Müller <tim at centricular dot net> * configure.ac: * gst-libs/gst/Makefile.am: * gst-libs/gst/utils/Makefile.am: * gst-libs/gst/utils/base-utils.c: (gst_base_utils_init): * gst-libs/gst/utils/base-utils.h: * gst-libs/gst/utils/descriptions.c: (format_info_get_desc), (find_format_info), (caps_are_rtp_caps), (gst_base_utils_get_source_description), (gst_base_utils_get_sink_description), (gst_base_utils_get_decoder_description), (gst_base_utils_get_encoder_description), (gst_base_utils_get_element_description), (gst_base_utils_add_codec_description_to_tag_list), (gst_base_utils_get_codec_description), (gst_base_utils_list_all): * gst-libs/gst/utils/descriptions.h: * gst-libs/gst/utils/missing-plugins.c: (missing_structure_get_type), (copy_and_clean_caps), (gst_missing_uri_source_message_new), (gst_missing_uri_sink_message_new), (gst_missing_element_message_new), (gst_missing_decoder_message_new), (gst_missing_encoder_message_new), (missing_structure_get_string_detail), (missing_structure_get_caps_detail), (gst_missing_plugin_message_get_installer_detail), (gst_missing_plugin_message_get_description), (gst_is_missing_plugin_message): * gst-libs/gst/utils/missing-plugins.h: API: add new libgstbaseutils library with functions - to create and parse missing-plugins messages - that provide (translated) descriptions for caps/decoders/sources/etc. Closes #392393. * pkgconfig/gstreamer-plugins-base-uninstalled.pc.in: * pkgconfig/gstreamer-plugins-base.pc.in: Add new lib. * docs/libs/gst-plugins-base-libs-docs.sgml: * docs/libs/gst-plugins-base-libs-sections.txt: Generate docs for new lib and API. * tests/check/Makefile.am: * tests/check/libs/.cvsignore: * tests/check/libs/utils.c: (missing_msg_check_getters), (GST_START_TEST), (libgstbaseutils_suite): Add some basic unit tests.