[Bug 733187] integrating the tracer branch

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=733187

--- Comment #34 from Tim-Philipp Müller <t.i.m at zen.co.uk> ---
Sorry if I'm late to the party, just a few questions/comments/observations:

- is gsttracerutils.h really supposed to be an installed header? (contains lots
of internal / priv_gst_* stuff) Are the dispatch macros supposed to be public?
they can only be used internally in GStreamer..

- gst_tracer_log_trace() takes a GstStructure, it might be nice to have a
variant that takes a vararg function so tracers don't have to do the
gst_structure_new(..) dance. The other reason the vararg approach might be nice
is that it gives us more options later to stuff the contents into something
else or process/serialise them more efficiently. Additionally, I wonder if it'd
make sense to specify explicitly what types are accepted (you seem to use
GST_TYPE_STRUCTURE a lot for nesting, which seems unfortunate in this context
to make it generic). GVariant would be perfect for this, only that unlike
GstStructure it doesn't make fieldnames mandatory, although we could just
require GVariant with key/value tuples like we do in GstStructure. I guess we
can always add some other format in future if we want to.

- gst_tracer_log_trace() is not annotated/documented. Is it supposed to be
*the* standard function by which tracers communicate results back, or just
something simple for now that always dumps the data into the debug log?

- does gst_tracing_register_hook_id() make sense as public API?
    - it's not performance-critical, so just passing strings is fine
    - quark table is not exported (good thing too really?)
    - hook id quarks are run-time generated and basically random instead of
constant, so an enum might be better if one really wants something like this
(like the old GstTracerHookId which was removed again)
    - tracer hooks are looked up in a hash table anyway, might just as well use
the strings directly as key then if we don't have an array or such

I get the sense that this is all done to allow code outside of GStreamer core
(say, libgstgl) to add additional hooks, but this all doesn't seem quite right
to me yet.

- is hook callback registration bindings friendly as is? (no user_data +
user_data_weak_notify; GSignal uses GClosure, no?)

- do we have a canonical list somewhere of implemented hooks + arguments they
will get? If not, that should probably be added to the docs of
gst_tracing_register_hook() ? Is gsttraceutils.h that list?

- core tracers seem to put the thread ID into a guint by casting the result of
g_thread_self(), don't think that's right

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.