GNOME Bugzilla – Bug 757047
tracer: [API]: add register_hook_for_target.
Last modified: 2018-11-03 12:30:22 UTC
See commit message
Created attachment 314005 [details] [review] tracer: [API]: add register_hook_for_target. This will allow monitoring specific objects in a generic way. + Add a test + Update the dispatching routines.
Review of attachment 314005 [details] [review]: Just out of curiosity, do you have a tracer that uses this? What does it do? :) ::: gst/gsttracerutils.c @@ +286,3 @@ +static void +gst_tracing_register_hook_full (GstTracer * tracer, GQuark detail, Should this one be public then maybe, and be called gst_tracing_register_hook() without the _full? @@ -291,3 @@ - * @func: (scope async): the callback - * - * Register @func to be called when the trace hook @detail is getting invoked. Since: 1.8 everywhere
(In reply to Sebastian Dröge (slomo) from comment #2) > Review of attachment 314005 [details] [review] [review]: > > Just out of curiosity, do you have a tracer that uses this? What does it do? > :) Yes, we have one tracer per pad as GstValidatePadMonitor (and so on for other components). > ::: gst/gsttracerutils.c > @@ +286,3 @@ > > +static void > +gst_tracing_register_hook_full (GstTracer * tracer, GQuark detail, > > Should this one be public then maybe, and be called > gst_tracing_register_hook() without the _full? We already have; void gst_tracing_register_hook (GstTracer * tracer, const gchar * detail, GCallback func) I do not think that needs to be public. > @@ -291,3 @@ > - * @func: (scope async): the callback > - * > - * Register @func to be called when the trace hook @detail is getting > invoked. > > Since: 1.8 everywhere The Tracers are Since 1.8, I thinking adding it once is enough no?
Created attachment 314007 [details] [review] tracer: [API]: add register_hook_for_target. This will allow monitoring specific objects in a generic way. + Add a test + Update the dispatching routines.
Ok, makes sense then (also for the Since). Let's wait a bit if there are other opinions :)
Why is target a gpointer? That's not nice for bindings and in general. Is it not always a GObject or GstObject?
We might want to add tracers later for miniobjects
(In reply to Tim-Philipp Müller from comment #6) > Why is target a gpointer? That's not nice for bindings and in general. Is it > not always a GObject or GstObject? The tracer API is really not nice for bindings... we discussed that in the bug about that new API and decided optimisation was more important, and we could plug a binding friendly API on top.
I think there is still some misunderstanding how the tracer is supposed to be used. The tracer plugins will collect data and stream then. That's it. The code that analyzes the data is supposed to read that from the stream. While the app is running or post-mostem. The fact that the stream is now part of the gst-debug log is interrim. I've posted this bug a loooong time ago for discussion: https://bugzilla.gnome.org/show_bug.cgi?id=733188 If we start filtering in the hooks already, where do we start and where do we stop? I think the filtering should be done in the app. If we want filtering in the hook already we need to rethink the api to be more flexible (e.g. using a callback). Otherwise we're going to add more and more functions here. So if we want this, what about gst_tracing_register_filtered_hook (GstTracer * tracer, const gchar * detail, GCallback func,GstTracerFilterFunc filter) + the hook_id variant.
(In reply to Stefan Sauer (gstreamer, gtkdoc dev) from comment #9) > I think there is still some misunderstanding how the tracer is supposed to > be used. The tracer plugins will collect data and stream then. That's it. > The code that analyzes the data is supposed to read that from the stream. > While the app is running or post-mostem. This model does not really work for GstValidate where we do lot of behaviour checks at runtime, doing that post-mortem is gonna be really complex and not bring much. Also doing that with the current API is not gonna look good codewise in GstValidate. Having GstMonitors (which monitor one particular object) subclassing GstTracer makes everything clean and nice. > The fact that the stream is now part of the gst-debug log is interrim. I've > posted this bug a loooong time ago for discussion: > https://bugzilla.gnome.org/show_bug.cgi?id=733188 I do not see how that is related to the subject here. > If we start filtering in the hooks already, where do we start and where do > we stop? I think the filtering should be done in the app. If we want > filtering in the hook already we need to rethink the api to be more flexible > (e.g. using a callback). Otherwise we're going to add more and more > functions here. > > So if we want this, what about > gst_tracing_register_filtered_hook (GstTracer * tracer, const gchar * > detail, > GCallback func,GstTracerFilterFunc filter) That only works if we have the 'target' in the GstTracerFilterFunc prototype and we make the function look like: gst_tracing_register_filtered_hook (GstTracer * tracer, const gchar * detail, GCallback func, GstTracerFilterFunc filter, gpointer data)
(In reply to Thibault Saunier from comment #10) > (In reply to Stefan Sauer (gstreamer, gtkdoc dev) from comment #9) > > I think there is still some misunderstanding how the tracer is supposed to > > be used. The tracer plugins will collect data and stream then. That's it. > > The code that analyzes the data is supposed to read that from the stream. > > While the app is running or post-mostem. > > This model does not really work for GstValidate where we do lot of behaviour > checks at runtime, doing that post-mortem is gonna be really complex and not > bring much. Also doing that with the current API is not gonna look good > codewise in GstValidate. Having GstMonitors (which monitor one particular > object) subclassing GstTracer makes everything clean and nice. I am aware that the post-mosted analysis is not a good fit for you. > > > The fact that the stream is now part of the gst-debug log is interrim. I've > > posted this bug a loooong time ago for discussion: > > https://bugzilla.gnome.org/show_bug.cgi?id=733188 > > I do not see how that is related to the subject here. The idea was that e.g. you run the tracers in process and the analysis out-of-process or even on a different device. We want to affect the traced process as little as possible. Right now a trace-consumer that would like to trace a live app, would need to install a GstLogFunction and out-of-process is not yet supported. Due to these issues, people will jump to the conclussion that they need to write their own tracers. This is why I mentioned the ticket. Why I am back in european TZ we should discuss what you actually want to do :) > > > If we start filtering in the hooks already, where do we start and where do > > we stop? I think the filtering should be done in the app. If we want > > filtering in the hook already we need to rethink the api to be more flexible > > (e.g. using a callback). Otherwise we're going to add more and more > > functions here. > > > > So if we want this, what about > > gst_tracing_register_filtered_hook (GstTracer * tracer, const gchar * > > detail, > > GCallback func,GstTracerFilterFunc filter) > > That only works if we have the 'target' in the GstTracerFilterFunc prototype > and we make the function look like: > > gst_tracing_register_filtered_hook (GstTracer * tracer, const gchar * detail, > GCallback func, GstTracerFilterFunc > filter, > gpointer data) Yes, I've forgot the user_data.
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gstreamer/issues/134.