On Mon, 17 Jun 2024, Zhao Zhili wrote:
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.
Oh, right, I had entirely missed that we already do this - so both Linux
perf and macOS kperf are used for START/STOP_TIMER, they're just not used
for AV_READ_TIME so far. I see...
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.
In any case, I much rather have ff_thread_once in an _init function, than
in every single timer invocation. Not sure if it's worth trying to get rid
of the ff_thread_once from there.
// 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".
