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 763142 - API: add async property change notification - gst_element_add_property_notify_watch()
API: add async property change notification - gst_element_add_property_notify...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal enhancement
: 1.9.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-03-05 18:05 UTC by Tim-Philipp Müller
Modified: 2016-04-08 12:29 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
element: add API to get property change notifications via messages (11.78 KB, patch)
2016-03-05 18:05 UTC, Tim-Philipp Müller
none Details | Review
tools: gst-launch: use new async property change notification API (2.84 KB, patch)
2016-03-05 18:06 UTC, Tim-Philipp Müller
none Details | Review
element: add API to get property change notifications via messages [v2] (18.60 KB, patch)
2016-03-26 13:44 UTC, Tim-Philipp Müller
committed Details | Review
tools: gst-launch: use new async property change notification API [v2] (2.91 KB, patch)
2016-03-26 13:44 UTC, Tim-Philipp Müller
committed Details | Review

Description Tim-Philipp Müller 2016-03-05 18:05:32 UTC
Created attachment 323156 [details] [review]
element: add API to get property change notifications via messages

I always thought it'd be nice if we had some helper API to receive GObject property notifications such as playbin notify::volume via the bus instead of callbacks from a non-application thread.

This adds API for that.

Open question: should we merge add_property_notify_watch() and _add_deep_...() ? That would mean changing the gboolean include_value into a flags argument with flags for include_value and recursive notification.

---
    
Be notified in the application thread via bus messages about
notify::* and deep-notify::* property changes, instead of
having to deal with it in a non-application thread.
    
API: gst_element_add_property_notify_watch()
API: gst_element_add_property_deep_notify_watch()
API: gst_element_remove_property_notify_watch()
API: gst_message_new_property_notify()
API: gst_message_parse_property_notify()
API: GST_MESSAGE_NOTIFY_PROPERTY
Comment 1 Tim-Philipp Müller 2016-03-05 18:06:33 UTC
Created attachment 323157 [details] [review]
tools: gst-launch: use new async property change notification API
Comment 2 Thibault Saunier 2016-03-05 23:55:34 UTC
Sounds like a very nice idea (in pitivi for example we got to always marshall properties changes to the app thread etc.. and I believe it is a problem other apps have) :)

Just wonder if the deep_notify watches should not rather go to the ChildProxy interface?
Comment 3 Sebastian Dröge (slomo) 2016-03-06 08:14:47 UTC
Review of attachment 323156 [details] [review]:

Very good idea. I think keeping deep and non-deep separate is fine :)

(While we're at it, a deep-element-added/removed is something we should also try to get in for 1.10 finally...)

::: gst/gstmessage.h
@@ +598,3 @@
+/* PROPERTY_NOTIFY */
+GstMessage *    gst_message_new_property_notify   (GstObject * src, const gchar * property_name, GValue * val) G_GNUC_MALLOC;
+void            gst_message_parse_property_notify (GstMessage * message, const gchar ** property_name, const GValue ** value);

Maybe add the object here too? GST_MESSAGE_SRC() basically but it would be easier to find and often you probably want to know the sender.
Comment 4 Sebastian Dröge (slomo) 2016-03-06 08:16:27 UTC
(In reply to Thibault Saunier from comment #2)
> Sounds like a very nice idea (in pitivi for example we got to always
> marshall properties changes to the app thread etc.. and I believe it is a
> problem other apps have) :)
> 
> Just wonder if the deep_notify watches should not rather go to the
> ChildProxy interface?

deep-notify is a GstObject thing, GstChildProxy operates on GObject.
Comment 5 Tim-Philipp Müller 2016-03-26 13:44:11 UTC
Created attachment 324791 [details] [review]
element: add API to get property change notifications via messages [v2]

v2:
 - added gtk-doc blurb for gst_message_parse_property_notify()

 - added GstObject ** out argument to gst_message_parse_property_notify()
   (should this be a GObject instead?)

 - fix property name leak in parse_property_name()

 - add unit test

-----
    Be notified in the application thread via bus messages about
    notify::* and deep-notify::* property changes, instead of
    having to deal with it in a non-application thread.
    
    API: gst_element_add_property_notify_watch()
    API: gst_element_add_property_deep_notify_watch()
    API: gst_element_remove_property_notify_watch()
    API: gst_message_new_property_notify()
    API: gst_message_parse_property_notify()
    API: GST_MESSAGE_PROPERTY_NOTIFY
Comment 6 Tim-Philipp Müller 2016-03-26 13:44:48 UTC
Created attachment 324792 [details] [review]
tools: gst-launch: use new async property change notification API [v2]
Comment 7 Tim-Philipp Müller 2016-04-08 12:28:59 UTC
commit d09dc8cbc0c72dde640a29134e74760d93f680ae
Author: Tim-Philipp Müller <tim@centricular.com>
Date:   Sat Mar 5 17:51:01 2016 +0000

    tools: gst-launch: use new async property change notification API
    
    https://bugzilla.gnome.org/show_bug.cgi?id=763142

commit 6e3fb7af523f11ecaff63702d24723cb44e5d625
Author: Tim-Philipp Müller <tim@centricular.com>
Date:   Sat Mar 5 14:12:36 2016 +0000

    element: add API to get property change notifications via messages
    
    Be notified in the application thread via bus messages about
    notify::* and deep-notify::* property changes, instead of
    having to deal with it in a non-application thread.
    
    API: gst_element_add_property_notify_watch()
    API: gst_element_add_property_deep_notify_watch()
    API: gst_element_remove_property_notify_watch()
    API: gst_message_new_property_notify()
    API: gst_message_parse_property_notify()
    API: GST_MESSAGE_PROPERTY_NOTIFY
    
    https://bugzilla.gnome.org/show_bug.cgi?id=763142