GNOME Bugzilla – Bug 450711
[GstController] Improve extensibility by providing a GstControlSource class
Last modified: 2007-07-06 21:53:53 UTC
Hi, the attached patches drastically change GstController to be much more flexible. With the current API it's nearly impossible to provide any other modes than interpolation. This is solved in those patches by adding a GstControlSource base class, which provides _get_value() and _get_value_array() methods and a _bind() method for binding it to a specific property. Also provided is a GstInterpolationControlSource, which implements the current interpolation logic as a GstControlSource. All old functions in GstController that had somethnig to do with itnerpolation are now deprecated and are wrappers for GstInterpolationControlSource. All old unit tests are working fine with this. The only drawback I'm aware of is, that the undocumented feature of handling live values in GstController is not possible anymore. Before it was possible to bypass the GstController by calling g_object_set() for some property, it detected this automatically and only updated the property again after the next control point. As GstControlSource does not (and must not) know anything about control points this is not possible anymore but IMHO this is not that large of a problem because a) the feature was not documented and b) it wasn't very intuitive anyway: why did the controller take effect again after the next control point? why not after 2msecs? or never? According to Stefan buzztard was the only application that used it and he's fine with removing the feature and replacing it with gst_controller_set_disabled() methods. Someone please review :) Bye PS: The next GstControlSource I plan to implement will be a osscilator, providing a few different wave forms. PPS: Everything should be completely backward compatible except the live value handling.
Created attachment 90570 [details] [review] controlsources-1.diff
Created attachment 90571 [details] [review] controlsources-docs-1.diff
Created attachment 90572 [details] [review] controlsources-tests-1.diff Remove the live value unit test that breaks now. Unit tests that don't use the deprecated functions will follow later, this tests almost everything anyway.
Created attachment 90574 [details] [review] controlsources-1.diff small bugfix
Created attachment 90638 [details] [review] controlsources-2.diff Some more bugfixes.
Created attachment 90639 [details] [review] controlsources-tests-2.diff Ported tests to the new API as we otherwise fail to build because of GST_DISABLE_DEPRECATED... otherwise all old unit tests would work fine, even without memleaks ;)
Created attachment 90686 [details] [review] controlsources-examples-1.diff
Created attachment 90825 [details] [review] controlsources-3.diff Make it possible to disable only one property instead of all or none.
Created attachment 90983 [details] [review] controlsources-4.diff and even some more fixes... this should be ready to go in.
+1 to commit this from my side. The only think I could think of is to add a read-only "number-of-control-points" property to the interpolation control-source.
Created attachment 91229 [details] [review] controlsources-5.diff Updated with some suggestions from Stefan: - added gst_interpolation_control_source_get_nvalues() to get the number of control points (maybe needs a better name?). I didn't create a GObject property for this because of consistency reasons, everything else has a function too and we don't use GObject properties for something else. - the get_value and get_value_array methods now return a gboolean, whether they were successful or not - if no control point is set the interpolators will return FALSE and do nothing - the interpolators don't have the default value at timestamp 0 anymore, instead they will return the value of the first control point until the first control point's timestamp. This is more logical behaviour.
Created attachment 91230 [details] [review] controlsources-docs-2.diff
Created attachment 91231 [details] [review] controlsources-tests-3.diff
Should GstInterpolateMode be better called GstInterpolationMode? beside something seem to have broken the trigger params. The trigger each sync here. Will do some more testing.
gst_interpolation_control_source_find_control_point_node() never returns NULL. The method could use some more comments too ;)
GstInterpolateMode can't be renamed because of API compatibility unfortunately... gst_interpolation_control_source_find_control_point_node() will always return NULL when the timestamp is before the first control point. And the trigger isn't triggered each sync(), it's just that the property is not set to the default value again after a trigger. That's the job of the element. For non-trigger timestamps nothing will happen, before the default value was set instead.
Created attachment 91307 [details] [review] controlsources-6.diff - _get_nvalues() is now _get_count(). Maybe _get_value_count() is better? - trigger always returns the default value if not on a trigger timestamp but control points exists, return FALSE if no control point exists. This fixes Stefan's problem
Created attachment 91308 [details] [review] controlsources-docs-3.diff
Created attachment 91309 [details] [review] controlsources-tests-4.diff
I'm not yet convinced that the behaviour for trigger is right though... shouldn't a property that works as trigger reset itself each time it gets something set? Whatever, this is probably ready to commit...
Now its works again. The trigger is reset whenever one syncs again (unless that again is a control-point).
Ok, done... 2007-07-06 Sebastian Dröge <slomo@circular-chaos.org> Reviewed by: Stefan Kost <ensonic@users.sf.net> * libs/gst/controller/Makefile.am: * libs/gst/controller/gstcontroller.c: (gst_controlled_property_add_interpolation_control_source), (gst_controlled_property_new), (gst_controlled_property_free), (gst_controller_find_controlled_property), (gst_controller_new_valist), (gst_controller_new_list), (gst_controller_new), (gst_controller_remove_properties_valist), (gst_controller_remove_properties_list), (gst_controller_remove_properties), (gst_controller_set_property_disabled), (gst_controller_set_disabled), (gst_controller_set_control_source), (gst_controller_get_control_source), (gst_controller_get), (gst_controller_sync_values), (gst_controller_get_value_array), (_gst_controller_dispose), (gst_controller_get_type), (gst_controlled_property_set_interpolation_mode), (gst_controller_set), (gst_controller_set_from_list), (gst_controller_unset), (gst_controller_unset_all), (gst_controller_get_all), (gst_controller_set_interpolation_mode): * libs/gst/controller/gstcontroller.h: * libs/gst/controller/gstcontrollerprivate.h: * libs/gst/controller/gstcontrolsource.c: (gst_control_source_class_init), (gst_control_source_init), (gst_control_source_get_value), (gst_control_source_get_value_array), (gst_control_source_bind): * libs/gst/controller/gstcontrolsource.h: * libs/gst/controller/gsthelper.c: (gst_object_set_control_source), (gst_object_get_control_source): * libs/gst/controller/gstinterpolation.c: (gst_interpolation_control_source_find_control_point_node), (gst_interpolation_control_source_get_first_value), (_interpolate_none_get), (interpolate_none_get), (interpolate_none_get_boolean_value_array), (interpolate_none_get_enum_value_array), (interpolate_none_get_string_value_array), (_interpolate_trigger_get), (interpolate_trigger_get), (interpolate_trigger_get_boolean_value_array), (interpolate_trigger_get_enum_value_array), (interpolate_trigger_get_string_value_array): * libs/gst/controller/gstinterpolationcontrolsource.c: (gst_control_point_free), (gst_interpolation_control_source_reset), (gst_interpolation_control_source_new), (gst_interpolation_control_source_set_interpolation_mode), (gst_interpolation_control_source_bind), (gst_control_point_compare), (gst_control_point_find), (gst_interpolation_control_source_set_internal), (gst_interpolation_control_source_set), (gst_interpolation_control_source_set_from_list), (gst_interpolation_control_source_unset), (gst_interpolation_control_source_unset_all), (gst_interpolation_control_source_get_all), (gst_interpolation_control_source_get_count), (gst_interpolation_control_source_init), (gst_interpolation_control_source_finalize), (gst_interpolation_control_source_dispose), (gst_interpolation_control_source_class_init): * libs/gst/controller/gstinterpolationcontrolsource.h: * libs/gst/controller/gstinterpolationcontrolsourceprivate.h: API: Refactor GstController into the core controller which can take a GstControlSource for providing actual values for timestamps. Implement a interpolation control source and use this for backward compatibility, deprecate a bunch of functions that are now handled by GstControlSource or GstInterpolationControlSource. Make it possible to disable the controller completely or only for specific properties. Fixes #450711. * docs/libs/gstreamer-libs-docs.sgml: * docs/libs/gstreamer-libs-sections.txt: * docs/libs/gstreamer-libs.types: Add new functions and classes to the docs. * tests/check/libs/controller.c: (GST_START_TEST), (gst_controller_suite): * tests/examples/controller/audio-example.c: (main): Port unit test and example to the new API and add some new unit tests.