[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 #52 from Stefan Kost (gstreamer, gtkdoc dev)  2008-03-05 17:26 UTC -------
(In reply to comment #51)
> *sigh* Well, speaking for myself, I didn't look at it until now, no. We're all
> busy people.

Thanks for taking time to review.
> 
> In general, I think it's a decent approach. I don't like:
> 
> * 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.

> * 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.

> * 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.

> * 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?

> * There are some spelling mistakes in the docs, but those are easy to fix
Yes, I'll spellcheck before updating the patch.

> 
> There are some spurious docs bits in the gstreamer-sections.txt in the patch
> above that shouldn't be there afaics:
> +gst_element_class_set_meta_data
> +gst_element_factory_get_meta_data_detail
> +gst_query_new_qos
> +gst_query_set_qos
> +gst_query_parse_qos

That happens when people leave me with uncommittable changes for that long.
I'll 
manually patch the section file when updating the patch.

> 
> You hopefully already realise this, but 1 day before the freeze really isn't
> the right time to have brought this up again - poking us continuously since the
> last release would have been better.

I was poking you since last release and relase before. I have sent several
emails about this. This time I misread the freeze schedule and took the 10th of
march for the freeze as the table seems to include week nuber for odd reasons.

This is slowly getting a hard test for patience. I am happy to discuss the
details and make changes. Its seems to be total failure to discuss this in
bugzilla. Any suggestions? Review day in irc?


-- 
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.