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 705189 - debug-dot-dump: Add the presence/state of a GstTask
debug-dot-dump: Add the presence/state of a GstTask
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal enhancement
: 1.3.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-07-31 07:33 UTC by Olivier Crête
Modified: 2014-02-26 08:17 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
debugutils: Print if there is a task started from a pad (1.83 KB, patch)
2013-07-31 07:33 UTC, Olivier Crête
needs-work Details | Review
debugutils: Print if there is a task started from a pad (1.91 KB, patch)
2013-12-06 02:59 UTC, Olivier Crête
committed Details | Review

Description Olivier Crête 2013-07-31 07:33:02 UTC
Created attachment 250521 [details] [review]
debugutils: Print if there is a task started from a pad

For those who don't know how each element is implemented, it's not obvious which ones have their own threads and which ones don't. Attached patch adds a [T] for started tasks and a [t] for paused tasks if the dot files request the state to be printed.

Am I right to take the object lock of the pad? I noticed the rest of the dot file logic never takes any lock. As I'm dereferencing a pointer, I'm afraid of non-atomic pointer writes could cause a crash otherwise.

Any stylistic preferences? Should we make it into another flag to enable it? Maybe change the color of the pad slightly ?
Comment 1 Sebastian Dröge (slomo) 2013-08-13 12:43:48 UTC
Review of attachment 250521 [details] [review]:

Good idea, I don't really have an opinion how it should be displayed though. Should be good enough that way I guess?

::: gst/gstdebugutils.c
@@ +169,3 @@
+    GST_OBJECT_LOCK (pad);
+    task = GST_PAD_TASK (pad);
+    GST_OBJECT_UNLOCK (pad);

Maybe take a new reference of the task here?

@@ +179,3 @@
+          break;
+        default:
+          g_assert_not_reached ();

This should probably not assert here, but instead silently do nothing
Comment 2 Olivier Crête 2013-12-06 02:59:58 UTC
Created attachment 263630 [details] [review]
debugutils: Print if there is a task started from a pad

Updated patch to keep the pad lock while getting the task and to remove
the assert.
Comment 3 Stefan Sauer (gstreamer, gtkdoc dev) 2014-02-25 20:17:00 UTC
Review of attachment 263630 [details] [review]:

Let do it. It would be nice if we could highlight the set of (pad-)nodes that are executed in the same thread, but thats not easy. I tried this once with a cluster, but those are always square and that makes the layout quite ugly.

It would be nice to add a layer with a legend though. I'll see if I can give that a try.
Comment 4 Olivier Crête 2014-02-26 08:13:46 UTC
Could do the grouping by color, but it's hard to know as some threads may not be started by a GstTask, for example, rtpsession creates a thread with g_thread_new().
Comment 5 Olivier Crête 2014-02-26 08:17:35 UTC
Pushed

commit e3d57dc11de3562448eba86a9b17b926682a9ec2
Author: Olivier Crête <olivier.crete@collabora.com>
Date:   Wed Jul 31 09:26:26 2013 +0200

    debugutils: Print if there is a task started from a pad
    
    https://bugzilla.gnome.org/show_bug.cgi?id=705189