tracelib: proxypad handling comments and cleanups

Add more comments in the source. Add some ascii art in TODO. Remove a useless
check and error code-path. Use plain CAST more often.

2 files changed, 98 insertions, 53 deletions

diff --git a/TODO b/TODO
index 585be91..50b11b8 100644
--- a/TODO
+++ b/TODO
@@ -2,14 +2,6 @@
 == logging ==
 - cache more stuff (ghostpad resolving, right now it differs a bit for
   transmission and element stats)
-- revise the proxypad handling
-  - could/should we always account all data activity on proxy-pads to its parent
-    ghostpad
-
-- we seem to not fully catch the removal of proxy-pads
-  - the playbin2 example has pad_link_full/pad_unlink pairs with a proxypad,
-    but the proxypad is never appearing for anything else
-    (e.g. decodepad3_proxypad4)
 
 == wrap more methods ==
 - gst_pad_query (we have gst_element_query)
@@ -115,9 +107,8 @@ gsttlui is work-in-progress of a UI where we can inspect a running pipeline:
   - http://www.graphviz.org/pdf/libguide.pdf
 - plot the layout using goocanvas
 
-future ideas
-- proxy-pads of ghostpads are not properly handled
-- tracelib should log pad-direction and ev. is_ghostpad
+
+== future ideas ==
 - check if we can update properties and redraw easily
   - state changes on elements and pads
   - think how we can show buffers, events, queries and messages
@@ -128,3 +119,63 @@ future ideas
   - cpu usage
   - data transfer
 
+== design ==
+
+            +------+    +------+
+            | elem |    | elem |
+            |     src--sink    |
+            +------+    +------+
+         
++------------------+
+|   bin            |
+| +------+         |    +------+
+| | elem |         |    | elem |
+| |     src--prx--gho--sink    |
+| +------+         |    +------+
++------------------+
+                      
+                        +------------------+
+                        |           bin    |
+            +------+    |         +------+ |
+            | elem |    |         | elem | |
+            |     src--gho--prx--sink    | |
+            +------+    |         +------+ |
+                        +------------------+
+
++------------------+    +------------------+
+|   bin            |    |           bin    |
+| +------+         |    |         +------+ |
+| | elem |         |    |         | elem | |
+| |     src--prx--gho--gho--prx--sink    | |
+| +------+         |    |         +------+ |
++------------------+    +------------------+
+
++------------------------------+
+|     bin                      |
+| +------------------+         |
+| |   bin            |         |
+| | +------+         |         |    +------+
+| | | elem |         |         |    | elem |
+| | |     src--prx--gho--prx--gho--sink    |
+| | +------+         |         |    +------+
+| +------------------+         |
++------------------------------+
+
+
+                    e->e       b->e          e->b          b->b
+gst_pad_push        src,sink   src,gho,sink  src,gho,sink  src,gho,gho,sink
+gst_pad_pull_range  src,sink   sink,gho,src  sink,gho,src  sink,gho,gho,sink
+gst_pad_send_event
+gst_pad_get_caps
+gst_pad_set_caps
+
+- proxy-pads of ghostpads are not properly handled
+  - could/should we always account all data activity on proxy-pads to its parent
+    ghostpad
+
+== bugs ==
+- we seem to not fully catch the removal of proxy-pads
+  - the playbin2 example has pad_link_full/pad_unlink pairs with a proxypad,
+    but the proxypad is never appearing for anything else
+    (e.g. decodepad3_proxypad4)
+
diff --git a/src/gsttracelib.c b/src/gsttracelib.c
index 7fe643c..b991393 100644
--- a/src/gsttracelib.c
+++ b/src/gsttracelib.c
@@ -522,18 +522,28 @@ _get_element_stats (GstElement *element)
   return _get_bin_or_element_stats (element, elements);
 }
 
