[webkit-reviews] review granted: [Bug 89350] [GStreamer] Use GST_DEBUG instead of LOG_VERBOSE : [Attachment 156081] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Aug 2 07:48:27 PDT 2012


Martin Robinson <mrobinson at webkit.org> has granted Philippe Normand
<pnormand at igalia.com>'s request for review:
Bug 89350: [GStreamer] Use GST_DEBUG instead of LOG_VERBOSE
https://bugs.webkit.org/show_bug.cgi?id=89350

Attachment 156081: Patch
https://bugs.webkit.org/attachment.cgi?id=156081&action=review

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=156081&action=review


Looks good to me, just a couple concerns that seem easy to fix before landing.

> Source/WebCore/platform/graphics/gstreamer/GStreamerUtilities.h:26
> +#define logMessageToGStreamer(...) do { \
> +    GST_DEBUG(__VA_ARGS__); \
> +    LOG_VERBOSE(Media, __VA_ARGS__); } while(0)
>  }

Don't most macros in WebKit FOLLOW_THIS_NAMING_CONVENTION? I don't think that
preprocessor macros exist in namespaces, so this should probably be at the top
of the file.

>
Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:189
> +	   GST_DEBUG_CATEGORY_INIT(webkit_media_player_debug,
"webkitmediaplayer", 0, "webkit media player");

Perhaps it would be good to have the string be "WebKit media player" ?


More information about the webkit-reviews mailing list