[gstreamer-bugs] [Bug 396779] Preset interface for elements

[GStreamer (bugzilla.gnome.org)]

If you have any questions why you received this email, please see the text at
the end of this email. Replies to this email are NOT read, please see the text
at the end of this email. You can add comments to this bug at:
  http://bugzilla.gnome.org/show_bug.cgi?id=396779

  GStreamer | gstreamer (core) | Ver: HEAD CVS




------- Comment #56 from Jan Schmidt  2008-03-06 22:07 UTC -------
(In reply to comment #52)
> (In reply to comment #51)
> > 
> > * the gst_preset_create_preset function. A function to randomise the properties
> > which have been designated for presets doesn't seem a generally useful thing. I
> > think if such a thing is needed, it is better implemented as a utility function
> > which iterates the list of properties and randomises them.
> 
> Nope. Its an interface with a default implementation. Element can override and
> provide a resonable randomization. Applications can call it regardless of the
> implementations. That won't work with utility functions.

More generally I meant that the idea behind the 'gst_preset_create_preset'
function seems broken. As I understand it, the definition is 'configure the
element in some unspecified way with a set of properties which could be saved
as a new preset'. Given that, the only sensible way to implement it is to
randomise the properties in some way. By putting it in the interface, it lets
the element control how the randomisation is done, but effectively this is a
'gst_preset_randomise_properties' function. My objection is that while the rest
of the interface is very generic, 'gst_preset_create_preset' is quite specific
to your use case and doesn't seem useful in any others I can see - for example
setting up video/audio encoder configurations, or standard colorkey profiles
for the alpha plugin. This same functionality could be implemented in several
other ways, without shoe-horning it into the interface:

* Elements which can be sensibly randomised could provide a 'virtual' profile
called 'Randomise' which performs the randomisation when loaded.
* Provide a signal named 'randomise' and emit it into the element.
* Provide a property named 'randomise' which would do the randomisation when
set.

To be clear - I'm raising an objection because I think this function doesn't
gel well with the rest of the interface, but I don't mind sufficiently to stop
you. At the least though, I'd prefer that randomisation wasn't the default
implementation unless the elements requested it in some way, since it really
doesn't make sense to randomise theoraenc's properties for example. 

> > * Doubling up GST_PARAM_CONTROLLABLE to mark properties which should be
> > included in the 'presets' set. It makes sense to have a simple way like that to
> > mark Preset-table properties, but I think it should be a separate flag. It
> > doesn't make sense, for example, for an MPEG encoder to have the GOP size be
> > controllable, but it makes sense for that to appear in a preset.
> 
> The is an interface function (gst_preset_get_property_names) that elements can
> override. This function allows them to drop names of parameter that should not
> be serialized. The use of the GST_PARAM_CONTROLLABLE is indeed debatable, but
> is useful right now.

Sure, but then people that want to implement the interface need to do more
work. As an idea, we can supply a couple of default functions for
implementations to choose from - list all GST_PARAM_CONTROLLABLE properties
might be one, present a NULL terminated string list could be another, a
property flag GST_PARAM_PRESETS and a function to list those might be another. 

> > * The function of gst_preset_set/get_meta methods is obscure to me. What's the
> > point of being able to add generic tagged info to a preset? What does it mean
> > to the application? To the user? Is there a structure to them? What are the
> > rules?
> 
> That was discussed several times in IRC. I will improve the docs.

OK

> > * Christopher Dehais mentioned using GKeyFile for his RB equalizer presets,
> > which, along with GValue transformations seems like a better approach than the
> > existing manual serialising, deserialising and parsing code.
> 
> Can you point me to his patch for a resulable implementation on the gstreamer
> level. Sarcasm asise, I am aware that they cooked up some own way to handle it,
> despite my and slomos efforts to make the equalizer usable for them. They are
> aware of this effort, but have not contributed to it. Regarding GKeyfile, could
> be used, but won't save many lines. Regarding GValue transformations, we need
> anything to string and string to anything. I am not sure how GValue to GValue
> will help. You mean converting a int GValue to a string GValue then get the
> string content from that?

Right - using the existing g_value_transforms would simplify the loops in
gst_preset_default_save_preset and gst_preset_default_load_preset and allow
automatic support for any type that has conversions to/from string defined -
like all the standard GTypes, and GstFraction, and even complex types like
GstCaps. Something like:

for prop_name in props_list:
  g_value_init (&str_value, G_TYPE_STRING);
  g_object_get_property (G_OBJECT (self), prop_name, &in_value);
  if (g_value_transform (&in_value, &str_value)) {
    g_hash_table_insert (data, prop_name, 
        g_strdup (g_value_get_string (&str_value));
  }
  g_value_reset (&str_value);


-- 
See http://bugzilla.gnome.org/page.cgi?id=email.html for more info about why you received
this email, why you can't respond via email, how to stop receiving
emails (or reduce the number you receive), and how to contact someone
if you are having problems with the system.

You can add comments to this bug at http://bugzilla.gnome.org/show_bug.cgi?id=396779.