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 760979 - tracers: Allow adding tracers programatically without defining environment variables
tracers: Allow adding tracers programatically without defining environment va...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
unspecified
Other All
: Normal blocker
: 1.7.90
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-01-22 10:32 UTC by Thibault Saunier
Modified: 2016-02-22 12:31 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gst: Concider gst as not initialized after deinit is called (3.66 KB, patch)
2016-01-22 10:32 UTC, Thibault Saunier
none Details | Review
tracer: Initialize GstTracer _priv_tracers and quarks unconditionnally (1.96 KB, patch)
2016-01-22 10:32 UTC, Thibault Saunier
none Details | Review
gst: Concider gst as not initialized after deinit is called (3.62 KB, patch)
2016-01-22 10:56 UTC, Thibault Saunier
none Details | Review
tracer: Initialize GstTracer _priv_tracers and quarks unconditionnally (2.11 KB, patch)
2016-01-22 13:24 UTC, Thibault Saunier
committed Details | Review

Description Thibault Saunier 2016-01-22 10:32:30 UTC
It allows us to use GstValidateRunner as a GstTracer in an application, and
not only through the GST_TRACER_PLUGINS mechanism.
Comment 1 Thibault Saunier 2016-01-22 10:32:34 UTC
Created attachment 319544 [details] [review]
gst: Concider gst as not initialized after deinit is called

Otherwise calling gst_init again will concider Gst is still initialized
and do nothing, which is wrong.

Also make sure to properly deinitialize GstValue on deinit().
Comment 2 Thibault Saunier 2016-01-22 10:32:39 UTC
Created attachment 319545 [details] [review]
tracer: Initialize GstTracer _priv_tracers and quarks unconditionnally

Some people might use tracer hooks even if GST_TRACER_PLUGINS is not
set.
Comment 3 Thibault Saunier 2016-01-22 10:56:25 UTC
Created attachment 319547 [details] [review]
gst: Concider gst as not initialized after deinit is called

Otherwise calling gst_init again will concider Gst is still initialized
and do nothing, which is wrong.

Also make sure to properly deinitialize GstValue on deinit().
Comment 4 Thibault Saunier 2016-01-22 12:45:17 UTC
Comment on attachment 319547 [details] [review]
gst: Concider gst as not initialized after deinit is called

After discussing it on IRC it looks like this is not handled on purpose (mainly not to have to handle all the corner cases it implies).
Comment 5 Stefan Sauer (gstreamer, gtkdoc dev) 2016-01-22 12:51:20 UTC
Review of attachment 319545 [details] [review]:

I'd like to understand your usecase first, so that we don#t break some assumtions in the code.
Comment 6 Stefan Sauer (gstreamer, gtkdoc dev) 2016-01-22 13:15:45 UTC
Review of attachment 319545 [details] [review]:

I don't like that with this change we unconditionally initialize the tracing. It was meant to be guarded by the env-var. Would it work if you do
g_setenv("GST_TRACERS", "", TRUE); before calling gst_init().
Comment 7 Thibault Saunier 2016-01-22 13:24:53 UTC
Created attachment 319552 [details] [review]
tracer: Initialize GstTracer _priv_tracers and quarks unconditionnally

Some people might use tracer hooks even if GST_TRACER_PLUGINS is not
set.
Comment 8 Thibault Saunier 2016-02-02 16:02:34 UTC
@Stefan, Are we good with that last patch? I think we agreed it was all right on IRC.
Comment 9 Sebastian Dröge (slomo) 2016-02-16 16:37:09 UTC
Ping?
Comment 10 Thibault Saunier 2016-02-16 17:03:12 UTC
(I am waiting for that to push my GstTracer based GstValidate branch)
Comment 11 Sebastian Dröge (slomo) 2016-02-17 07:58:32 UTC
The change makes sense to me at least. It allows to enable and add tracers independent of the environment variables via API. Just like we allow with the debugging system.
Comment 12 Sebastian Dröge (slomo) 2016-02-19 08:28:46 UTC
Note that adding tracers at a later time (when multiple threads are doing GStreamer things already) is racy and can easily cause crashes. That must be mentioned in the documentation.

See e.g. GST_TRACER_DISPATCH() accessing the hash table.
Comment 13 Sebastian Dröge (slomo) 2016-02-22 08:19:20 UTC
Let's get this in then so that the tracer enabled gst-validate can be merged and tested properly for a long enough time before 1.8.0?
Comment 14 Sebastian Dröge (slomo) 2016-02-22 08:19:52 UTC
Add the documentation improvements before pushing though.
Comment 15 Thibault Saunier 2016-02-22 12:30:06 UTC
Attachment 319552 [details] pushed as 7e5a892 - tracer: Initialize GstTracer _priv_tracers and quarks unconditionnally
Comment 16 Thibault Saunier 2016-02-22 12:31:12 UTC
(In reply to Sebastian Dröge (slomo) from comment #14)
> Add the documentation improvements before pushing though.

Did that.