[PATCH] avutil: expand the size of LINE_SZ to 2048.
It's possible to make FFMPEG_CONFIGURATION exceed the 1024 limit, causing a cosmetic issue when invoking the ff* binaries. 2048 should be more than enough to fit it comfortably. --- libavutil/log.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavutil/log.c b/libavutil/log.c index 29ad2d0..9ad6875 100644 --- a/libavutil/log.c +++ b/libavutil/log.c @@ -39,7 +39,7 @@ #include "internal.h" #include "log.h" -#define LINE_SZ 1024 +#define LINE_SZ 2048 static int av_log_level = AV_LOG_INFO; static int flags; -- 1.8.1.2
On Tue, Aug 06, 2013 at 08:55:49PM -0400, Stephen Hutchinson wrote:
It's possible to make FFMPEG_CONFIGURATION exceed the 1024 limit, causing a cosmetic issue when invoking the ff* binaries. 2048 should be more than enough to fit it comfortably. --- libavutil/log.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
maybe the way FFMPEG_CONFIGURATION is printed could be changed so its nicer formated and then also is printed in smaller parts not needing this. I dont think 2048 is enough, one could arbitrarily repeat --enables or list all codecs explicitly or have very long pathes, ... and always increasing LINE_SZ would eventually cause problems with embeded systems that have only little stack space (such do exist AFAIK) [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The real ebay dictionary, page 3 "Rare item" - "Common item with rare defect or maybe just a lie" "Professional" - "'Toy' made in china, not functional except as doorstop" "Experts will know" - "The seller hopes you are not an expert"
On Wed, Aug 7, 2013 at 4:49 AM, Michael Niedermayer <michaelni@gmx.at>wrote:
maybe the way FFMPEG_CONFIGURATION is printed could be changed so its nicer formated and then also is printed in smaller parts not needing this.
I dont think 2048 is enough, one could arbitrarily repeat --enables or list all codecs explicitly or have very long pathes, ...
and always increasing LINE_SZ would eventually cause problems with embeded systems that have only little stack space (such do exist AFAIK)
I was thinking that would be the problem. My other inclination would be to remove the configuration report from a plain invocation, but add an option under help that would display it, optionally in a better-formatted way (like one option per line or grouped into categories like the options appear in configure). But I wasn't quite sure where to look in the code to find how to do that.
On 2013-08-07 05:11 -0400, Stephen Hutchinson wrote:
On Wed, Aug 7, 2013 at 4:49 AM, Michael Niedermayer <michaelni@gmx.at>wrote:
maybe the way FFMPEG_CONFIGURATION is printed could be changed so its nicer formated and then also is printed in smaller parts not needing this.
I dont think 2048 is enough, one could arbitrarily repeat --enables or list all codecs explicitly or have very long pathes, ...
and always increasing LINE_SZ would eventually cause problems with embeded systems that have only little stack space (such do exist AFAIK)
I was thinking that would be the problem. My other inclination would be to remove the configuration report from a plain invocation, but add an option under help that would display it, optionally in a better-formatted way (like one option per line or grouped into categories like the options appear in configure). But I wasn't quite sure where to look in the code to find how to do that.
It might be rather unfortunate to remove the configure line from normal invocation. It speeds up trouble shooting and can be helpful when working on bug reports. Printing it in a nicer way and splitting over multiple lines sounds like a good idea though. But try to do it in the simplest and robust way you can think of. Over-engineering won't add much value here and some trouble in the worst case. Alexander
On Wed, Aug 7, 2013 at 8:02 PM, Alexander Strasser <eclipse7@gmx.net> wrote:
It might be rather unfortunate to remove the configure line from normal invocation. It speeds up trouble shooting and can be helpful when working on bug reports.
Printing it in a nicer way and splitting over multiple lines sounds like a good idea though. But try to do it in the simplest and robust way you can think of. Over-engineering won't add much value here and some trouble in the worst case.
As a compromise, the solution I came up with will only disable the configuration line from normal invocation if it's in danger of hitting the character limit, since if it did that there's no telling what kind of information would get truncated. It'll notify the user to use the new -buildconf option to see that information instead. If the size of FFMPEG_CONFIGURATION is smaller than the limit (currently set at 1000 characters so that there's no danger of it actually hitting 1024), then it still displays like normal. Unfortunately, my ideas about how to get it to print one option per line didn't really pan out since I was just messing around with sed*, and there's no way of predicting that sed's behavior will remain the same across the different versions of it used on various platforms (and I even encountered this; with one method, it'd screw up on Linux but work reasonably okay on OSX, and with a different method, exactly the reverse). So for right now it just looks near-exactly the same as the current 'configuration:' line, just printed after the basic report where it's not in danger of colliding with any other text. It's set up to where a better idea on formatting the output of -buildconf can probably be dropped into the print_buildconf function and just work. *the way that one-per-line can work this way is creating a secondary variable, I went with FFMPEG_CONFIGURATION_FORMATTED, that forces a literal '///n ' into the define between each parameter. av_log/printf can deal with it correctly if the string is reproduced exactly like that. But there's too much wiggle-room that can cause it to insert real newlines into the define, and that doesn't work (everything still shows up, but the newlines are erased and the parameters are all spaced apart too much as a result). I'll send the patches momentarily.
Currently, its output is exactly the same as the 'configuration:' line when invoking one of the binaries. --- cmdutils.c | 15 +++++++++++++++ cmdutils.h | 7 +++++++ cmdutils_common_opts.h | 1 + 3 files changed, 23 insertions(+) diff --git a/cmdutils.c b/cmdutils.c index 6eb093d..fed8213 100644 --- a/cmdutils.c +++ b/cmdutils.c @@ -1066,6 +1066,13 @@ static void print_program_info(int flags, int level) av_log(NULL, level, "%sconfiguration: " FFMPEG_CONFIGURATION "\n", indent); } +static void print_buildconf(int flags, int level) +{ + const char *indent = flags & INDENT? " " : ""; + + av_log(NULL, level, "\n%sconfiguration:\n" "%s%s"FFMPEG_CONFIGURATION "\n", indent, indent, indent); +} + void show_banner(int argc, char **argv, const OptionDef *options) { int idx = locate_option(argc, argv, options, "version"); @@ -1086,6 +1093,14 @@ int show_version(void *optctx, const char *opt, const char *arg) return 0; } +int show_buildconf(void *optctx, const char *opt, const char *arg) +{ + av_log_set_callback(log_callback_help); + print_buildconf (INDENT|0, AV_LOG_INFO); + + return 0; +} + int show_license(void *optctx, const char *opt, const char *arg) { #if CONFIG_NONFREE diff --git a/cmdutils.h b/cmdutils.h index 3af4476..4d5d2f4 100644 --- a/cmdutils.h +++ b/cmdutils.h @@ -412,6 +412,13 @@ void show_banner(int argc, char **argv, const OptionDef *options); int show_version(void *optctx, const char *opt, const char *arg); /** + * Print the build configuration of the program to stdout. The contents + * depend on the definition of FFMPEG_CONFIGURATION. + * This option processing function does not utilize the arguments. + */ +int show_buildconf(void *optctx, const char *opt, const char *arg); + +/** * Print the license of the program to stdout. The license depends on * the license of the libraries compiled into the program. * This option processing function does not utilize the arguments. diff --git a/cmdutils_common_opts.h b/cmdutils_common_opts.h index b57abaf..48b0af2 100644 --- a/cmdutils_common_opts.h +++ b/cmdutils_common_opts.h @@ -4,6 +4,7 @@ { "help" , OPT_EXIT, {.func_arg = show_help}, "show help", "topic" }, { "-help" , OPT_EXIT, {.func_arg = show_help}, "show help", "topic" }, { "version" , OPT_EXIT, {.func_arg = show_version}, "show version" }, + { "buildconf" , OPT_EXIT, {.func_arg = show_buildconf}, "show build configuration" }, { "formats" , OPT_EXIT, {.func_arg = show_formats }, "show available formats" }, { "codecs" , OPT_EXIT, {.func_arg = show_codecs }, "show available codecs" }, { "decoders" , OPT_EXIT, {.func_arg = show_decoders }, "show available decoders" }, -- 1.8.1.2
1000 is a safe distance away from the actual limit of 1024. If it hit 1024 previously, then two things would happen: 1) The 'configuration:' line gets truncated. 2) It collides with the library version info and throws off the formatting. --- cmdutils.c | 5 ++++- ffmpeg.c | 3 +++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/cmdutils.c b/cmdutils.c index fed8213..4faf5ad 100644 --- a/cmdutils.c +++ b/cmdutils.c @@ -1063,7 +1063,10 @@ static void print_program_info(int flags, int level) av_log(NULL, level, "%sbuilt on %s %s with %s\n", indent, __DATE__, __TIME__, CC_IDENT); - av_log(NULL, level, "%sconfiguration: " FFMPEG_CONFIGURATION "\n", indent); + if (strlen(FFMPEG_CONFIGURATION) > 1000) { + } else { + av_log(NULL, level, "%sconfiguration: " FFMPEG_CONFIGURATION "\n", indent); + } } static void print_buildconf(int flags, int level) diff --git a/ffmpeg.c b/ffmpeg.c index cc709ab..ed00081 100644 --- a/ffmpeg.c +++ b/ffmpeg.c @@ -3371,6 +3371,9 @@ int main(int argc, char **argv) if (nb_output_files <= 0 && nb_input_files == 0) { show_usage(); av_log(NULL, AV_LOG_WARNING, "Use -h to get full help or, even better, run 'man %s'\n", program_name); + if (strlen(FFMPEG_CONFIGURATION) > 1000) { + av_log(NULL, AV_LOG_WARNING, "\nUse -buildconf to get info about how this build of %s was compiled.\n", program_name); + } exit_program(1); } -- 1.8.1.2
--- cmdutils.c | 9 ++++++++- configure | 4 ++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/cmdutils.c b/cmdutils.c index 4faf5ad..ffe7bba 100644 --- a/cmdutils.c +++ b/cmdutils.c @@ -1072,8 +1072,15 @@ static void print_program_info(int flags, int level) static void print_buildconf(int flags, int level) { const char *indent = flags & INDENT? " " : ""; + char str[] = { FFMPEG_CONFIGURATION_FORMATTED }; + char *splitconf; - av_log(NULL, level, "\n%sconfiguration:\n" "%s%s"FFMPEG_CONFIGURATION "\n", indent, indent, indent); + splitconf = strtok(str, "~"); + av_log(NULL, level, "\n%sconfiguration:\n",indent); + while(splitconf != NULL) { + av_log(NULL, level, "%s%s%s\n", indent, indent, splitconf); + splitconf = strtok(NULL, "~"); + } } void show_banner(int argc, char **argv, const OptionDef *options) diff --git a/configure b/configure index 3946632..e18a929 100755 --- a/configure +++ b/configure @@ -4623,11 +4623,14 @@ enabled stripping || strip="echo skipping strip" config_files="$TMPH config.mak doc/config.texi" +FFMPEG_CONFIGURATION_FORMATTED=`echo $FFMPEG_CONFIGURATION | sed -e 's/\ --/~--/g' -e 's/pkg-config~/pkg-config\ /'` + cat > config.mak <<EOF # Automatically generated by configure - do not modify! ifndef FFMPEG_CONFIG_MAK FFMPEG_CONFIG_MAK=1 FFMPEG_CONFIGURATION=$FFMPEG_CONFIGURATION +FFMPEG_CONFIGURATION_FORMATTED=$FFMPEG_CONFIGURATION_FORMATTED prefix=$prefix LIBDIR=\$(DESTDIR)$libdir SHLIBDIR=\$(DESTDIR)$shlibdir @@ -4751,6 +4754,7 @@ cat > $TMPH <<EOF #ifndef FFMPEG_CONFIG_H #define FFMPEG_CONFIG_H #define FFMPEG_CONFIGURATION "$(c_escape $FFMPEG_CONFIGURATION)" +#define FFMPEG_CONFIGURATION_FORMATTED "$(c_escape $FFMPEG_CONFIGURATION_FORMATTED)" #define FFMPEG_LICENSE "$(c_escape $license)" #define FFMPEG_DATADIR "$(eval c_escape $datadir)" #define AVCONV_DATADIR "$(eval c_escape $datadir)" -- 1.8.1.2
On Sat, Aug 10, 2013 at 09:45:38AM -0400, Stephen Hutchinson wrote:
--- cmdutils.c | 9 ++++++++- configure | 4 ++++ 2 files changed, 12 insertions(+), 1 deletion(-)
iam not sure such hybrid sed + C solution is a good idea also see "13:57 To FFmpeg devel (4.7K) └─>[FFmpeg-devel] [PATCH 2/2] avutil/log: Use bprint for part" [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Complexity theory is the science of finding the exact solution to an approximation. Benchmarking OTOH is finding an approximation of the exact
On Sat, Aug 10, 2013 at 10:23 AM, Michael Niedermayer <michaelni@gmx.at>wrote:
iam not sure such hybrid sed + C solution is a good idea
Noted, and fixed. It no longer relies on a separate variable and the string replacement is done with strstr and strncpy now. I also rolled the two patches together so that -buildconf is added with the one-line-per-option formatting.
also see "13:57 To FFmpeg devel (4.7K) └─>[FFmpeg-devel] [PATCH 2/2] avutil/log: Use bprint for part"
This resolves the need for patch #2. Instead, I've altered it so that there's now a configure option that can be used to turn off the 'configuration:' line if the user so desires. I'll send both of the revised patches shortly.
The output is formatted to display one option per line. --- cmdutils.c | 34 ++++++++++++++++++++++++++++++++++ cmdutils.h | 7 +++++++ cmdutils_common_opts.h | 1 + 3 files changed, 42 insertions(+) diff --git a/cmdutils.c b/cmdutils.c index 6eb093d..43ea21c 100644 --- a/cmdutils.c +++ b/cmdutils.c @@ -1066,6 +1066,32 @@ static void print_program_info(int flags, int level) av_log(NULL, level, "%sconfiguration: " FFMPEG_CONFIGURATION "\n", indent); } +static void print_buildconf(int flags, int level) +{ + const char *indent = flags & INDENT? " " : ""; + char str[] = { FFMPEG_CONFIGURATION }; + char *conflist, *remove_tilde, *splitconf; + + // Change all the ' --' strings to '~--' so that + // they can be identified as tokens. + while( (conflist = strstr(str, " --")) != NULL ) { + strncpy(conflist, "~--", 3); + } + + // Compensate for the weirdness this would cause + // when passing 'pkg-config --static'. + while( (remove_tilde = strstr(str, "pkg-config~")) != NULL ) { + strncpy(remove_tilde, "pkg-config ",11); + } + + splitconf = strtok(str, "~"); + av_log(NULL, level, "\n%sconfiguration:\n",indent); + while(splitconf != NULL) { + av_log(NULL, level, "%s%s%s\n", indent, indent, splitconf); + splitconf = strtok(NULL, "~"); + } +} + void show_banner(int argc, char **argv, const OptionDef *options) { int idx = locate_option(argc, argv, options, "version"); @@ -1086,6 +1112,14 @@ int show_version(void *optctx, const char *opt, const char *arg) return 0; } +int show_buildconf(void *optctx, const char *opt, const char *arg) +{ + av_log_set_callback(log_callback_help); + print_buildconf (INDENT|0, AV_LOG_INFO); + + return 0; +} + int show_license(void *optctx, const char *opt, const char *arg) { #if CONFIG_NONFREE diff --git a/cmdutils.h b/cmdutils.h index 3af4476..4d5d2f4 100644 --- a/cmdutils.h +++ b/cmdutils.h @@ -412,6 +412,13 @@ void show_banner(int argc, char **argv, const OptionDef *options); int show_version(void *optctx, const char *opt, const char *arg); /** + * Print the build configuration of the program to stdout. The contents + * depend on the definition of FFMPEG_CONFIGURATION. + * This option processing function does not utilize the arguments. + */ +int show_buildconf(void *optctx, const char *opt, const char *arg); + +/** * Print the license of the program to stdout. The license depends on * the license of the libraries compiled into the program. * This option processing function does not utilize the arguments. diff --git a/cmdutils_common_opts.h b/cmdutils_common_opts.h index b57abaf..48b0af2 100644 --- a/cmdutils_common_opts.h +++ b/cmdutils_common_opts.h @@ -4,6 +4,7 @@ { "help" , OPT_EXIT, {.func_arg = show_help}, "show help", "topic" }, { "-help" , OPT_EXIT, {.func_arg = show_help}, "show help", "topic" }, { "version" , OPT_EXIT, {.func_arg = show_version}, "show version" }, + { "buildconf" , OPT_EXIT, {.func_arg = show_buildconf}, "show build configuration" }, { "formats" , OPT_EXIT, {.func_arg = show_formats }, "show available formats" }, { "codecs" , OPT_EXIT, {.func_arg = show_codecs }, "show available codecs" }, { "decoders" , OPT_EXIT, {.func_arg = show_decoders }, "show available decoders" }, -- 1.8.1.2
This gives the user the ability to disable 'configuration:' from the regular invocation report. --- cmdutils.c | 2 ++ configure | 6 ++++++ ffmpeg.c | 3 +++ ffplay.c | 3 +++ ffprobe.c | 3 +++ 5 files changed, 17 insertions(+) diff --git a/cmdutils.c b/cmdutils.c index 43ea21c..827a3f5 100644 --- a/cmdutils.c +++ b/cmdutils.c @@ -1063,7 +1063,9 @@ static void print_program_info(int flags, int level) av_log(NULL, level, "%sbuilt on %s %s with %s\n", indent, __DATE__, __TIME__, CC_IDENT); +#ifndef CONCISE_CONF av_log(NULL, level, "%sconfiguration: " FFMPEG_CONFIGURATION "\n", indent); +#endif } static void print_buildconf(int flags, int level) diff --git a/configure b/configure index 3946632..a39b3d6 100755 --- a/configure +++ b/configure @@ -294,6 +294,7 @@ Advanced options (experts only): (faster, but may crash) --enable-memalign-hack emulate memalign, interferes with memory debuggers --enable-lto use link-time optimization + --enable-concise-version omit 'configuration:' from regular invocation Optimization options (experts only): --disable-asm disable all assembler optimizations @@ -1592,6 +1593,7 @@ CMDLINE_SELECT=" lto optimizations stripping + concise_version " PATHS_LIST=' @@ -4621,6 +4623,10 @@ test -e Makefile || $ln_s "$source_path/Makefile" . enabled stripping || strip="echo skipping strip" +if enabled concise_version; then + add_cflags -DCONCISE_CONF +fi + config_files="$TMPH config.mak doc/config.texi" cat > config.mak <<EOF diff --git a/ffmpeg.c b/ffmpeg.c index cc709ab..69dab71 100644 --- a/ffmpeg.c +++ b/ffmpeg.c @@ -3371,6 +3371,9 @@ int main(int argc, char **argv) if (nb_output_files <= 0 && nb_input_files == 0) { show_usage(); av_log(NULL, AV_LOG_WARNING, "Use -h to get full help or, even better, run 'man %s'\n", program_name); +#ifdef CONCISE_CONF + av_log(NULL, AV_LOG_WARNING, "\nUse -buildconf to get info about how this build of %s was compiled.\n", program_name); +#endif exit_program(1); } diff --git a/ffplay.c b/ffplay.c index d462eee..8643080 100644 --- a/ffplay.c +++ b/ffplay.c @@ -3524,6 +3524,9 @@ int main(int argc, char **argv) av_log(NULL, AV_LOG_FATAL, "An input file must be specified\n"); av_log(NULL, AV_LOG_FATAL, "Use -h to get full help or, even better, run 'man %s'\n", program_name); +#ifdef CONCISE_CONF + av_log(NULL, AV_LOG_FATAL, "\nUse -buildconf to get info about how this build of %s was compiled.\n", program_name); +#endif exit(1); } diff --git a/ffprobe.c b/ffprobe.c index 8a65e1a..e7bc1a6 100644 --- a/ffprobe.c +++ b/ffprobe.c @@ -2414,6 +2414,9 @@ int main(int argc, char **argv) show_usage(); av_log(NULL, AV_LOG_ERROR, "You have to specify one input file.\n"); av_log(NULL, AV_LOG_ERROR, "Use -h to get full help or, even better, run 'man %s'.\n", program_name); +#ifdef CONCISE_CONF + av_log(NULL, AV_LOG_ERROR, "\nUse -buildconf to get info about how this build of %s was compiled.\n", program_name); +#endif ret = AVERROR(EINVAL); } else if (input_filename) { ret = probe_file(wctx, input_filename); -- 1.8.1.2
On Sun, Aug 11, 2013 at 12:17 PM, Stephen Hutchinson <qyot27@gmail.com> wrote:
This gives the user the ability to disable 'configuration:' from the regular invocation report. --- cmdutils.c | 2 ++ configure | 6 ++++++ ffmpeg.c | 3 +++ ffplay.c | 3 +++ ffprobe.c | 3 +++ 5 files changed, 17 insertions(+)
diff --git a/cmdutils.c b/cmdutils.c index 43ea21c..827a3f5 100644 --- a/cmdutils.c +++ b/cmdutils.c @@ -1063,7 +1063,9 @@ static void print_program_info(int flags, int level) av_log(NULL, level, "%sbuilt on %s %s with %s\n", indent, __DATE__, __TIME__, CC_IDENT);
+#ifndef CONCISE_CONF av_log(NULL, level, "%sconfiguration: " FFMPEG_CONFIGURATION "\n", indent); +#endif }
static void print_buildconf(int flags, int level) diff --git a/configure b/configure index 3946632..a39b3d6 100755 --- a/configure +++ b/configure @@ -294,6 +294,7 @@ Advanced options (experts only): (faster, but may crash) --enable-memalign-hack emulate memalign, interferes with memory debuggers --enable-lto use link-time optimization + --enable-concise-version omit 'configuration:' from regular invocation
Optimization options (experts only): --disable-asm disable all assembler optimizations @@ -1592,6 +1593,7 @@ CMDLINE_SELECT=" lto optimizations stripping + concise_version "
PATHS_LIST=' @@ -4621,6 +4623,10 @@ test -e Makefile || $ln_s "$source_path/Makefile" .
enabled stripping || strip="echo skipping strip"
+if enabled concise_version; then + add_cflags -DCONCISE_CONF +fi + config_files="$TMPH config.mak doc/config.texi"
cat > config.mak <<EOF diff --git a/ffmpeg.c b/ffmpeg.c index cc709ab..69dab71 100644 --- a/ffmpeg.c +++ b/ffmpeg.c @@ -3371,6 +3371,9 @@ int main(int argc, char **argv) if (nb_output_files <= 0 && nb_input_files == 0) { show_usage(); av_log(NULL, AV_LOG_WARNING, "Use -h to get full help or, even better, run 'man %s'\n", program_name); +#ifdef CONCISE_CONF + av_log(NULL, AV_LOG_WARNING, "\nUse -buildconf to get info about how this build of %s was compiled.\n", program_name); +#endif exit_program(1); }
diff --git a/ffplay.c b/ffplay.c index d462eee..8643080 100644 --- a/ffplay.c +++ b/ffplay.c @@ -3524,6 +3524,9 @@ int main(int argc, char **argv) av_log(NULL, AV_LOG_FATAL, "An input file must be specified\n"); av_log(NULL, AV_LOG_FATAL, "Use -h to get full help or, even better, run 'man %s'\n", program_name); +#ifdef CONCISE_CONF + av_log(NULL, AV_LOG_FATAL, "\nUse -buildconf to get info about how this build of %s was compiled.\n", program_name); +#endif exit(1); }
diff --git a/ffprobe.c b/ffprobe.c index 8a65e1a..e7bc1a6 100644 --- a/ffprobe.c +++ b/ffprobe.c @@ -2414,6 +2414,9 @@ int main(int argc, char **argv) show_usage(); av_log(NULL, AV_LOG_ERROR, "You have to specify one input file.\n"); av_log(NULL, AV_LOG_ERROR, "Use -h to get full help or, even better, run 'man %s'.\n", program_name); +#ifdef CONCISE_CONF + av_log(NULL, AV_LOG_ERROR, "\nUse -buildconf to get info about how this build of %s was compiled.\n", program_name); +#endif ret = AVERROR(EINVAL); } else if (input_filename) { ret = probe_file(wctx, input_filename); -- 1.8.1.2
Ping. This goes along with the -buildconf patch so that if a user only wants to see the configuration through -buildconf, they can (it's not enabled by default, so you'd see configuration: with regular invocation, plus you could also use -buildconf). The patch as posted in August does still apply cleanly to master.
On Tue, Nov 19, 2013 at 07:38:27AM -0500, Stephen Hutchinson wrote:
On Sun, Aug 11, 2013 at 12:17 PM, Stephen Hutchinson <qyot27@gmail.com> wrote:
This gives the user the ability to disable 'configuration:' from the regular invocation report. --- cmdutils.c | 2 ++ configure | 6 ++++++ ffmpeg.c | 3 +++ ffplay.c | 3 +++ ffprobe.c | 3 +++ 5 files changed, 17 insertions(+)
diff --git a/cmdutils.c b/cmdutils.c index 43ea21c..827a3f5 100644 --- a/cmdutils.c +++ b/cmdutils.c @@ -1063,7 +1063,9 @@ static void print_program_info(int flags, int level) av_log(NULL, level, "%sbuilt on %s %s with %s\n", indent, __DATE__, __TIME__, CC_IDENT);
+#ifndef CONCISE_CONF av_log(NULL, level, "%sconfiguration: " FFMPEG_CONFIGURATION "\n", indent); +#endif }
static void print_buildconf(int flags, int level) diff --git a/configure b/configure index 3946632..a39b3d6 100755 --- a/configure +++ b/configure @@ -294,6 +294,7 @@ Advanced options (experts only): (faster, but may crash) --enable-memalign-hack emulate memalign, interferes with memory debuggers --enable-lto use link-time optimization + --enable-concise-version omit 'configuration:' from regular invocation
iam not completely sure this is a good idea its quite important that bug reports contain this information
Optimization options (experts only): --disable-asm disable all assembler optimizations @@ -1592,6 +1593,7 @@ CMDLINE_SELECT=" lto optimizations stripping + concise_version "
PATHS_LIST=' @@ -4621,6 +4623,10 @@ test -e Makefile || $ln_s "$source_path/Makefile" .
enabled stripping || strip="echo skipping strip"
+if enabled concise_version; then + add_cflags -DCONCISE_CONF +fi
please see config.h and the code that creates it [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Freedom in capitalist society always remains about the same as it was in ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin
Michael Niedermayer <michaelni <at> gmx.at> writes:
+ --enable-concise-version omit 'configuration:' from regular invocation
iam not completely sure this is a good idea
+1
its quite important that bug reports contain this information
It is. Carl Eugen
On Tue, Nov 19, 2013 at 5:23 PM, Carl Eugen Hoyos <cehoyos@ag.or.at> wrote:
Michael Niedermayer <michaelni <at> gmx.at> writes:
+ --enable-concise-version omit 'configuration:' from regular invocation
iam not completely sure this is a good idea
+1
its quite important that bug reports contain this information
It is.
Carl Eugen
The information being there on regular invocation to make bug reports easier is why this is set up purely as an opt-in. The user has to actively not want to see it and tell configure they don't want it. Otherwise, you get both configuration reports and using -buildconf will show the configuration twice - once as part of the regular invocation, and then the one-per-line formatted output of -buildconf itself. In the event the user does choose to disable configuration:, the regular invocation has an additional notice that the configuration can be viewed with -buildconf (the output of which can be copied into the bug report, like I did with ticket #3017; maybe there could be a mandatory field added to the reporting form that makes sure the configuration is always posted? Is that possible with how trac is set up?).
Michael Niedermayer <michaelni <at> gmx.at> writes:
please see config.h and the code that creates it
So it should be enabled concise_version && add_cflags -DCONCISE_CONF instead? I'm not sure what I should be looking at in config.h. Having it as a discrete #define that shows up in the list of defines in config.h? Using a different name (prefixed with HAVE_ or CONFIG_)?
On Tue, Nov 19, 2013 at 07:45:40PM -0500, Stephen Hutchinson wrote:
On Tue, Nov 19, 2013 at 5:23 PM, Carl Eugen Hoyos <cehoyos@ag.or.at> wrote:
Michael Niedermayer <michaelni <at> gmx.at> writes:
+ --enable-concise-version omit 'configuration:' from regular invocation
iam not completely sure this is a good idea
+1
its quite important that bug reports contain this information
It is.
Carl Eugen
The information being there on regular invocation to make bug reports easier is why this is set up purely as an opt-in. The user has to actively not want to see it and tell configure they don't want it. Otherwise, you get both configuration reports and using -buildconf will show the configuration twice - once as part of the regular invocation, and then the one-per-line formatted output of -buildconf itself.
In the event the user does choose to disable configuration:, the regular invocation has an additional notice that the configuration can be viewed with -buildconf (the output of which can be copied into the bug report, like I did with ticket #3017; maybe there could be a mandatory field added to the reporting form that makes sure the configuration is always posted? Is that possible with how trac is set up?).
Michael Niedermayer <michaelni <at> gmx.at> writes:
please see config.h and the code that creates it
So it should be enabled concise_version && add_cflags -DCONCISE_CONF instead?
I'm not sure what I should be looking at in config.h.
Having it as a discrete #define that shows up in the list of defines in config.h?
yes, but it seems there is no consensus on the feature as such so maybe wait with modifying the patch until/if everyone agrees on how/if this should be done
Using a different name (prefixed with HAVE_ or CONFIG_)? _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
-- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Opposition brings concord. Out of discord comes the fairest harmony. -- Heraclitus
On Sun, Aug 11, 2013 at 12:17 PM, Stephen Hutchinson <qyot27@gmail.com> wrote:
The output is formatted to display one option per line. --- cmdutils.c | 34 ++++++++++++++++++++++++++++++++++ cmdutils.h | 7 +++++++ cmdutils_common_opts.h | 1 + 3 files changed, 42 insertions(+)
diff --git a/cmdutils.c b/cmdutils.c index 6eb093d..43ea21c 100644 --- a/cmdutils.c +++ b/cmdutils.c @@ -1066,6 +1066,32 @@ static void print_program_info(int flags, int level) av_log(NULL, level, "%sconfiguration: " FFMPEG_CONFIGURATION "\n", indent); }
+static void print_buildconf(int flags, int level) +{ + const char *indent = flags & INDENT? " " : ""; + char str[] = { FFMPEG_CONFIGURATION }; + char *conflist, *remove_tilde, *splitconf; + + // Change all the ' --' strings to '~--' so that + // they can be identified as tokens. + while( (conflist = strstr(str, " --")) != NULL ) { + strncpy(conflist, "~--", 3); + } + + // Compensate for the weirdness this would cause + // when passing 'pkg-config --static'. + while( (remove_tilde = strstr(str, "pkg-config~")) != NULL ) { + strncpy(remove_tilde, "pkg-config ",11); + } + + splitconf = strtok(str, "~"); + av_log(NULL, level, "\n%sconfiguration:\n",indent); + while(splitconf != NULL) { + av_log(NULL, level, "%s%s%s\n", indent, indent, splitconf); + splitconf = strtok(NULL, "~"); + } +} + void show_banner(int argc, char **argv, const OptionDef *options) { int idx = locate_option(argc, argv, options, "version"); @@ -1086,6 +1112,14 @@ int show_version(void *optctx, const char *opt, const char *arg) return 0; }
+int show_buildconf(void *optctx, const char *opt, const char *arg) +{ + av_log_set_callback(log_callback_help); + print_buildconf (INDENT|0, AV_LOG_INFO); + + return 0; +} + int show_license(void *optctx, const char *opt, const char *arg) { #if CONFIG_NONFREE diff --git a/cmdutils.h b/cmdutils.h index 3af4476..4d5d2f4 100644 --- a/cmdutils.h +++ b/cmdutils.h @@ -412,6 +412,13 @@ void show_banner(int argc, char **argv, const OptionDef *options); int show_version(void *optctx, const char *opt, const char *arg);
/** + * Print the build configuration of the program to stdout. The contents + * depend on the definition of FFMPEG_CONFIGURATION. + * This option processing function does not utilize the arguments. + */ +int show_buildconf(void *optctx, const char *opt, const char *arg); + +/** * Print the license of the program to stdout. The license depends on * the license of the libraries compiled into the program. * This option processing function does not utilize the arguments. diff --git a/cmdutils_common_opts.h b/cmdutils_common_opts.h index b57abaf..48b0af2 100644 --- a/cmdutils_common_opts.h +++ b/cmdutils_common_opts.h @@ -4,6 +4,7 @@ { "help" , OPT_EXIT, {.func_arg = show_help}, "show help", "topic" }, { "-help" , OPT_EXIT, {.func_arg = show_help}, "show help", "topic" }, { "version" , OPT_EXIT, {.func_arg = show_version}, "show version" }, + { "buildconf" , OPT_EXIT, {.func_arg = show_buildconf}, "show build configuration" }, { "formats" , OPT_EXIT, {.func_arg = show_formats }, "show available formats" }, { "codecs" , OPT_EXIT, {.func_arg = show_codecs }, "show available codecs" }, { "decoders" , OPT_EXIT, {.func_arg = show_decoders }, "show available decoders" }, -- 1.8.1.2
Ping. This seemed to have gotten passed over when the character limit issue was fixed, but I still think this is worth having. The patch as posted in August does still apply cleanly to master.
On Sun, Aug 11, 2013 at 12:17:57PM -0400, Stephen Hutchinson wrote:
The output is formatted to display one option per line. --- cmdutils.c | 34 ++++++++++++++++++++++++++++++++++ cmdutils.h | 7 +++++++ cmdutils_common_opts.h | 1 + 3 files changed, 42 insertions(+)
applied thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I know you won't believe me, but the highest form of Human Excellence is to question oneself and others. -- Socrates
Le tridi 23 thermidor, an CCXXI, Stephen Hutchinson a écrit :
Unfortunately, my ideas about how to get it to print one option per line didn't really pan out since I was just messing around with sed*, and
Why don't you just do it in C? For example, use strcspn/strchr to find the length of the next word, and then use this to print it: av_log(..., "%.*s ", length, str);
there's no way of predicting that sed's behavior will remain the same across the different versions of it used on various platforms
sed is standardized; if you carefully adhere to the standard, you can do pretty advanced stuff. Not that I think this is a good idea, it would be unreadable. Also, I do not believe that printing one option per line is a good idea; my usual build have 36 build options, mostly --enable-foo: that is exactly the size of most of my work ttys. I can suggest this: replace "--enable-foo" by "+foo", and wrap to 76 columns. Regards, -- Nicolas George
On Sat, Aug 10, 2013 at 4:22 AM, Nicolas George < nicolas.george@normalesup.org> wrote:
Why don't you just do it in C? For example, use strcspn/strchr to find the length of the next word, and then use this to print it:
av_log(..., "%.*s ", length, str);
To be honest, I wanted to do it all in C, but for all the looking I was doing at the man pages for the str* and printf* functions, I couldn't find the right documentation or explanations to show how to make the pieces work together like that. This helps, so I'll start digging more along these lines. Also, I do not believe that printing one option per line is a good idea; my
usual build have 36 build options, mostly --enable-foo: that is exactly the size of most of my work ttys.
The one-per-line situation would be applied exclusively to the output of the -buildconf option, to be consistent with other -h/-formats/-codecs/etc. options. The 'configuration:' line displayed during regular invocation wouldn't change. I have to deal with 48 build options when doing cross-compiles; there's no way I'd want to make regular invocation display it as one-per-line - I already have to scroll up (or increase the window height of the Terminal/cmd.exe) to see the version number/copyright/build date lines when running it with all those in there.
I can suggest this: replace "--enable-foo" by "+foo", and wrap to 76 columns.
I like the +foo format for regular invocation display, but it'll need additional rules to compensate for options that aren't prefixed by --enable- (--extra-cflags/ldflags, --cross-prefix, --arch, --cpu, etc.), unless those just get ignored for display purposes.
participants (5)
-
Alexander Strasser -
Carl Eugen Hoyos -
Michael Niedermayer -
Nicolas George -
Stephen Hutchinson