cisco / openh264

Open Source H.264 Codec
BSD 2-Clause "Simplified" License
5.51k stars 1.78k forks source link

Incorrect detection of logical CPU number #3725

Open Lastique opened 8 months ago

Lastique commented 8 months ago

On a Core i7 12700K libopenh264 detects 64 logical cores as it prints "NUMBER OF LOGIC PROCESSORS ON CHIP: 64" during encoder context initialization. On this CPU, with E-cores disabled, there are 8 physical cores with HT (i.e. a total of 16 logical cores).

The problem is in this portion of code:

https://github.com/cisco/openh264/blob/b29fd81e72b9c668d2c86a8e088e669ac956baf9/codec/common/src/cpu.cpp#L158-L171

First, the line 159 uses a stale value of uiFeatureB that was initialized at line 141 by calling WelsCPUId(7, ...). Leaf 7 does not contain information about logical core count, and the value obtained here is basically garbage. It looks like the intention was to use uiFeatureB initialized at line 75 by WelsCPUId(1, ...).

Next, the line 168 uses bits 26-31 of EAX of CPUID leaf 4 to deduce the number of logical cores. This is incorrect as this field indicates the maximum number of core IDs per package, not the actual number of logical processors. And indeed, on my CPU this field reads the value of 63.

To obtain the number of logical/physical processors you should parse CPUID leaf 0x1F or, if not supported, 0x0B. See Intel Software Developer's Manual, CPUID description for details.

yump commented 6 months ago

IMO CPUID is not a good source of information on the number of threads a parallel program should use. CPUs may be offlined, or your process may be affined to a subset of all CPUs on the system. Better to ask the OS (sched_getaffinity on Linux).

Lastique commented 6 months ago

I don't disagree, but still the current code is incorrect.

Talaqalotaibipmp commented 1 week ago

https://www.foxit.com/word-to-pdf/