GNOME Bugzilla – Bug 754678
controller: the cubic interpolation we have implemented can overshoot min/max of the control-point values
Last modified: 2016-01-05 12:59:22 UTC
I will attach screenshots + patches for a new interpolation mode.
Created attachment 310816 [details] [review] gtk demo to try different interpolationmodes
Created attachment 310817 [details] [review] refactor the interpolation code
Created attachment 310818 [details] [review] add new monotonic cubic splines
Created attachment 310819 [details] [review] add new monotonic cubic splines
Created attachment 310820 [details] cubic splines
Created attachment 310821 [details] monotonic cubic splines
commit 56f12705cab3affa2fb03fad7e9eb9859f1b9137 Author: Stefan Sauer <ensonic@users.sf.net> Date: Mon Sep 7 09:39:32 2015 +0200 interpolationcontrolsource: add cubic_mono interpolation This new mode won't overshoot the min/max y values set by the control-points. Fixes #754678 API: GST_INTERPOLATION_MODE_CUBIC_MONO commit 2fe3939ce7ea84c45dd922e7f1097dd07f11fc5d Author: Stefan Sauer <ensonic@users.sf.net> Date: Mon Sep 7 09:37:05 2015 +0200 interpolationcontrolsource: refactor code Extract common code that looks up the control-points around the timestamp. Add some comments for future investigation. commit 6e7915d5cdc00c5f9d3ad880f5ac96d771bb63de Author: Stefan Sauer <ensonic@users.sf.net> Date: Fri Sep 4 16:38:37 2015 +0200 tests/examples: add a demo for the interpolation control source modes This is in preparation for new modes to be added. In particullar it demonstrates how the cubic splines overshoot the range.
Meh, this breaks the libs/libsabi test since I added another double in the union in GstControlPoint. Unfortunately this stuct does not even have padding. This struct should not have been in a public header in the first place :/ It is not used on any public functions as a parameter. So what can we do? a) I privately clone GstControlPoint once more and add a bunch of TODOs b) I prep a patch that move GstControlPoint to a private header now and remove it from the abi checks. b) would be an API+ABI break, if there is actually someone peeking into this structure. Any opinions?
Meh #2, this struct needs padding. If we need to preserve compat for now, I'll have to revert the changes and wait for 2.0. The only solution that keeps the ABI I can think of would be to use a gpointer in the union (smaller than two doubles) and then in the GstInterpolationControlSource keep a GHashTable<gpointer, struct-with-3-doubles>. The struct-with-3-doubles would be allocates on the fly. The hashtables would be nuked on finalize.
I think you should take a common-sense and risk-based approach to this: do you know if any external code is using this structure? is it likely anyone is using this? As far as I can see the only reason to use this is for subclasses to peek into the GSequence directly or modify it? What are the chances of anyone doing that? It's not directly used in any API, so I think you have some leeway here even if strictly speaking it would be an ABI break. Question is also who allocates these structs? It's not like they'll be used on the stack. So I guess the main risk is some <=1.6 code allocating such a struct which would be too small then and being used in combination with libgstcontroller >= 1.7 which expects the struct to be larger.
commit d4f81fb4e62d34a4c1dabc65b23ede7ce7694c63 Author: Stefan Sauer <ensonic@users.sf.net> Date: Mon Sep 28 16:19:40 2015 +0200 controller: add the missing abi padding While this technically is an abi break, we decided to do this: 1) the struct is documented to be internal 2) the struct is alloced and freed inside the library 3) there are no public methods that receive or return instances 4) the only code known to use this struct are classes containd here
Just came across this when writing release notes. Is there any particular reason this was named _MONO rather than _MONOTONIC? Is 'mono' a common abbreviation for 'monotonic' used elsewhere? I think in general _MONOTONIC would be better, even if it's a bit longer.
Yes _MONOTONIC is more precise and _MONO is probably not a common abbreviation in this context. The enum names are getting close to the 80 column limits though (especially when used with the api). GST_INTERPOLATION_MODE_CUBIC_MONO{,TONIC} Do you want me to rename everything?
Renaming to MONOTONIC seems like a good idea
Okay, let me rename it ...
commit ab17881cf0e77c36098a5dd873e3b64d135e1eb1 Author: Stefan Sauer <ensonic@users.sf.net> Date: Tue Jan 5 13:57:12 2016 +0100 controller: rename new cubic interpolation mode Don't abbreviate to 'mono' and use 'monotonic' instead.