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 638848 - new debugspy element
new debugspy element
Status: RESOLVED WONTFIX
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal enhancement
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2011-01-06 17:45 UTC by Guillaume Emont (guijemont)
Modified: 2018-05-04 09:42 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
identity: add an md5sum-compute property (4.05 KB, patch)
2011-01-06 17:45 UTC, Guillaume Emont (guijemont)
none Details | Review
New debugspy element (11.43 KB, patch)
2011-01-21 18:49 UTC, Guillaume Emont (guijemont)
none Details | Review
debugspy: add new element (11.14 KB, patch)
2011-02-25 19:09 UTC, Guillaume Emont (guijemont)
committed Details | Review
debugspy: added csv export code (7.62 KB, patch)
2011-02-25 19:10 UTC, Guillaume Emont (guijemont)
needs-work Details | Review
debugspy: added property csv-output-path (4.07 KB, patch)
2011-02-25 19:10 UTC, Guillaume Emont (guijemont)
needs-work Details | Review

Description Guillaume Emont (guijemont) 2011-01-06 17:45:40 UTC
Created attachment 177677 [details] [review]
identity: add an md5sum-compute property

If enabled, the md5sum-compute property triggers the computation of the md5sum
of the data of each buffer, output in last-message. Works only if silent is
FALSE.

Can be very useful for debugging purpose (e.g. to easily identify badly decoded frames in different conditions, such as playing backwards).
Comment 1 David Schleef 2011-01-07 00:05:41 UTC
Obviously, we have other debugging capability in indentity (and fakesink), but I'd prefer to move these extras to the debugutils plugins in -good and -bad.  This is pretty minor, as the actual code is in glib.

We already have a checksumsink element in -bad/gst/debugutils.  It should probably post an element message instead of using g_print(), however.

SHA1 is preferred over MD5 in new applications.  In any case, referring to MD5 specifically in property names is not a good idea.
Comment 2 Sebastian Dröge (slomo) 2011-01-11 17:08:18 UTC
Yes, agreed. We should move all these debugging things into debugutils elements... and make them emit proper GstMessages
Comment 3 Guillaume Emont (guijemont) 2011-01-14 16:07:00 UTC
Thanks for making me realise there is a checksumsink, it somehow passed under my radar...
Unfortunately, checksumsink is a sink and not a "filter" like identity, which make it less practical for some tasks. Does the world need a "debugidentity" or "debugfilter" for such things?
Will try to have a look at this
Comment 4 David Schleef 2011-01-15 22:15:09 UTC
It's a sink because I needed a sink for debugging.  If you need a filter, then I recommend writing it as a filter.

Ideally, it would be cool to have both a filter and sink that have the same properties and work the same way, but that would be more work than typically goes into debugutils in -bad.
Comment 5 Guillaume Emont (guijemont) 2011-01-15 23:27:10 UTC
I've started hacking a bit on a filter, though I'm on a really early stage now. Any advice on how to easily share code between a filter and a sink? Else I might just do a filter that provides all the debugging features I want, as, at least in a gst-launch case, it's easy to do filter ! fakesink. Using a debug sink in the middle of something sounds more complicated (I guess it can be done with a tee, but it sounds less obvious).
Comment 6 David Schleef 2011-01-15 23:42:03 UTC
I think a filter is more than sufficient.  If someone wants a sink, they can write it themselves.  :)
Comment 7 Sebastian Dröge (slomo) 2011-01-16 12:21:05 UTC
And if someone wants a sink... just put a fakesink after the filter element ;)
Comment 8 Guillaume Emont (guijemont) 2011-01-21 18:49:32 UTC
Created attachment 178988 [details] [review]
New debugspy element

Here's a new debugspy element.
It provides the same information than identity with its notify on last-message, but in a programmer-friendly way, while still being easily accessible with a simple gst-launch -m.
Comment 9 Guillaume Emont (guijemont) 2011-02-25 19:09:36 UTC
Created attachment 181939 [details] [review]
debugspy: add new element

Here is a new version, split in 3 patches, adding a new csv-output-path property that allows you to output buffer information in a file that is easy to parse.
Comment 10 Guillaume Emont (guijemont) 2011-02-25 19:10:15 UTC
Created attachment 181940 [details] [review]
debugspy: added csv export code
Comment 11 Guillaume Emont (guijemont) 2011-02-25 19:10:43 UTC
Created attachment 181941 [details] [review]
debugspy: added property csv-output-path
Comment 12 Sebastian Dröge (slomo) 2011-05-26 07:58:53 UTC
Comment on attachment 181939 [details] [review]
debugspy: add new element

commit 555959a852ae594f7eee56bec881d294ce3db5e8
Author: Guillaume Emont <gemont@igalia.com>
Date:   Fri Jan 14 17:42:50 2011 +0100

    debugspy: add new element
    
    This element allows you to get information about buffers with bus messages. 
    provides the same kind of information as identity does through a notify sign
    on a string property, but in a more programmer-friendly way.
Comment 13 Sebastian Dröge (slomo) 2011-05-26 08:02:43 UTC
Review of attachment 181940 [details] [review]:

::: gst/debugutils/csvexporter.c
@@ +79,3 @@
+
+  if (value)
+    /* Will gst_value_serialize always do something that makes sense in csv? */

No it doesn't, the output will also contain commas (at least for the caps) which will conflict with the CSV commas

@@ +85,3 @@
+    content = g_strdup (" ");
+
+  return write_cell (exporter, content, &exporter->current_error);

You're leaking "content" here

@@ +98,3 @@
+
+  /* NOTE: we're assuming the foreach will always end in writing fields in the
+   * same order, but that might be wrong */

That's a valid assumption with the current implementation of GstStructure only
Comment 14 Sebastian Dröge (slomo) 2011-05-26 08:09:59 UTC
Review of attachment 181941 [details] [review]:

::: gst/debugutils/gstdebugspy.c
@@ +239,3 @@
+
+  debugspy->exporter = csv_exporter_new_for_file (path);
+  debugspy->csv_output_path = g_strdup (path);

You're leaking this string, free it in GObject::finalize please
Comment 15 Guillaume Emont (guijemont) 2011-05-27 14:36:45 UTC
(In reply to comment #13)
> Review of attachment 181940 [details] [review]:
> 
> ::: gst/debugutils/csvexporter.c
> @@ +79,3 @@
> +
> +  if (value)
> +    /* Will gst_value_serialize always do something that makes sense in csv?
> */
> 
> No it doesn't, the output will also contain commas (at least for the caps)
> which will conflict with the CSV commas
> 
I guess I should protect that between quotes. Any idea if caps (or anything else) could contain quotes?
Comment 16 Sebastian Dröge (slomo) 2011-05-30 05:52:27 UTC
Yes, caps fields can be strings, which could contain all valid UTF8 characters.
Comment 17 Sebastian Dröge (slomo) 2013-08-23 10:52:49 UTC
Guillaume?
Comment 18 Sebastian Dröge (slomo) 2018-05-04 09:42:03 UTC
Let's close this then as there seems to be no interest in taking up this work and getting it ready to be merged.