[gstreamer-bugs] [Bug 396774] Make GstElementDetails extensible

GStreamer (bugzilla.gnome.org) bugzilla at gnome.org
Fri Aug 13 07:26:03 PDT 2010


https://bugzilla.gnome.org/show_bug.cgi?id=396774
  GStreamer | gstreamer (core) | git

--- Comment #25 from Tim-Philipp Müller <t.i.m at zen.co.uk> 2010-08-13 14:25:59 UTC ---
I'm a bit undecided about this. What's the intention here exactly? Just make
things more extensible at the API level and more generic at the implementation
level?

Do we want to allow 'unsanctioned' new fields? (This I'm not sure about - of
course it doesn't *hurt*, but is it useful? Does it make good API?)

If we do not want to allow arbitrary fields, why not just add

  gst_element_class_set_doc_uri (),
  gst_element_class_set_icon_name(), etc. ?

If we do want to allow arbitrary fields, I think simple API like:

  gst_element_class_set_string_detail (klass, "key", "value"), or
  gst_element_class_set_detail_string (klass, "key", "value"), or just
  gst_element_class_set_detail (klass, "key", "value");

would be both sufficient for now and much nicer (not least for bindings). I
prefer the latter, fwiw :) I would avoid the word 'METADATA' then, and just go
for GST_ELEMENT_DETAIL_DOC_URI or somesuch.

I would avoid vararg functions, and G_TYPE_* and all that. Surely another two
or three function calls here aren't really signficant performance-wise?
Avoiding this would also make it easier to use a different implementation
that's more suitable/efficient for serialisation/deserialisation at a later
stage (e.g. GVariant).

I'm not in favour of a registration system for fields here, that seems a bit
over the top (I saw that the patch doesn't implement that, just saying).


> /*< private >*/
> GstStructure		 *meta_data;

I think we should hide the implementation of this field for the reasons
described above. Even if we declare it private, it never really is. We can use
a dummy pointer typedef here that is then typedef'ed in gst_private.h to
GstStructure *. That way we can be 100% sure we can change it later.


> gst_element_factory_get_meta_data_detail (GstElementFactory * factory,
>                                                                                 const gchar              * detail);

Similar to what I wrote above, I'd avoid the "meta_data" phrase here and just
make it

    gst_element_factory_get_detail (factory, detail);

    (or gst_element_factory_get_detail_string (factory, detail);
     or gst_element_factory_get_string_string (factory, detail); if needed).

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.



More information about the Gstreamer-bugs mailing list