> On Jun 17, 2024, at 19:15, Martin Storsjö <mar...@martin.st> wrote:
> 
> On Wed, 12 Jun 2024, Zhao Zhili wrote:
> 
>> From: Zhao Zhili <zhiliz...@tencent.com>
>> 
>> Firstly, make ff_kperf_cycles as an implementation of AV_READ_TIME
>> avoids code duplication.
>> 
>> Secondly, fix compilation error since 6a18c0bc87e when macos-kperf
>> is enabled. mach_time.h is included only when CONFIG_MACOS_KPERF
>> is 0. The error happened due to define mach_absolute_time as
>> AV_READ_TIME but missing include mach_time.h. Define macos kperf
>> as AV_READ_TIME fixed the issue.
> 
> Can you elaborate on what your actual goal is here? We have relatively little 
> use of AV_READ_TIME (mostly START/STOP_TIMER), while most benchmarking these 
> days is done via checkasm. Do you have a real case where you want to do 
> benchmarking with this api, outside of checkasm?
> 
> Or do you just want to fix the compilation error? In that case I guess it's 
> possible to fix differently by adding the missing includes.
> 
> By doing this change, we'd be adding one call to ff_thread_once to every 
> single invocation of the timers - which seems suboptimal (even if it probably 
> is quite quick). We don't use Linux perf for AV_READ_TIME either, we only use 
> it in checkasm. So I'd prefer not to do this change, especially unless you 
> have a concrete case where you actively desire to use START/STOP_TIMER 
> benchmarking with macOS kperf?

I’m trying to fix the missing include header file first. Then I saw 
ff_kperf_init() is called each time by START_TIMER, which can be simplified by 
merge ff_kperf_init into ff_kperf_cycles.

#define START_TIMER                                                         \
    uint64_t tperf;                                                         \
    ff_kperf_init();                                                        \
    tperf = ff_kperf_cycles();

Now I think I have chose the wrong example. checkasm bench_init_kperf is the 
right one.

We can remove the ff_thread_once in ff_kperf_init, and let caller make 
guarantee to only call it once. But kperf is only for test, so not urgent to do 
such change.

Will add missing include in v2.

> 
> // 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".

Reply via email to