-/* FIXME: this looks a bit weired, check that we really want to do this */
+/* FIXME: this looks a bit weired, check that we really want to do this
+ *
+ * in: a normal pad
+ * out: the element
+ *
+ * in: a proxy pad
+ * out: the element that contains the peer of the proxy
+ *
+ * in: a ghost pad
+ * out: the bin owning the ghostpad
+ */
 static GstObject*
 _get_real_pad_parent (GstPad *pad)
 {
   GstObject *parent = GST_OBJECT_PARENT (pad);
 
   /* if parent of pad is a ghost-pad, then pad is a proxy_pad */
-  while (parent && GST_IS_GHOST_PAD (parent)) {
-    pad = GST_PAD (parent);
+  if (parent && GST_IS_GHOST_PAD (parent)) {
+    pad = GST_PAD_CAST (parent);
     parent = GST_OBJECT_PARENT (pad);
   }
-  /* if pad is a ghost-pad, then parent is a bin an it is the parent of a
+  /* if pad is a ghost-pad, then parent is a bin and it is the parent of a
    * proxy_pad */
   while (parent && GST_IS_GHOST_PAD (pad)) {
     pad = gst_ghost_pad_get_target (GST_GHOST_PAD (pad));
@@ -924,36 +934,28 @@ _do_transmission_stats(GstPad *pad, GstBuffer *buf, GstClockTimeDiff elapsed)
   if (!peer_pad)
     return;
 
-  /* FIXME: sync with _get_real_pad_parent() */
+  /* walk the ghost pad chain downstream to get the real pad */
   /* if parent of peer_pad is a ghost-pad, then peer_pad is a proxy_pad */
   parent = GST_OBJECT_PARENT (peer_pad);
   while (parent && GST_IS_GHOST_PAD (parent)) {
-    peer_pad = GST_PAD (parent);
+    peer_pad = GST_PAD_CAST (parent);
     /* if this is now the ghost pad, get the peer of this */
-    if (GST_IS_GHOST_PAD (peer_pad)) {
-      _get_pad_stats (peer_pad);
-      if (parent = GST_OBJECT_PARENT (peer_pad)) {
-        peer_stats = _get_bin_stats (GST_ELEMENT (parent));
-        bin_o_stats = g_slist_prepend (bin_o_stats, peer_stats);
-      }
-      peer_pad = GST_PAD_PEER (GST_GHOST_PAD (peer_pad));
-      parent = GST_OBJECT_PARENT (peer_pad);
-    }
-    else {
-      GsttlPadStats *pad_stats=_get_pad_stats (pad);
-
-      fprintf (stderr, "%" GST_TIME_FORMAT " parent of proxy pad %s is %s_%s, which is not a ghostpad\n",
-        GST_TIME_ARGS(elapsed), pad_stats->name, GST_DEBUG_PAD_NAME (peer_pad));
-      return;
+    _get_pad_stats (peer_pad);
+    if (parent = GST_OBJECT_PARENT (peer_pad)) {
+      peer_stats = _get_bin_stats (GST_ELEMENT_CAST (parent));
+      bin_o_stats = g_slist_prepend (bin_o_stats, peer_stats);
     }
+    peer_pad = GST_PAD_PEER (GST_GHOST_PAD_CAST (peer_pad));
+    parent = peer_pad ? GST_OBJECT_PARENT (peer_pad) : NULL;
   }
+  /* walk the ghost pad chain upstream to get the real pad */
   /* if peer_pad is a ghost-pad, then parent is a bin adn it is the parent of
    * a proxy_pad */
-  while (GST_IS_GHOST_PAD(peer_pad)) {
+  while (peer_pad && GST_IS_GHOST_PAD(peer_pad)) {
     _get_pad_stats (peer_pad);
     peer_stats = _get_bin_stats (GST_ELEMENT_CAST(parent));
     bin_i_stats = g_slist_prepend (bin_i_stats, peer_stats);
-    peer_pad = gst_ghost_pad_get_target (GST_GHOST_PAD(peer_pad));
+    peer_pad = gst_ghost_pad_get_target (GST_GHOST_PAD_CAST(peer_pad));
     parent = peer_pad ? GST_OBJECT_PARENT (peer_pad) : NULL;
   }
 
@@ -1032,34 +1034,26 @@ _do_element_stats(GstPad *pad, GstClockTimeDiff elapsed1, GstClockTimeDiff elaps
   if (!peer_pad)
     return;
 
-  /* FIXME: sync with _get_real_pad_parent() */
+  /* walk the ghost pad chain downstream to get the real pad */
   /* if parent of peer_pad is a ghost-pad, then peer_pad is a proxy_pad */
   parent = GST_OBJECT_PARENT (peer_pad);
-  while(parent && GST_IS_GHOST_PAD (parent)) {
-    peer_pad = GST_PAD (parent);
+  if (parent && GST_IS_GHOST_PAD (parent)) {
+    peer_pad = GST_PAD_CAST (parent);
     /* if this is now the ghost pad, get the peer of this */
-    if (GST_IS_GHOST_PAD (peer_pad)) {
-      _get_pad_stats (peer_pad);
-      if (parent = GST_OBJECT_PARENT (peer_pad)) {
-        _get_bin_stats (GST_ELEMENT (parent));
-      }
-      peer_pad = GST_PAD_PEER (GST_GHOST_PAD (peer_pad));
-      parent = GST_OBJECT_PARENT (peer_pad);
-    }
-    else {
-      GsttlPadStats *pad_stats=_get_pad_stats (pad);
-
-      printf("%" GST_TIME_FORMAT " parent of proxy pad %s is %s_%s, which is not a ghostpad\n",
-        GST_TIME_ARGS(elapsed), pad_stats->name, GST_DEBUG_PAD_NAME (peer_pad));
-      return;
+    _get_pad_stats (peer_pad);
+    if (parent = GST_OBJECT_PARENT (peer_pad)) {
+      _get_bin_stats (GST_ELEMENT_CAST (parent));
     }
+    peer_pad = GST_PAD_PEER (GST_GHOST_PAD_CAST (peer_pad));
+    parent = peer_pad ? GST_OBJECT_PARENT (peer_pad) : NULL;
   }
+  /* walk the ghost pad chain upstream to get the real pad */
   /* if peer_pad is a ghost-pad, then parent is a bin and it is the parent of
    * a proxy_pad */
-  while (GST_IS_GHOST_PAD(peer_pad)) {
+  while (peer_pad && GST_IS_GHOST_PAD(peer_pad)) {
     _get_pad_stats (peer_pad);
     _get_bin_stats (GST_ELEMENT_CAST(parent));
-    peer_pad = gst_ghost_pad_get_target (GST_GHOST_PAD(peer_pad));
+    peer_pad = gst_ghost_pad_get_target (GST_GHOST_PAD_CAST(peer_pad));
     parent = peer_pad ? GST_OBJECT_PARENT (peer_pad) : NULL;
   }