[FFmpeg-devel] [PATCH] fftools/cmdutils.c: Add cmd line option to override detection of cpu count

[Thilo Borgmann]

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 at 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 at 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