| Message ID | 5bf9410a-a108-0294-8c33-c7fc78e07c5f@mail.de |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] fftools/cmdutils.c: Add cmd line option to override detection of cpu count | expand |
| Context | Check | Description |
|---|---|---|
| andriy/configure | warning | Failed to apply patch |
Quoting Thilo Borgmann (2021-06-05 14:29:05) > Hi, > > add an option to override auto-detection of cpu count. Defaults to auto-detection, of course. > > -Thilo > > From 38612f3e1339354dbaa6be4f36072320ff71c707 Mon Sep 17 00:00:00 2001 > From: Thilo Borgmann <thilo.borgmann@mail.de> > Date: Sat, 5 Jun 2021 14:26:23 +0200 > Subject: [PATCH] fftools/cmdutils.c: Add cmd line option to override detection > of cpu count > > Suggested-By: ffmpeg@fb.com > --- > doc/fftools-common-opts.texi | 7 +++++++ > fftools/cmdutils.c | 27 +++++++++++++++++++++++++++ > fftools/cmdutils.h | 7 +++++++ > libavutil/cpu.c | 14 ++++++++++++++ > libavutil/cpu.h | 6 ++++++ > 5 files changed, 61 insertions(+) > > diff --git a/doc/fftools-common-opts.texi b/doc/fftools-common-opts.texi > index 5260ecb8f3..7643dd8396 100644 > --- a/doc/fftools-common-opts.texi > +++ b/doc/fftools-common-opts.texi > @@ -353,6 +353,13 @@ Possible flags for this option are: > @end table > @end table > > +@item -cpucount @var{count} (@emph{global}) > +Override detection of CPU count. This option is intended > +for testing. Do not use it unless you know what you're doing. > +@example > +ffmpeg -cpucount 2 > +@end example > + > @item -max_alloc @var{bytes} > Set the maximum size limit for allocating a block on the heap by ffmpeg's > family of malloc functions. Exercise @strong{extreme caution} when using > diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c > index 4eb68d2201..583a706e5d 100644 > --- a/fftools/cmdutils.c > +++ b/fftools/cmdutils.c > @@ -72,6 +72,7 @@ AVDictionary *format_opts, *codec_opts, *resample_opts; > static FILE *report_file; > static int report_file_level = AV_LOG_DEBUG; > int hide_banner = 0; > +int cpu_count = -1; > > enum show_muxdemuxers { > SHOW_DEFAULT, > @@ -866,6 +867,32 @@ int opt_cpuflags(void *optctx, const char *opt, const char *arg) > return 0; > } > > +int opt_cpucount(void *optctx, const char *opt, const char *arg) > +{ > + int ret; > + int count; > + > + static const AVOption opts[] = { > + {"count", NULL, 0, AV_OPT_TYPE_INT, { .i64 = -1}, -1, INT_MAX, NULL}, > + {NULL}, > + }; > + static const AVClass class = { > + .class_name = "cpucount", > + .item_name = av_default_item_name, > + .option = opts, > + .version = LIBAVUTIL_VERSION_INT, > + }; > + const AVClass *pclass = &class; > + > + ret = av_opt_eval_int(&pclass, opts, arg, &count); > + > + if (!ret) { > + av_force_cpu_count(count); > + } > + > + return ret; This looks way overcomplicated. Why not just call strtol on arg? > +} > + > int opt_loglevel(void *optctx, const char *opt, const char *arg) > { > const struct { const char *name; int level; } log_levels[] = { > diff --git a/fftools/cmdutils.h b/fftools/cmdutils.h > index 1917510589..29a45dd0be 100644 > --- a/fftools/cmdutils.h > +++ b/fftools/cmdutils.h > @@ -50,6 +50,7 @@ extern AVDictionary *sws_dict; > extern AVDictionary *swr_opts; > extern AVDictionary *format_opts, *codec_opts, *resample_opts; > extern int hide_banner; > +extern int cpu_count; > > /** > * Register a program-specific cleanup routine. > @@ -88,6 +89,11 @@ void log_callback_help(void* ptr, int level, const char* fmt, va_list vl); > */ > int opt_cpuflags(void *optctx, const char *opt, const char *arg); > > +/** > + * Override the cpucount. > + */ > +int opt_cpucount(void *optctx, const char *opt, const char *arg); > + > /** > * Fallback for options that are not explicitly handled, these will be > * parsed through AVOptions. > @@ -239,6 +245,7 @@ void show_help_options(const OptionDef *options, const char *msg, int req_flags, > { "report", 0, { .func_arg = opt_report }, "generate a report" }, \ > { "max_alloc", HAS_ARG, { .func_arg = opt_max_alloc }, "set maximum size of a single allocated block", "bytes" }, \ > { "cpuflags", HAS_ARG | OPT_EXPERT, { .func_arg = opt_cpuflags }, "force specific cpu flags", "flags" }, \ > + { "cpucount", HAS_ARG | OPT_EXPERT, { .func_arg = opt_cpucount }, "force specific cpu count", "count" }, \ > { "hide_banner", OPT_BOOL | OPT_EXPERT, {&hide_banner}, "do not show program banner", "hide_banner" }, \ > CMDUTILS_COMMON_OPTIONS_AVDEVICE \ > > diff --git a/libavutil/cpu.c b/libavutil/cpu.c > index 52f6b9a3bf..ccd5b5adac 100644 > --- a/libavutil/cpu.c > +++ b/libavutil/cpu.c > @@ -48,6 +48,7 @@ > #endif > > static atomic_int cpu_flags = ATOMIC_VAR_INIT(-1); > +static atomic_int cpu_count = ATOMIC_VAR_INIT(-1); > > static int get_cpu_flags(void) > { > @@ -306,14 +307,27 @@ int av_cpu_count(void) > nb_cpus = sysinfo.dwNumberOfProcessors; > #endif > > + int count = atomic_load_explicit(&cpu_count, memory_order_relaxed); Just use a normal atomic_load(). > + > if (!printed) { > av_log(NULL, AV_LOG_DEBUG, "detected %d logical cores\n", nb_cpus); > + if (count > 0) { > + av_log(NULL, AV_LOG_DEBUG, "overriding to %d logical cores\n", count); > + } > printed = 1; > } > > + if (count > 0) { > + nb_cpus = count; > + } > + > return nb_cpus; > } > > +void av_force_cpu_count(int count){ Style. > + atomic_store_explicit(&cpu_count, count, memory_order_relaxed); > +} > + > size_t av_cpu_max_align(void) > { > if (ARCH_MIPS) > diff --git a/libavutil/cpu.h b/libavutil/cpu.h > index 83099dd969..9c682ad50b 100644 > --- a/libavutil/cpu.h > +++ b/libavutil/cpu.h > @@ -119,6 +119,12 @@ int av_parse_cpu_caps(unsigned *flags, const char *s); > */ > int av_cpu_count(void); > > +/** > + * Overrides cpu count detection and forces the specified count. > + * Count < 1 disables forcing of specific count. > + */ > +void av_force_cpu_count(int count); I'd prefer if the name was namespaced as av_cpu_
Hi, Am 05.06.21 um 16:33 schrieb Anton Khirnov: > Quoting Thilo Borgmann (2021-06-05 14:29:05) >> Hi, >> >> add an option to override auto-detection of cpu count. Defaults to auto-detection, of course. >> >> -Thilo >> >> From 38612f3e1339354dbaa6be4f36072320ff71c707 Mon Sep 17 00:00:00 2001 >> From: Thilo Borgmann <thilo.borgmann@mail.de> >> Date: Sat, 5 Jun 2021 14:26:23 +0200 >> Subject: [PATCH] fftools/cmdutils.c: Add cmd line option to override detection >> of cpu count >> >> Suggested-By: ffmpeg@fb.com >> --- >> doc/fftools-common-opts.texi | 7 +++++++ >> fftools/cmdutils.c | 27 +++++++++++++++++++++++++++ >> fftools/cmdutils.h | 7 +++++++ >> libavutil/cpu.c | 14 ++++++++++++++ >> libavutil/cpu.h | 6 ++++++ >> 5 files changed, 61 insertions(+) >> >> [...] >> /** >> * Fallback for options that are not explicitly handled, these will be >> * parsed through AVOptions. >> @@ -239,6 +245,7 @@ void show_help_options(const OptionDef *options, const char *msg, int req_flags, >> { "report", 0, { .func_arg = opt_report }, "generate a report" }, \ >> { "max_alloc", HAS_ARG, { .func_arg = opt_max_alloc }, "set maximum size of a single allocated block", "bytes" }, \ >> { "cpuflags", HAS_ARG | OPT_EXPERT, { .func_arg = opt_cpuflags }, "force specific cpu flags", "flags" }, \ >> + { "cpucount", HAS_ARG | OPT_EXPERT, { .func_arg = opt_cpucount }, "force specific cpu count", "count" }, \ >> { "hide_banner", OPT_BOOL | OPT_EXPERT, {&hide_banner}, "do not show program banner", "hide_banner" }, \ >> CMDUTILS_COMMON_OPTIONS_AVDEVICE \ >> >> diff --git a/libavutil/cpu.c b/libavutil/cpu.c >> index 52f6b9a3bf..ccd5b5adac 100644 >> --- a/libavutil/cpu.c >> +++ b/libavutil/cpu.c >> @@ -48,6 +48,7 @@ >> #endif >> >> static atomic_int cpu_flags = ATOMIC_VAR_INIT(-1); >> +static atomic_int cpu_count = ATOMIC_VAR_INIT(-1); >> >> static int get_cpu_flags(void) >> { >> @@ -306,14 +307,27 @@ int av_cpu_count(void) >> nb_cpus = sysinfo.dwNumberOfProcessors; >> #endif >> >> + int count = atomic_load_explicit(&cpu_count, memory_order_relaxed); > > Just use a normal atomic_load(). > >> [...] >> +/** >> + * Overrides cpu count detection and forces the specified count. >> + * Count < 1 disables forcing of specific count. >> + */ >> +void av_force_cpu_count(int count); > > I'd prefer if the name was namespaced as av_cpu_ For both of these, I copied the behavior of av_force_cpu_flags(). For function names as well as the atomic load. Let me know which way you prefer. If you prefer your reviewed version, I could also send a patch for similar changes to av_force_cpu_flags() if that makes sense. All other remarks changed locally. Thanks, Thilo
Thilo Borgmann: > Hi, > > add an option to override auto-detection of cpu count. Defaults to auto-detection, of course. > > -Thilo > > > doc/fftools-common-opts.texi | 7 +++++++ > fftools/cmdutils.c | 27 +++++++++++++++++++++++++++ > fftools/cmdutils.h | 7 +++++++ > libavutil/cpu.c | 14 ++++++++++++++ > libavutil/cpu.h | 6 ++++++ The changes to libavutil and cmdutils should be in separate patches; and of course the commit message should mention that you are changing libavutil -- I would have nearly missed this patch (given that I don't pay much attention to fftools in general). > > diff --git a/libavutil/cpu.c b/libavutil/cpu.c > index 52f6b9a3bf..ccd5b5adac 100644 > --- a/libavutil/cpu.c > +++ b/libavutil/cpu.c 52f6b9a3bf is the state of cpu.c after e387fcd01cb84d9493f3b96158addd2a85f086c6. This is completely outdated. > > @@ -306,14 +307,27 @@ int av_cpu_count(void) > nb_cpus = sysinfo.dwNumberOfProcessors; > #endif > > + int count = atomic_load_explicit(&cpu_count, memory_order_relaxed); > + > if (!printed) { > av_log(NULL, AV_LOG_DEBUG, "detected %d logical cores\n", nb_cpus); > + if (count > 0) { > + av_log(NULL, AV_LOG_DEBUG, "overriding to %d logical cores\n", count); > + } > printed = 1; > } > > + if (count > 0) { > + nb_cpus = count; > + } > + > return nb_cpus; > } > In particular, this hunk doesn't apply to git master at all any more (did I already mention that cpu.c only has 245 lines atm, not >300?). And the declaration of count would lead to a statement-after-declaration warning. - Andreas
Am 06.06.21 um 14:35 schrieb Andreas Rheinhardt: > Thilo Borgmann: >> Hi, >> >> add an option to override auto-detection of cpu count. Defaults to auto-detection, of course. >> >> -Thilo >> > >> >> doc/fftools-common-opts.texi | 7 +++++++ >> fftools/cmdutils.c | 27 +++++++++++++++++++++++++++ >> fftools/cmdutils.h | 7 +++++++ >> libavutil/cpu.c | 14 ++++++++++++++ >> libavutil/cpu.h | 6 ++++++ > > The changes to libavutil and cmdutils should be in separate patches; and > of course the commit message should mention that you are changing > libavutil -- I would have nearly missed this patch (given that I don't > pay much attention to fftools in general). > >> >> diff --git a/libavutil/cpu.c b/libavutil/cpu.c >> index 52f6b9a3bf..ccd5b5adac 100644 >> --- a/libavutil/cpu.c >> +++ b/libavutil/cpu.c > > 52f6b9a3bf is the state of cpu.c after > e387fcd01cb84d9493f3b96158addd2a85f086c6. This is completely outdated. Yes, messed up branches for the patch, thx! >> >> @@ -306,14 +307,27 @@ int av_cpu_count(void) >> nb_cpus = sysinfo.dwNumberOfProcessors; >> #endif >> >> + int count = atomic_load_explicit(&cpu_count, memory_order_relaxed); >> + >> if (!printed) { >> av_log(NULL, AV_LOG_DEBUG, "detected %d logical cores\n", nb_cpus); >> + if (count > 0) { >> + av_log(NULL, AV_LOG_DEBUG, "overriding to %d logical cores\n", count); >> + } >> printed = 1; >> } >> >> + if (count > 0) { >> + nb_cpus = count; >> + } >> + >> return nb_cpus; >> } >> > In particular, this hunk doesn't apply to git master at all any more > (did I already mention that cpu.c only has 245 lines atm, not >300?). > And the declaration of count would lead to a statement-after-declaration > warning. Done locally to apply to today's HEAD. I think I'll wait with sending both patches once we decided on atomic loads in "[PATCH] Stop using _explicit atomic operations where not necessary.". Thanks, Thilo
Am 06.06.21 um 14:35 schrieb Andreas Rheinhardt: > Thilo Borgmann: >> Hi, >> >> add an option to override auto-detection of cpu count. Defaults to auto-detection, of course. >> >> -Thilo >> > >> >> doc/fftools-common-opts.texi | 7 +++++++ >> fftools/cmdutils.c | 27 +++++++++++++++++++++++++++ >> fftools/cmdutils.h | 7 +++++++ >> libavutil/cpu.c | 14 ++++++++++++++ >> libavutil/cpu.h | 6 ++++++ > > The changes to libavutil and cmdutils should be in separate patches; and > of course the commit message should mention that you are changing > libavutil -- I would have nearly missed this patch (given that I don't > pay much attention to fftools in general). > >> >> diff --git a/libavutil/cpu.c b/libavutil/cpu.c >> index 52f6b9a3bf..ccd5b5adac 100644 >> --- a/libavutil/cpu.c >> +++ b/libavutil/cpu.c > > 52f6b9a3bf is the state of cpu.c after > e387fcd01cb84d9493f3b96158addd2a85f086c6. This is completely outdated. Yes, messed up branches for the patch, thx! >> >> @@ -306,14 +307,27 @@ int av_cpu_count(void) >> nb_cpus = sysinfo.dwNumberOfProcessors; >> #endif >> >> + int count = atomic_load_explicit(&cpu_count, memory_order_relaxed); >> + >> if (!printed) { >> av_log(NULL, AV_LOG_DEBUG, "detected %d logical cores\n", nb_cpus); >> + if (count > 0) { >> + av_log(NULL, AV_LOG_DEBUG, "overriding to %d logical cores\n", count); >> + } >> printed = 1; >> } >> >> + if (count > 0) { >> + nb_cpus = count; >> + } >> + >> return nb_cpus; >> } >> > In particular, this hunk doesn't apply to git master at all any more > (did I already mention that cpu.c only has 245 lines atm, not >300?). > And the declaration of count would lead to a statement-after-declaration > warning. Done locally to apply to today's HEAD. I think I'll wait with sending both patches once we decided on atomic loads in "[PATCH] Stop using _explicit atomic operations where not necessary.". Thanks, Thilo
Quoting Thilo Borgmann (2021-06-06 12:44:41) > Hi, > > Am 05.06.21 um 16:33 schrieb Anton Khirnov: > > Quoting Thilo Borgmann (2021-06-05 14:29:05) > >> Hi, > >> > >> add an option to override auto-detection of cpu count. Defaults to auto-detection, of course. > >> > >> -Thilo > >> > >> From 38612f3e1339354dbaa6be4f36072320ff71c707 Mon Sep 17 00:00:00 2001 > >> From: Thilo Borgmann <thilo.borgmann@mail.de> > >> Date: Sat, 5 Jun 2021 14:26:23 +0200 > >> Subject: [PATCH] fftools/cmdutils.c: Add cmd line option to override detection > >> of cpu count > >> > >> Suggested-By: ffmpeg@fb.com > >> --- > >> doc/fftools-common-opts.texi | 7 +++++++ > >> fftools/cmdutils.c | 27 +++++++++++++++++++++++++++ > >> fftools/cmdutils.h | 7 +++++++ > >> libavutil/cpu.c | 14 ++++++++++++++ > >> libavutil/cpu.h | 6 ++++++ > >> 5 files changed, 61 insertions(+) > >> > >> [...] > > >> /** > >> * Fallback for options that are not explicitly handled, these will be > >> * parsed through AVOptions. > >> @@ -239,6 +245,7 @@ void show_help_options(const OptionDef *options, const char *msg, int req_flags, > >> { "report", 0, { .func_arg = opt_report }, "generate a report" }, \ > >> { "max_alloc", HAS_ARG, { .func_arg = opt_max_alloc }, "set maximum size of a single allocated block", "bytes" }, \ > >> { "cpuflags", HAS_ARG | OPT_EXPERT, { .func_arg = opt_cpuflags }, "force specific cpu flags", "flags" }, \ > >> + { "cpucount", HAS_ARG | OPT_EXPERT, { .func_arg = opt_cpucount }, "force specific cpu count", "count" }, \ > >> { "hide_banner", OPT_BOOL | OPT_EXPERT, {&hide_banner}, "do not show program banner", "hide_banner" }, \ > >> CMDUTILS_COMMON_OPTIONS_AVDEVICE \ > >> > >> diff --git a/libavutil/cpu.c b/libavutil/cpu.c > >> index 52f6b9a3bf..ccd5b5adac 100644 > >> --- a/libavutil/cpu.c > >> +++ b/libavutil/cpu.c > >> @@ -48,6 +48,7 @@ > >> #endif > >> > >> static atomic_int cpu_flags = ATOMIC_VAR_INIT(-1); > >> +static atomic_int cpu_count = ATOMIC_VAR_INIT(-1); > >> > >> static int get_cpu_flags(void) > >> { > >> @@ -306,14 +307,27 @@ int av_cpu_count(void) > >> nb_cpus = sysinfo.dwNumberOfProcessors; > >> #endif > >> > >> + int count = atomic_load_explicit(&cpu_count, memory_order_relaxed); > > > > Just use a normal atomic_load(). > > > >> [...] > > >> +/** > >> + * Overrides cpu count detection and forces the specified count. > >> + * Count < 1 disables forcing of specific count. > >> + */ > >> +void av_force_cpu_count(int count); > > > > I'd prefer if the name was namespaced as av_cpu_ > > For both of these, I copied the behavior of av_force_cpu_flags(). For function names as well as the atomic load. > Let me know which way you prefer. If you prefer your reviewed version, I could also send a patch for similar changes to av_force_cpu_flags() if that makes sense. Sadly we cannot change existing function names. But at least new functions can use consistent namespacing.
Hi, >>> add an option to override auto-detection of cpu count. Defaults to auto-detection, of course. >>> >>> -Thilo >>> >> >>> >>> doc/fftools-common-opts.texi | 7 +++++++ >>> fftools/cmdutils.c | 27 +++++++++++++++++++++++++++ >>> fftools/cmdutils.h | 7 +++++++ >>> libavutil/cpu.c | 14 ++++++++++++++ >>> libavutil/cpu.h | 6 ++++++ >> >> The changes to libavutil and cmdutils should be in separate patches; and >> of course the commit message should mention that you are changing >> libavutil -- I would have nearly missed this patch (given that I don't >> pay much attention to fftools in general). >> >>> >>> diff --git a/libavutil/cpu.c b/libavutil/cpu.c >>> index 52f6b9a3bf..ccd5b5adac 100644 >>> --- a/libavutil/cpu.c >>> +++ b/libavutil/cpu.c >> >> 52f6b9a3bf is the state of cpu.c after >> e387fcd01cb84d9493f3b96158addd2a85f086c6. This is completely outdated. > > Yes, messed up branches for the patch, thx! > > >>> >>> @@ -306,14 +307,27 @@ int av_cpu_count(void) >>> nb_cpus = sysinfo.dwNumberOfProcessors; >>> #endif >>> >>> + int count = atomic_load_explicit(&cpu_count, memory_order_relaxed); >>> + >>> if (!printed) { >>> av_log(NULL, AV_LOG_DEBUG, "detected %d logical cores\n", nb_cpus); >>> + if (count > 0) { >>> + av_log(NULL, AV_LOG_DEBUG, "overriding to %d logical cores\n", count); >>> + } >>> printed = 1; >>> } >>> >>> + if (count > 0) { >>> + nb_cpus = count; >>> + } >>> + >>> return nb_cpus; >>> } >>> >> In particular, this hunk doesn't apply to git master at all any more >> (did I already mention that cpu.c only has 245 lines atm, not >300?). >> And the declaration of count would lead to a statement-after-declaration >> warning. > > Done locally to apply to today's HEAD. I think I'll wait with sending both patches once we decided on atomic loads in "[PATCH] Stop using _explicit atomic operations where not necessary.". Discussion on atomic loads appear to have stalled mid-June... If there are no hard feeling, I'll push this version soon as the explicit (or not) loads can then be changed later. -Thilo
Am 04.07.21 um 14:31 schrieb Thilo Borgmann: > Hi, > >>>> add an option to override auto-detection of cpu count. Defaults to auto-detection, of course. >>>> >>>> -Thilo >>>> >>> >>>> >>>> doc/fftools-common-opts.texi | 7 +++++++ >>>> fftools/cmdutils.c | 27 +++++++++++++++++++++++++++ >>>> fftools/cmdutils.h | 7 +++++++ >>>> libavutil/cpu.c | 14 ++++++++++++++ >>>> libavutil/cpu.h | 6 ++++++ >>> >>> The changes to libavutil and cmdutils should be in separate patches; and >>> of course the commit message should mention that you are changing >>> libavutil -- I would have nearly missed this patch (given that I don't >>> pay much attention to fftools in general). >>> >>>> >>>> diff --git a/libavutil/cpu.c b/libavutil/cpu.c >>>> index 52f6b9a3bf..ccd5b5adac 100644 >>>> --- a/libavutil/cpu.c >>>> +++ b/libavutil/cpu.c >>> >>> 52f6b9a3bf is the state of cpu.c after >>> e387fcd01cb84d9493f3b96158addd2a85f086c6. This is completely outdated. >> >> Yes, messed up branches for the patch, thx! >> >> >>>> >>>> @@ -306,14 +307,27 @@ int av_cpu_count(void) >>>> nb_cpus = sysinfo.dwNumberOfProcessors; >>>> #endif >>>> >>>> + int count = atomic_load_explicit(&cpu_count, memory_order_relaxed); >>>> + >>>> if (!printed) { >>>> av_log(NULL, AV_LOG_DEBUG, "detected %d logical cores\n", nb_cpus); >>>> + if (count > 0) { >>>> + av_log(NULL, AV_LOG_DEBUG, "overriding to %d logical cores\n", count); >>>> + } >>>> printed = 1; >>>> } >>>> >>>> + if (count > 0) { >>>> + nb_cpus = count; >>>> + } >>>> + >>>> return nb_cpus; >>>> } >>>> >>> In particular, this hunk doesn't apply to git master at all any more >>> (did I already mention that cpu.c only has 245 lines atm, not >300?). >>> And the declaration of count would lead to a statement-after-declaration >>> warning. >> >> Done locally to apply to today's HEAD. I think I'll wait with sending both patches once we decided on atomic loads in "[PATCH] Stop using _explicit atomic operations where not necessary.". > > Discussion on atomic loads appear to have stalled mid-June... If there are no hard feeling, I'll push this version soon as the explicit (or not) loads can then be changed later. Pushed, thanks! -Thilo
On 6/6/2021 7:44 AM, Thilo Borgmann wrote: > Hi, > > Am 05.06.21 um 16:33 schrieb Anton Khirnov: >> Quoting Thilo Borgmann (2021-06-05 14:29:05) >>> Hi, >>> >>> add an option to override auto-detection of cpu count. Defaults to auto-detection, of course. >>> >>> -Thilo >>> >>> From 38612f3e1339354dbaa6be4f36072320ff71c707 Mon Sep 17 00:00:00 2001 >>> From: Thilo Borgmann <thilo.borgmann@mail.de> >>> Date: Sat, 5 Jun 2021 14:26:23 +0200 >>> Subject: [PATCH] fftools/cmdutils.c: Add cmd line option to override detection >>> of cpu count >>> >>> Suggested-By: ffmpeg@fb.com >>> --- >>> doc/fftools-common-opts.texi | 7 +++++++ >>> fftools/cmdutils.c | 27 +++++++++++++++++++++++++++ >>> fftools/cmdutils.h | 7 +++++++ >>> libavutil/cpu.c | 14 ++++++++++++++ >>> libavutil/cpu.h | 6 ++++++ >>> 5 files changed, 61 insertions(+) >>> >>> [...] > >>> /** >>> * Fallback for options that are not explicitly handled, these will be >>> * parsed through AVOptions. >>> @@ -239,6 +245,7 @@ void show_help_options(const OptionDef *options, const char *msg, int req_flags, >>> { "report", 0, { .func_arg = opt_report }, "generate a report" }, \ >>> { "max_alloc", HAS_ARG, { .func_arg = opt_max_alloc }, "set maximum size of a single allocated block", "bytes" }, \ >>> { "cpuflags", HAS_ARG | OPT_EXPERT, { .func_arg = opt_cpuflags }, "force specific cpu flags", "flags" }, \ >>> + { "cpucount", HAS_ARG | OPT_EXPERT, { .func_arg = opt_cpucount }, "force specific cpu count", "count" }, \ >>> { "hide_banner", OPT_BOOL | OPT_EXPERT, {&hide_banner}, "do not show program banner", "hide_banner" }, \ >>> CMDUTILS_COMMON_OPTIONS_AVDEVICE \ >>> >>> diff --git a/libavutil/cpu.c b/libavutil/cpu.c >>> index 52f6b9a3bf..ccd5b5adac 100644 >>> --- a/libavutil/cpu.c >>> +++ b/libavutil/cpu.c >>> @@ -48,6 +48,7 @@ >>> #endif >>> >>> static atomic_int cpu_flags = ATOMIC_VAR_INIT(-1); >>> +static atomic_int cpu_count = ATOMIC_VAR_INIT(-1); >>> >>> static int get_cpu_flags(void) >>> { >>> @@ -306,14 +307,27 @@ int av_cpu_count(void) >>> nb_cpus = sysinfo.dwNumberOfProcessors; >>> #endif >>> >>> + int count = atomic_load_explicit(&cpu_count, memory_order_relaxed); >> >> Just use a normal atomic_load(). >> >>> [...] > >>> +/** >>> + * Overrides cpu count detection and forces the specified count. >>> + * Count < 1 disables forcing of specific count. >>> + */ >>> +void av_force_cpu_count(int count); >> >> I'd prefer if the name was namespaced as av_cpu_ > > For both of these, I copied the behavior of av_force_cpu_flags(). For function names as well as the atomic load. > Let me know which way you prefer. If you prefer your reviewed version, I could also send a patch for similar changes to av_force_cpu_flags() if that makes sense. > > All other remarks changed locally. Looks like you did not rename it to av_cpu_ namespace. > > Thanks, > Thilo > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
Am 16.07.21 um 14:16 schrieb James Almer: > On 6/6/2021 7:44 AM, Thilo Borgmann wrote: >> Hi, >> >> Am 05.06.21 um 16:33 schrieb Anton Khirnov: >>> Quoting Thilo Borgmann (2021-06-05 14:29:05) >>>> Hi, >>>> >>>> add an option to override auto-detection of cpu count. Defaults to auto-detection, of course. >>>> >>>> -Thilo >>>> >>>> From 38612f3e1339354dbaa6be4f36072320ff71c707 Mon Sep 17 00:00:00 2001 >>>> From: Thilo Borgmann <thilo.borgmann@mail.de> >>>> Date: Sat, 5 Jun 2021 14:26:23 +0200 >>>> Subject: [PATCH] fftools/cmdutils.c: Add cmd line option to override detection >>>> of cpu count >>>> >>>> Suggested-By: ffmpeg@fb.com >>>> --- >>>> doc/fftools-common-opts.texi | 7 +++++++ >>>> fftools/cmdutils.c | 27 +++++++++++++++++++++++++++ >>>> fftools/cmdutils.h | 7 +++++++ >>>> libavutil/cpu.c | 14 ++++++++++++++ >>>> libavutil/cpu.h | 6 ++++++ >>>> 5 files changed, 61 insertions(+) >>>> >>>> [...] >> >>>> /** >>>> * Fallback for options that are not explicitly handled, these will be >>>> * parsed through AVOptions. >>>> @@ -239,6 +245,7 @@ void show_help_options(const OptionDef *options, const char *msg, int req_flags, >>>> { "report", 0, { .func_arg = opt_report }, "generate a report" }, \ >>>> { "max_alloc", HAS_ARG, { .func_arg = opt_max_alloc }, "set maximum size of a single allocated block", "bytes" }, \ >>>> { "cpuflags", HAS_ARG | OPT_EXPERT, { .func_arg = opt_cpuflags }, "force specific cpu flags", "flags" }, \ >>>> + { "cpucount", HAS_ARG | OPT_EXPERT, { .func_arg = opt_cpucount }, "force specific cpu count", "count" }, \ >>>> { "hide_banner", OPT_BOOL | OPT_EXPERT, {&hide_banner}, "do not show program banner", "hide_banner" }, \ >>>> CMDUTILS_COMMON_OPTIONS_AVDEVICE \ >>>> diff --git a/libavutil/cpu.c b/libavutil/cpu.c >>>> index 52f6b9a3bf..ccd5b5adac 100644 >>>> --- a/libavutil/cpu.c >>>> +++ b/libavutil/cpu.c >>>> @@ -48,6 +48,7 @@ >>>> #endif >>>> static atomic_int cpu_flags = ATOMIC_VAR_INIT(-1); >>>> +static atomic_int cpu_count = ATOMIC_VAR_INIT(-1); >>>> static int get_cpu_flags(void) >>>> { >>>> @@ -306,14 +307,27 @@ int av_cpu_count(void) >>>> nb_cpus = sysinfo.dwNumberOfProcessors; >>>> #endif >>>> + int count = atomic_load_explicit(&cpu_count, memory_order_relaxed); >>> >>> Just use a normal atomic_load(). >>> >>>> [...] >> >>>> +/** >>>> + * Overrides cpu count detection and forces the specified count. >>>> + * Count < 1 disables forcing of specific count. >>>> + */ >>>> +void av_force_cpu_count(int count); >>> >>> I'd prefer if the name was namespaced as av_cpu_ >> >> For both of these, I copied the behavior of av_force_cpu_flags(). For function names as well as the atomic load. >> Let me know which way you prefer. If you prefer your reviewed version, I could also send a patch for similar changes to av_force_cpu_flags() if that makes sense. >> >> All other remarks changed locally. > > Looks like you did not rename it to av_cpu_ namespace. Indeed. Sent another patch to prefix them all to av_cpu_ Thanks, Thilo
diff --git a/doc/fftools-common-opts.texi b/doc/fftools-common-opts.texi index 5260ecb8f3..7643dd8396 100644 --- a/doc/fftools-common-opts.texi +++ b/doc/fftools-common-opts.texi @@ -353,6 +353,13 @@ Possible flags for this option are: @end table @end table +@item -cpucount @var{count} (@emph{global}) +Override detection of CPU count. This option is intended +for testing. Do not use it unless you know what you're doing. +@example +ffmpeg -cpucount 2 +@end example + @item -max_alloc @var{bytes} Set the maximum size limit for allocating a block on the heap by ffmpeg's family of malloc functions. Exercise @strong{extreme caution} when using diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c index 4eb68d2201..583a706e5d 100644 --- a/fftools/cmdutils.c +++ b/fftools/cmdutils.c @@ -72,6 +72,7 @@ AVDictionary *format_opts, *codec_opts, *resample_opts; static FILE *report_file; static int report_file_level = AV_LOG_DEBUG; int hide_banner = 0; +int cpu_count = -1; enum show_muxdemuxers { SHOW_DEFAULT, @@ -866,6 +867,32 @@ int opt_cpuflags(void *optctx, const char *opt, const char *arg) return 0; } +int opt_cpucount(void *optctx, const char *opt, const char *arg) +{ + int ret; + int count; + + static const AVOption opts[] = { + {"count", NULL, 0, AV_OPT_TYPE_INT, { .i64 = -1}, -1, INT_MAX, NULL}, + {NULL}, + }; + static const AVClass class = { + .class_name = "cpucount", + .item_name = av_default_item_name, + .option = opts, + .version = LIBAVUTIL_VERSION_INT, + }; + const AVClass *pclass = &class; + + ret = av_opt_eval_int(&pclass, opts, arg, &count); + + if (!ret) { + av_force_cpu_count(count); + } + + return ret; +} + int opt_loglevel(void *optctx, const char *opt, const char *arg) { const struct { const char *name; int level; } log_levels[] = { diff --git a/fftools/cmdutils.h b/fftools/cmdutils.h index 1917510589..29a45dd0be 100644 --- a/fftools/cmdutils.h +++ b/fftools/cmdutils.h @@ -50,6 +50,7 @@ extern AVDictionary *sws_dict; extern AVDictionary *swr_opts; extern AVDictionary *format_opts, *codec_opts, *resample_opts; extern int hide_banner; +extern int cpu_count; /** * Register a program-specific cleanup routine. @@ -88,6 +89,11 @@ void log_callback_help(void* ptr, int level, const char* fmt, va_list vl); */ int opt_cpuflags(void *optctx, const char *opt, const char *arg); +/** + * Override the cpucount. + */ +int opt_cpucount(void *optctx, const char *opt, const char *arg); + /** * Fallback for options that are not explicitly handled, these will be * parsed through AVOptions. @@ -239,6 +245,7 @@ void show_help_options(const OptionDef *options, const char *msg, int req_flags, { "report", 0, { .func_arg = opt_report }, "generate a report" }, \ { "max_alloc", HAS_ARG, { .func_arg = opt_max_alloc }, "set maximum size of a single allocated block", "bytes" }, \ { "cpuflags", HAS_ARG | OPT_EXPERT, { .func_arg = opt_cpuflags }, "force specific cpu flags", "flags" }, \ + { "cpucount", HAS_ARG | OPT_EXPERT, { .func_arg = opt_cpucount }, "force specific cpu count", "count" }, \ { "hide_banner", OPT_BOOL | OPT_EXPERT, {&hide_banner}, "do not show program banner", "hide_banner" }, \ CMDUTILS_COMMON_OPTIONS_AVDEVICE \ diff --git a/libavutil/cpu.c b/libavutil/cpu.c index 52f6b9a3bf..ccd5b5adac 100644 --- a/libavutil/cpu.c +++ b/libavutil/cpu.c @@ -48,6 +48,7 @@ #endif static atomic_int cpu_flags = ATOMIC_VAR_INIT(-1); +static atomic_int cpu_count = ATOMIC_VAR_INIT(-1); static int get_cpu_flags(void) { @@ -306,14 +307,27 @@ int av_cpu_count(void) nb_cpus = sysinfo.dwNumberOfProcessors; #endif + int count = atomic_load_explicit(&cpu_count, memory_order_relaxed); + if (!printed) { av_log(NULL, AV_LOG_DEBUG, "detected %d logical cores\n", nb_cpus); + if (count > 0) { + av_log(NULL, AV_LOG_DEBUG, "overriding to %d logical cores\n", count); + } printed = 1; } + if (count > 0) { + nb_cpus = count; + } + return nb_cpus; } +void av_force_cpu_count(int count){ + atomic_store_explicit(&cpu_count, count, memory_order_relaxed); +} + size_t av_cpu_max_align(void) { if (ARCH_MIPS) diff --git a/libavutil/cpu.h b/libavutil/cpu.h index 83099dd969..9c682ad50b 100644 --- a/libavutil/cpu.h +++ b/libavutil/cpu.h @@ -119,6 +119,12 @@ int av_parse_cpu_caps(unsigned *flags, const char *s); */ int av_cpu_count(void); +/** + * Overrides cpu count detection and forces the specified count. + * Count < 1 disables forcing of specific count. + */ +void av_force_cpu_count(int count); + /** * Get the maximum data alignment that may be required by FFmpeg. *
Hi, add an option to override auto-detection of cpu count. Defaults to auto-detection, of course. -Thilo From 38612f3e1339354dbaa6be4f36072320ff71c707 Mon Sep 17 00:00:00 2001 From: Thilo Borgmann <thilo.borgmann@mail.de> Date: Sat, 5 Jun 2021 14:26:23 +0200 Subject: [PATCH] fftools/cmdutils.c: Add cmd line option to override detection of cpu count Suggested-By: ffmpeg@fb.com --- doc/fftools-common-opts.texi | 7 +++++++ fftools/cmdutils.c | 27 +++++++++++++++++++++++++++ fftools/cmdutils.h | 7 +++++++ libavutil/cpu.c | 14 ++++++++++++++ libavutil/cpu.h | 6 ++++++ 5 files changed, 61 insertions(+)