[Bug 773463] core(debug): hard to distinguish related log at multi-instance env

[GStreamer (GNOME Bugzilla)]

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

--- Comment #16 from Tim-Philipp Müller <t.i.m at zen.co.uk> ---
Comment on attachment 341696
  --> https://bugzilla.gnome.org/attachment.cgi?id=341696
info: Give more control to use about how to print objects

Haven't looked at the meat of the patch yet, just some quick drive-by comments:
>--- a/gst/gst_private.h
>+++ b/gst/gst_private.h
> 
>+/* GstObject debugging representation level */
>+gint object_repr_level;

Don't think this will work properly everywhere. I think this should be

  extern gint object_repr_level;

in the header file and then

  gint object_repr_level;

in gstinfo.c or wherever.



>--- a/gst/gstbin.c
>+++ b/gst/gstbin.c

>+void
>+gst_bin_update_children_repr (GstObject * object, GstObject * parent)
>+{
>+#ifndef GST_DISABLE_GST_DEBUG
>+  ...
>+#endif
>+}

I think these #ifndef #endif should be outside of the entire function block and
there should be a #define for the function in the header that expands for
nothing if the logging system is disabled (and no prototype of course).

The reason to do it that way is that if you have code like below, the run-time
object run-time checks will still be executed even though the function is
empty. We don't want that in that case, we want it all to be a no-op I think.

>+  if (result)
>+    gst_bin_update_children_repr (GST_OBJECT (element), GST_OBJECT (bin));

We know here that the objects are okay, right? So maybe GST_OBJECT_CAST to
avoid the run-time check.


>+  gst_object_ref (element);
>   GST_TRACER_BIN_REMOVE_PRE (bin, element);
>   result = bclass->remove_element (bin, element);
>   GST_TRACER_BIN_REMOVE_POST (bin, result);
> 
>+  gst_bin_update_children_repr (GST_OBJECT_CAST (element),
>+      result ? NULL : GST_OBJECT_CAST (bin));
>+  gst_object_unref (element);

Why do we not just set it to NULL if result was TRUE? Wouldn't that keep it as
before if not?

I'm not a big fan of the "Repr" name, but not so important for now.

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