GNOME Bugzilla – Bug 740502
Add absolute property to GstDirectControlBinding
Last modified: 2016-01-12 23:49:31 UTC
Currently you can not slide an image/text overlay in and out of the frame. One fix would be to remove the CLAMP's constricting the values inside the property range and use the relative properties of textoverlay and gdkpixbufoverlay but it doesn't seem right to me. I propose adding an 'absolute' property that doesn't modify the values being input in the controller at all.
Created attachment 291183 [details] [review] Add absolute property patch
Created attachment 291185 [details] Example usecase
Review of attachment 291183 [details] [review]: The documentation blob at the top needs to be updates as well. I would not mind to see the example being turned into a patch for tests/examples/controller/. ::: libs/gst/controller/gstdirectcontrolbinding.c @@ +191,3 @@ + g_param_spec_boolean ("absolute", "Absolute", + "Whether the control values are absolute", + DEFAULT_PROP_ABSOLUTE, G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS); just put FALSE here and remove the DEFAULT_PROP_ABSOLUTE (please not that G_PARAM_CONSTRUCT can be used to initialize the default). Also does it make sense to ever switch this at run-time? Can this be G_PARAM_CONSTRUCT_ONLY? If construct-only you can make two sets of mapping functions and avoid the if (self->want_absolute) at runtime - this code is getting called quite often. @@ +199,3 @@ gst_direct_control_binding_init (GstDirectControlBinding * self) { + self->want_absolute = DEFAULT_PROP_ABSOLUTE; the object is cleared to zero by default.
Created attachment 291987 [details] [review] fixes after review
Review of attachment 291987 [details] [review]: I think it looks good (besides the name for the constructor). I still like to see some testing. We also have tests/examples/controller - maybe your example can be merged there? Can you try that as a 2nd patch? ::: libs/gst/controller/gstdirectcontrolbinding.c @@ +532,3 @@ + */ +GstControlBinding * + * perhaps gst_direct_control_binding_new_absolute() ?
Created attachment 303905 [details] [review] Changed constructor name as per review and added example usage
Created attachment 303977 [details] [review] Changed constructor name as per review and added example usage ( fixed author )
Review of attachment 303977 [details] [review]: Please still add unit tests as asked before. ::: libs/gst/controller/gstdirectcontrolbinding.h @@ +75,3 @@ */ +struct _GstDirectControlBinding +{ please don't reformat code unless you change it. especially don't gst-indent header files
Created attachment 304937 [details] [review] Added unit test
Created attachment 304940 [details] [review] Fixed header
The following fix has been pushed: cb2c141 controller: Added absolute direct control binding, example and test
Created attachment 305120 [details] [review] controller: Added absolute direct control binding, example and test Fixes: 740502 API: gst_direct_control_binding_new_absolute
I added a Since: marker to the api and updated the example a bit to not set properties that don't exist (anymore).
commit 86abdbfb5552a11219275e5b597e7d8b476748b2 Author: Tim-Philipp Müller <tim@centricular.com> Date: Mon Jun 29 10:41:27 2015 +0100 directcontrolbinding: fix ABI break Structure size was increased without adjustment of the padding. https://bugzilla.gnome.org/show_bug.cgi?id=751622 https://bugzilla.gnome.org/show_bug.cgi?id=740502
*** Bug 760526 has been marked as a duplicate of this bug. ***