| Message ID | tencent_9A5AB234E1811C35893E15575F9D7AE17609@qq.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,v2,1/4] avutil/aarch64: Skip define AV_READ_TIME for apple | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On Fri, 7 Jun 2024, Zhao Zhili wrote: > From: Zhao Zhili <zhilizhao@tencent.com> > > B0 is defined by system header. Can you add more details about which header defines this? (I did a quick grep in a copy of the android NDK, and found it in asm-generic/termbits-common.h.) // Martin
> On Jun 7, 2024, at 16:21, Martin Storsjö <martin@martin.st> wrote: > > On Fri, 7 Jun 2024, Zhao Zhili wrote: > >> From: Zhao Zhili <zhilizhao@tencent.com> >> >> B0 is defined by system header. > > Can you add more details about which header defines this? (I did a quick grep in a copy of the android NDK, and found it in asm-generic/termbits-common.h.) The include path is: timer.h => <sys/ioctl.h> => <linux/termios.h> => <asm/termios.h> => <asm-generic/termios.h> => <asm/termbits.h> => <asm-generic/termbits.h> => <asm-generic/termbits-common.h> #define B0 0x00000000 #define B50 0x00000001 #define B75 0x00000002 The issue has occurred multiple times, e.g., commit f0f596dbc6b45b544d2d2d4fb78c0a2bdc3e6eb1 avutil/internal: remove timer.h again timer.h has been removed from internal.h, and then added back with 3e6088f for convenience. This patch removed it again for the following reasons: 1. Only includes what's necessary is a common and safe strategy. 2. It fixed some build errors on Android: a. libavutil/timer.h includes sys/ioctl.h, and ioctl.h includes termios.h on Android. b. termios.h reserves names prefixed with ‘c_’, ‘V’, ‘I’, ‘O’, and ‘TC’; and names prefixed with ‘B’ followed by a digit. c. libavcodec uses B0 B1 and so on as variable names a lot. So the code failed to build with --enable-linux-perf, or --target-os=Linux. > > // 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".
On Fri, 7 Jun 2024, Martin Storsjö wrote: > On Fri, 7 Jun 2024, Zhao Zhili wrote: > >> From: Zhao Zhili <zhilizhao@tencent.com> >> >> B0 is defined by system header. > > Can you add more details about which header defines this? (I did a quick grep > in a copy of the android NDK, and found it in asm-generic/termbits-common.h.) Btw, in addition to this, is perf actually usable for doing benchmarking with checkasm on android, or is this just to avoid build errors? // Martin
On Fri, 7 Jun 2024, Zhao Zhili wrote: > > >> On Jun 7, 2024, at 16:21, Martin Storsjö <martin@martin.st> wrote: >> >> On Fri, 7 Jun 2024, Zhao Zhili wrote: >> >>> From: Zhao Zhili <zhilizhao@tencent.com> >>> >>> B0 is defined by system header. >> >> Can you add more details about which header defines this? (I did a quick grep in a copy of the android NDK, and found it in asm-generic/termbits-common.h.) > > The include path is: > > timer.h => > <sys/ioctl.h> => > <linux/termios.h> => > <asm/termios.h> => > <asm-generic/termios.h> => > <asm/termbits.h> => > <asm-generic/termbits.h> => > <asm-generic/termbits-common.h> > > #define B0 0x00000000 > #define B50 0x00000001 > #define B75 0x00000002 > > The issue has occurred multiple times, e.g., > > commit f0f596dbc6b45b544d2d2d4fb78c0a2bdc3e6eb1 > > avutil/internal: remove timer.h again > > timer.h has been removed from internal.h, and then added back with > 3e6088f for convenience. This patch removed it again for the > following reasons: > > 1. Only includes what's necessary is a common and safe strategy. > > 2. It fixed some build errors on Android: > a. libavutil/timer.h includes sys/ioctl.h, and ioctl.h includes > termios.h on Android. > b. termios.h reserves names prefixed with ‘c_’, ‘V’, ‘I’, ‘O’, and > ‘TC’; and names prefixed with ‘B’ followed by a digit. > c. libavcodec uses B0 B1 and so on as variable names a lot. So > the code failed to build with --enable-linux-perf, or > --target-os=Linux. Ah, that's a good explanation. Please reference this commit in this one, then it's good with me! // Martin
> On Jun 7, 2024, at 16:38, Martin Storsjö <martin@martin.st> wrote: > > On Fri, 7 Jun 2024, Martin Storsjö wrote: > >> On Fri, 7 Jun 2024, Zhao Zhili wrote: >> >>> From: Zhao Zhili <zhilizhao@tencent.com> >>> B0 is defined by system header. >> >> Can you add more details about which header defines this? (I did a quick grep in a copy of the android NDK, and found it in asm-generic/termbits-common.h.) > > Btw, in addition to this, is perf actually usable for doing benchmarking with checkasm on android, or is this just to avoid build errors? There is a builtin cmd simpleperf on Android, the source code is at [1], which is a simplified version of perf on Linux. I have tested checkasm with linux perf on Android, the result seems reliable. I’m not sure whether there is any difference between perf on Android and on normal linux. https://android.googlesource.com/platform/system/extras/+/refs/heads/main/simpleperf/main.cpp#29 > > // 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".
diff --git a/tests/checkasm/llviddsp.c b/tests/checkasm/llviddsp.c index b75c0ea099..9f8de65df4 100644 --- a/tests/checkasm/llviddsp.c +++ b/tests/checkasm/llviddsp.c @@ -71,7 +71,7 @@ static void check_add_bytes(LLVidDSPContext *c, int width) } static void check_add_median_pred(LLVidDSPContext *c, int width) { - int A0, A1, B0, B1; + int a0, a1, b0, b1; uint8_t *dst0 = av_mallocz(width); uint8_t *dst1 = av_mallocz(width); uint8_t *src0 = av_calloc(width, sizeof(*src0)); @@ -85,18 +85,18 @@ static void check_add_median_pred(LLVidDSPContext *c, int width) { init_buffer(src0, src1, uint8_t, width); init_buffer(diff0, diff1, uint8_t, width); - A0 = rnd() & 0xFF; - B0 = rnd() & 0xFF; - A1 = A0; - B1 = B0; + a0 = rnd() & 0xFF; + b0 = rnd() & 0xFF; + a1 = a0; + b1 = b0; if (check_func(c->add_median_pred, "add_median_pred")) { - call_ref(dst0, src0, diff0, width, &A0, &B0); - call_new(dst1, src1, diff1, width, &A1, &B1); - if (memcmp(dst0, dst1, width) || (A0 != A1) || (B0 != B1)) + call_ref(dst0, src0, diff0, width, &a0, &b0); + call_new(dst1, src1, diff1, width, &a1, &b1); + if (memcmp(dst0, dst1, width) || (a0 != a1) || (b0 != b1)) fail(); - bench_new(dst1, src1, diff1, width, &A1, &B1); + bench_new(dst1, src1, diff1, width, &a1, &b1); } av_free(src0);
From: Zhao Zhili <zhilizhao@tencent.com> B0 is defined by system header. --- tests/checkasm/llviddsp.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)