GNOME Bugzilla – Bug 638848
new debugspy element
Last modified: 2018-05-04 09:42:03 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).
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.
Yes, agreed. We should move all these debugging things into debugutils elements... and make them emit proper GstMessages
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
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.
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).
I think a filter is more than sufficient. If someone wants a sink, they can write it themselves. :)
And if someone wants a sink... just put a fakesink after the filter element ;)
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.
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.
Created attachment 181940 [details] [review] debugspy: added csv export code
Created attachment 181941 [details] [review] debugspy: added property csv-output-path
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.
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
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
(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?
Yes, caps fields can be strings, which could contain all valid UTF8 characters.
Guillaume?
Let's close this then as there seems to be no interest in taking up this work and getting it ready to be merged.