On Mon, Jun 17, 2024, at 13:10, Martin Storsjö 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?
>
>> ---
>> 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?
>
>>     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.

It’s a private kernel API there are no guarantees about it working at any 
point. I just know that it used to work. If it needs changes to work currently 
and this fixes it for you then that’s fine, you should probably also port this 
change to dav1d. I’m personally unable to test this as I am just running Linux 
on my M1 machine now.

-- 
jd
_______________________________________________
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".

Reply via email to