Re: [FFmpeg-devel] [PATCH 1/2] avutil/timer: define macos kperf as AV_READ_TIME

["Martin Storsjö" <martin@martin.st>]

On Wed, 12 Jun 2024, Zhao Zhili wrote:

> From: Zhao Zhili <zhilizhao@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?

// 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-request@ffmpeg.org with subject "unsubscribe".