> On Jun 17, 2024, at 19:10, Martin Storsjö <mar...@martin.st> wrote: > > On Wed, 12 Jun 2024, Zhao Zhili wrote: > >> From: Zhao Zhili <zhiliz...@tencent.com> >> >> On m1, kpc_get_counter_count(KPC_MASK) return 8. The exact value >> doesn't matter in our case. > > This is somewhat unexpected, I had expected that this API was originally > tested on an m1. I guess it might depend on what OS version you're using as > well?
On arm64 [1] #define KPC_ARM64_FIXED_COUNT (2) #define KPC_ARM64_CONFIGURABLE_COUNT (CORE_NCTRS - KPC_ARM64_FIXED_COUNT) #define KPC_MAX_COUNTERS (KPC_ARM64_FIXED_COUNT + KPC_ARM64_CONFIGURABLE_COUNT + 1) On x86_64: #define KPC_MAX_COUNTERS 32 [1] https://github.com/apple/darwin-xnu/blob/2ff845c2e033bd0ff64b5b6aa6063a1f8f65aa32/osfmk/arm64/machine_kpc.h#L36 > >> --- >> libavutil/macos_kperf.c | 15 +++++++++------ >> 1 file changed, 9 insertions(+), 6 deletions(-) >> >> diff --git a/libavutil/macos_kperf.c b/libavutil/macos_kperf.c >> index a0bc845fd3..906b276a34 100644 >> --- a/libavutil/macos_kperf.c >> +++ b/libavutil/macos_kperf.c >> @@ -67,14 +67,15 @@ KPERF_LIST >> #define KPC_CLASS_POWER_MASK (1 << 2) >> #define KPC_CLASS_RAWPMU_MASK (1 << 3) >> >> -#define COUNTERS_COUNT 10 >> +#define KPC_MAX_COUNTERS 32 >> #define CONFIG_COUNT 8 >> #define KPC_MASK (KPC_CLASS_CONFIGURABLE_MASK | KPC_CLASS_FIXED_MASK) >> >> static void kperf_init(void) >> { >> - uint64_t config[COUNTERS_COUNT] = {0}; >> + uint64_t config[CONFIG_COUNT] = {0}; > > Hmm, this changes the array from 10 to 8 elements. While the change looks > reasonable based on the variable names, I just wanted to doublecheck that we > have some clues that this is right? The change is base on the check av_assert0(kpc_get_config_count(KPC_MASK) == CONFIG_COUNT > >> void *kperf = NULL; >> + uint32_t n; >> >> av_assert0(kperf = >> dlopen("/System/Library/PrivateFrameworks/kperf.framework/Versions/A/kperf", >> RTLD_LAZY)); >> >> @@ -82,8 +83,10 @@ static void kperf_init(void) >> KPERF_LIST >> #undef F >> >> - av_assert0(kpc_get_counter_count(KPC_MASK) == COUNTERS_COUNT); >> - av_assert0(kpc_get_config_count(KPC_MASK) == CONFIG_COUNT); >> + n = kpc_get_counter_count(KPC_MASK); >> + av_assert0(n <= KPC_MAX_COUNTERS); >> + n = kpc_get_config_count(KPC_MASK); >> + av_assert0(n <= CONFIG_COUNT); > > I guess this is the actual functional change here, I think this seems right. > > I CC's Josh on this change too, in case he has something to add here, but it > looks mostly reasonable to me. > > // Martin > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
