On 2015-02-24 15:33:53 +0000, Vittorio Giovara wrote:
> Enabled only in debug mode and only when run under Valgrind.
>
> Signed-off-by: Vittorio Giovara <[email protected]>
> ---
> This is a neat trick I found reading
> https://blog.mozilla.org/nnethercote/2011/01/11/using-valgrind-to-get-stack-traces/
>
> It should help in identifying where an error occurred if it is logged.
>
> I was looking for such a thing for some time, and other solutions like
> debugbreak https://github.com/scottt/debugbreak are simply too fragile.
>
> The function is a static import so there are no additional libraries or
> dependencies. Additionally it only appears on valgrind runs so it doesn't
> pollute normal log and it is possible to turn it off if desired.
>
> I am not sure if I should keep everything under a single CONFIG_ or if it's
> better to have the header check serpatedly (like in the proposed patch).
>
> Vittorio
>
> configure | 10 ++++++++++
> libavutil/log.c | 7 +++++++
> 2 files changed, 17 insertions(+)
>
> diff --git a/configure b/configure
> index c1d673c..4e8fd6e 100755
> --- a/configure
> +++ b/configure
> @@ -315,6 +315,8 @@ Developer options (useful when working on Libav itself):
> (group) and PROB the probability associated with
> NAME (default 0.5).
> --random-seed=VALUE seed value for --enable/disable-random
> + --disable-backtrace-printf when debug is enabled, do not print a backtrace
> + under Valgrind when an error is logged
The name is a little strange, log{-error}?-backtrace would be better. I
also would invert it to --enable-... so it's clear that it can be
enabled without debug. Whether it should be enabled with debug is
another question.
> NOTE: Object files are built at the place where configure is launched.
> EOF
> @@ -1242,6 +1244,7 @@ SUBSYSTEM_LIST="
> "
>
> CONFIG_LIST="
> + backtrace_printf
> $COMPONENT_LIST
> $EXAMPLE_LIST
> $EXTERNAL_LIBRARY_LIST
> @@ -1408,6 +1411,7 @@ HEADERS_LIST="
> sys_un_h
> sys_videoio_h
> unistd_h
> + valgrind_valgrind_h
> windows_h
> winsock2_h
> "
> @@ -3771,6 +3775,9 @@ enabled version3 && { enabled gpl && enable gplv3 ||
> enable lgplv3; }
>
> disabled optimizations || check_cflags -fomit-frame-pointer
>
> +enabled debug && { disabled backtrace_printf ||
> + { check_header valgrind/valgrind.h && enable backtrace_printf; } }
too complicated, log-backtrace should depend on valgrind_valgrind_h, the
'check_header valgrind/valgrind.h' can on it's own. If you want to
enabled it by default with debug, please use 'enabled debug &&
enable_weak log-backtrace'
I'm not convinced that it is the right thing to do.
> +
> enable_weak_pic() {
> disabled pic && return
> enable pic
> @@ -4629,6 +4636,9 @@ if enabled ppc; then
> echo "dcbzl available ${dcbzl-no}"
> fi
> echo "debug symbols ${debug-no}"
> +if enabled debug; then
I don't see the reason for this
> + echo "Valgrind backtrace ${backtrace_printf-no}"
Not sure if it should be printed
> +fi
> echo "optimize for size ${small-no}"
> echo "optimizations ${optimizations-no}"
> echo "static ${static-no}"
> diff --git a/libavutil/log.c b/libavutil/log.c
> index d38e40b..3d1d1ef 100644
> --- a/libavutil/log.c
> +++ b/libavutil/log.c
> @@ -40,6 +40,10 @@
> #include "internal.h"
> #include "log.h"
>
> +#if HAVE_VALGRIND_VALGRIND_H
> +#include <valgrind/valgrind.h>
> +#endif
> +
> static int av_log_level = AV_LOG_INFO;
> static int flags;
>
> @@ -161,6 +165,9 @@ void av_log_default_callback(void *avcl, int level, const
> char *fmt, va_list vl)
> }
> colored_fputs(av_clip(level >> 3, 0, 6), tint >> 8, line);
> av_strlcpy(prev, line, sizeof line);
> +
> + if (CONFIG_BACKTRACE_PRINTF && level <= AV_LOG_ERROR)
> + VALGRIND_PRINTF_BACKTRACE("");
Not sure how much overhead this has when not running under valgrind, it
might be nicer to have a flag/option for this. It's conceivable that it
would be useful for AV_LOG_WARNING too
Janne
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
