GNOME Bugzilla – Bug 613081
info: make GST_PTR_FORMAT etc. work on win32, OSX and non-glibc systems
Last modified: 2013-04-13 10:19:33 UTC
On Windows, OSX and other systems (embedded builds etc.) neither register_printf_specifier() nor register_printf_function() are available. This makes GStreamer debug logs much less useful, since you don't get the caps serialised in the debug log. This applies to other GST_PTR_FORMAT arguments as well, but I think 95% of those are caps. Therefore I propose to add some new API that works like GST_LOG_CAPS ("get_caps returned: ", caps); GST_LOG_OBJECT_CAPS (pad, "get_caps returned: ", caps); etc. so people on other platforms are not left out in the cold.
Alternatively, we could handle %P right before calling g_strdup_vprintf() in gst_debug_message_get() by replacing all instances of %P in the pattern with the caps string. This would only work if %P is the first % instance, though. Or... implement %P in glib, since glib's printf implementation is used if glibc is not used. (Er, maybe. Don't know if that's true always.)
> Or... implement %P in glib, since glib's printf implementation is used if glibc > is not used. (Er, maybe. Don't know if that's true always.) Not sure that's always true (surely then we wouldn't problems with NULL string pointers crashing on win32?). There's a define now in glibconfig.h that tells us which implementation is used, but I don't think that's particularly useful here. Besides, it's not really nice to hog these specifiers as a library, just like setting up signal handlers isn't very nice, so if we can find a way to avoid that (in future versions), all the better, no?
Also, it would be really awesome if we could get rid of the %P specifier somehow. It's unportable, unstable glibc API and we can't use G_GNUC_PRINTF() on the debug function.
Any progress on this?
Can't we just have a *big* static array of *big* string buffers, a counter and a couple of functions like GST_DEBUG_FORMAT_CAPS(caps). The function behind the macro obtains the next array slot, inc the counter, format the parameter into the string buffer and return the reference. The counter will wrapp around on the number of slots. That is if we have 20 slots, we support log prints with up to 20 formatted args. There might be more slots in use due to threads. Still we should be able to get away with a large number of slots.
> Can't we just have a *big* static array of *big* string buffers, a counter and > a couple of functions like GST_DEBUG_FORMAT_CAPS(caps). That's possible, and could probably even be done without the artificial limitation using GPrivate or GStaticPrivate or so), but I think I'd still prefer dedicated API for this, for at least two reasons: - dedicated API (macros) makes this much more discoverable than GST_PTR_FORMAT or GST_DEBUG_FORMAT_CAPS(). A counter-argument to this would be the fact that many newbies just do GST_LOG("%s", gst_caps_to_string (caps)), so offering something along those lines, maybe a gst_caps_to_static_string() could be seen as more intuitive. - we log a lot of caps. This is really frequently used. It would be nice to have nice API for that, so the code looks nice. IMHO GST_FORMAT_TIME / GST_TIME_ARGS() often make things quite ugly, would be nice to avoid adding more of those. > Any progress? Not really, still not sure about the right API yet I think.
A gst_caps_to_static_string() seems simple enough no ? This is a major pain on osx, I'm forced to patch the code each time I want some caps with a gst_caps_to_string/g_free ... I'm willing to do a lot of the boring patching job if we can agree on an API... quickly ?
(In reply to comment #7) > A gst_caps_to_static_string() seems simple enough no ? cannot be used easily as t allocated a buffer and one has to free it.
That's why it's to_"static"_string rather then to string... I'm not sure how tim planned to do the static part of it... ? My first guess is to keep the string in the caps like GstStaticCaps does... And use it as a cache, each time we call to_static_string it could free the previous caps and generate a new one... and the caps would be responsible to destroy the string in the end... But that would break the ABI I guess , but might be an idea for 0.11 ?
Current plan is to simply add our own printf implementation for the debugging functions. http://cgit.freedesktop.org/~tpm/gstreamer/log/?h=gnulib-printf has some work-in-progress (though the cgit web interface seems to be a bit slow to pick up forced updates in the branch view)
What's missing for that gnulib-printf branch to get merged?
It's not complete yet :) The extension isn't hooked up yet (I've mostly done that locally), and we also need to add support for the old extension markers %P and %Q for binary compatibility.
*** Bug 696885 has been marked as a duplicate of this bug. ***
Updated http://cgit.freedesktop.org/~tpm/gstreamer/log/?h=gnulib-printf some more. Works now, at least for me. Appears to be 20-25% faster even (for decoding a flac file with uridecodebin to fakesink with log level 9 to /dev/null). Left to do: 1) someone needs to go through the list of HAVE_XYZ in the code, extract the configure checks from glib's configure.ac and add them to ours to make sure we check for everything we need to check for (alternatively, maybe we can just use GLib's g_snprintf and assume most issues covered). 2) build and test on 32-bit x86 3) build and test on Windows 4) build and test on OS/X
Well, let's try this and see what happens: commit 0e1dd050a56dcbfbd9583fc4f43ed3c3adc1a5f2 Author: Tim-Philipp Müller <tim@centricular.net> Date: Fri Apr 12 23:58:52 2013 +0100 printf: don't build if debugging subsystem was disabled commit 5299a0cd65e3233dd61a5c8862519be4961f1b27 Author: Tim-Philipp Müller <tim@centricular.net> Date: Wed Apr 10 11:51:37 2013 +0100 printf: deal with some of the HAVE_FOO used in the printf code Probably needs some more work for MSVC. commit 3c1d9c6d41e4f209af5be1baa933234d6ef51560 Author: Tim-Philipp Müller <tim.muller@collabora.co.uk> Date: Mon Apr 8 19:42:27 2013 +0100 printf: fix alloca use for windows with mingw32 Don't use just GLIB_HAVE_ALLOCA_H to check if alloca is available, that's just for the header. GLib may define alloca for us otherwise too irrespective of GLIB_HAVE_ALLOCA_H. Fixes compiler warning with mingw32: gst/printf/vasnprintf.c:73:0: warning: "alloca" redefined commit ff292d530c126191bf220f70a8d114aa8f54e4b9 Author: Tim-Philipp Müller <tim@centricular.net> Date: Sun Apr 7 20:11:21 2013 +0100 printf: enable and fix compiler warnings But suppress -Wformat-nonliteral warnings since sprintf is used with a runtime-generated format string in our vasnprintf implementation. commit 97b3948a9fd86da7f80b0703060a0a3324f899e4 Author: Tim-Philipp Müller <tim@centricular.net> Date: Sun Apr 7 18:21:00 2013 +0100 printf: fix up dodgy use of #if HAVE_FOO and #if !HAVE_FOO Should use #ifdef and #ifndef. commit 79d6b91e27fd187cdaad78728c91a59104a3c85d Author: Tim-Philipp Müller <tim@centricular.net> Date: Sun Apr 7 17:36:29 2013 +0100 printf: mark internal functions as internal commit 3778c1878cf59b486228fcf3c500a091e7607e71 Author: Tim-Philipp Müller <tim@centricular.net> Date: Sun Apr 7 17:29:02 2013 +0100 printf: skip pointer extension signifier chars after %p So they don't get printed after the serialised pointer string. commit 6d8a6470d59142ff1de3841894bb33e71d133566 Author: Tim-Philipp Müller <tim@centricular.net> Date: Sun Apr 7 17:21:10 2013 +0100 printf: don't leak serialised pointer extension strings commit 9a9b449c5f1e872b85b2888754fa41b5b4b271a2 Author: Tim-Philipp Müller <tim@centricular.net> Date: Sun Apr 7 17:02:55 2013 +0100 printf: handle old GST_PTR_FORMAT %P and GST_SEGMENT_FORMAT %Q defines too For binary backwards compatibility. commit 5803da553ca354362f3673503988745b7eb60607 Author: Tim-Philipp Müller <tim@centricular.net> Date: Sun Apr 7 16:41:40 2013 +0100 printf: make printf parser recognise our pointer extension format and call the hook to get a string for the pointer instead. https://bugzilla.gnome.org/show_bug.cgi?id=613081 commit fe7f7135e0399bd5a970164a5ac962c6800a69bd Author: Tim-Philipp Müller <tim@centricular.net> Date: Sat Mar 30 18:28:38 2013 +0000 printf: add infrastructure for pointer extensions hook Does not do anything yet. On a sidenote, we can't just use %p\001 or so to signal the extension because g-i complains about an invalid ascii character then, so have to resort to something more elaborate, such as %p\aA etc. https://bugzilla.gnome.org/show_bug.cgi?id=613081 commit 7b1994428074a05795d7fe4418e29f921c2c0a09 Author: Tim-Philipp Müller <tim@centricular.net> Date: Sat Mar 30 17:20:13 2013 +0000 info: use new internal printf for debug message printing and remove all the printf extension/specifier stuff for the system printf. Next we need to add back the custom specifiers to our own printf implementation. https://bugzilla.gnome.org/show_bug.cgi?id=613081 commit 8fc876f09fb4f76902cedcb2edb650fad9f76021 Author: Tim-Philipp Müller <tim@centricular.net> Date: Sat Mar 30 15:13:32 2013 +0000 printf: add our own printf implementation for debug logging We will add support for our own printf modifiers, so we can get nice debug log output on all operating systems irrespective of the specific libc version used. https://bugzilla.gnome.org/show_bug.cgi?id=613081