After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 737616 - timedvaluecontrolsource: Add some signals about values changes
timedvaluecontrolsource: Add some signals about values changes
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
unspecified
Other All
: Normal enhancement
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-09-29 19:19 UTC by Thibault Saunier
Modified: 2014-10-22 16:10 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
timedvaluecontrolsource: Add some signals about values changes (7.26 KB, patch)
2014-09-29 19:19 UTC, Thibault Saunier
none Details | Review
Make sure not to held the TimeValueControlSource.lock when emitting signals. (9.31 KB, patch)
2014-09-30 09:11 UTC, Thibault Saunier
none Details | Review
Avoid breaking ABI making GstControlPoint a Boxed type (8.13 KB, patch)
2014-09-30 13:23 UTC, Thibault Saunier
accepted-commit_now Details | Review
Update fixing typos (8.13 KB, patch)
2014-10-01 13:30 UTC, Thibault Saunier
none Details | Review
:%s/_signals/gst_timed_value_control_source_signals/g (8.38 KB, patch)
2014-10-02 08:18 UTC, Thibault Saunier
committed Details | Review

Description Thibault Saunier 2014-09-29 19:19:01 UTC
In order for user to be able to track changes in the value set in
GstTimedValueControlSource the following signals have been added:
 * value-added
 * value-removed
 * value-changed

To be able to use a GstTimedValue to be marshalled into the signals,
the GstTimedValue structure is now registerd as a GBoxed type.

New API:
~~~~~~~
  * GstTimedValueControlSource::value-added
  * GstTimedValueControlSource::value-removed
  * GstTimedValueControlSource::value-added
Comment 1 Thibault Saunier 2014-09-29 19:19:05 UTC
Created attachment 287395 [details] [review]
timedvaluecontrolsource: Add some signals about values changes
Comment 2 Thibault Saunier 2014-09-30 09:11:19 UTC
Created attachment 287431 [details] [review]
Make sure not to held the TimeValueControlSource.lock when emitting signals.

Also this actually break the GstControlPoint ABI, I am wondering why this structure is public at all?
It is actually usable nowhere and is not in the document for 1.0, to me it has totally been replaced
with GstTimedValue and we should just concider that structure internal.
Comment 3 Thibault Saunier 2014-09-30 13:23:06 UTC
Created attachment 287446 [details] [review]
Avoid breaking ABI making GstControlPoint a Boxed type
Comment 4 Stefan Sauer (gstreamer, gtkdoc dev) 2014-10-01 13:20:14 UTC
Review of attachment 287446 [details] [review]:

::: libs/gst/controller/gsttimedvaluecontrolsource.c
@@ +344,1 @@
       g_sequence_remove (iter);

I want g_sequence_steal()

@@ +469,3 @@
+  /**
+   * GstTimedValueControlSource::value-changed
+   * @self: The #GstTimedValueControlSource on which a #GstTimedValue as changed

s/as/has/

@@ +484,3 @@
+  /**
+   * GstTimedValueControlSource::value-added
+   */

also here and below
Comment 5 Thibault Saunier 2014-10-01 13:30:01 UTC
Created attachment 287516 [details] [review]
Update fixing typos

> I want a g_sequence_steal

Indeed, that would much nicer!
Comment 6 Tim-Philipp Müller 2014-10-02 07:59:35 UTC
Minor style comment: could we please avoid adding new functions/variable that use a leading underscore for 'local' variables/functions? It's very ugly.
Comment 7 Thibault Saunier 2014-10-02 08:18:45 UTC
Created attachment 287562 [details] [review]
:%s/_signals/gst_timed_value_control_source_signals/g

Even though tastes are pretty personnal... I personnaly prefer to have short
variable names when possible, and knowing that the variable/function is static
only by seeing its name is something I find nice ;)