Bug 89660 - [GTK] Add support for graphviz debug output for the gstreamer video pipeline
Summary: [GTK] Add support for graphviz debug output for the gstreamer video pipeline
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Unspecified
: P2 Enhancement
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-06-21 07:14 PDT by Allan Xavier
Modified: 2012-06-25 10:48 PDT (History)
7 users (show)

See Also:


Attachments
Patch for supporting graphviz debug output (3.15 KB, patch)
2012-06-21 07:17 PDT, Allan Xavier
pnormand: review+
mrobinson: commit-queue-
Details | Formatted Diff | Diff
Patch for supporting graphviz debug output (3.19 KB, patch)
2012-06-21 11:05 PDT, Allan Xavier
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Allan Xavier 2012-06-21 07:14:18 PDT
GStreamer has a useful debugging feature where it can output a given pipeline to a Graphviz dot file. Having this feature enabled in WebKit would be helpful for debugging development/end user problems with the video pipeline.

Some more information on this feature (along with an example) can be found in the GStreamer SDK documentation http://docs.gstreamer.com/display/GstSDK/Basic+tutorial+11%3A+Debugging+tools

The following patch will output a dot file for errors and state changes in the video pipeline when the environment variable GST_DEBUG_DUMP_DOT_DIR is set. This is similar to how gst-launch implements the same functionality.
Comment 1 Allan Xavier 2012-06-21 07:17:09 PDT
Created attachment 148794 [details]
Patch for supporting graphviz debug output
Comment 2 Philippe Normand 2012-06-21 08:34:41 PDT
Let me know if you need the commit-queue to land this patch.
Comment 3 Martin Robinson 2012-06-21 08:39:37 PDT
Comment on attachment 148794 [details]
Patch for supporting graphviz debug output

View in context: https://bugs.webkit.org/attachment.cgi?id=148794&action=review

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:799
> +            // Construct a filename for the graphviz dot file output

Nit: You're missing a period at the end of this sentence.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:801
> +            dotFileName = g_strdup_printf("webkit-video.%s_%s", gst_element_state_get_name(oldState), gst_element_state_get_name(newState));

It's probably better to use 

CString dotFileName = String::format("webkit-video.%s_%s", gst_element_state_get_name(oldState), gst_element_state_get_name(newState)).utf8();

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:803
> +            GST_DEBUG_BIN_TO_DOT_FILE_WITH_TS(GST_BIN(m_playBin), GST_DEBUG_GRAPH_SHOW_ALL, dotFileName);

This would become:
GST_DEBUG_BIN_TO_DOT_FILE_WITH_TS(GST_BIN(m_playBin), GST_DEBUG_GRAPH_SHOW_ALL, dotFileName.data());
Comment 4 Allan Xavier 2012-06-21 11:05:46 PDT
Created attachment 148844 [details]
Patch for supporting graphviz debug output

This revised patch has the following changes:

* The improvements recommended by Martin in comment #3.
* The string formatting call is now split onto multiple lines for better readability.
* The pendingState variable has been removed (gst_message_parse_state_changed can handle null arguments).
Comment 5 WebKit Review Bot 2012-06-25 10:48:00 PDT
Comment on attachment 148844 [details]
Patch for supporting graphviz debug output

Clearing flags on attachment: 148844

Committed r121169: <http://trac.webkit.org/changeset/121169>
Comment 6 WebKit Review Bot 2012-06-25 10:48:06 PDT
All reviewed patches have been landed.  Closing bug.