[FFmpeg-devel] [PATCH 1/4] checkasm: Make checkasm_fail_func return whether we should print verbosely

[FFmpeg-devel] [PATCH 1/4] checkasm: Make checkasm_fail_func return whether we should print verbosely

[Martin Storsjö]

This makes it easier to implement custom error printouts in tests.

This is a port of dav1d's commit
13a7d78655f8747c2cd01e8a48d44dcc7f60a8e5 into ffmpeg's checkasm.
---
 tests/checkasm/checkasm.c | 9 +++++----
 tests/checkasm/checkasm.h | 2 +-
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/tests/checkasm/checkasm.c b/tests/checkasm/checkasm.c
index 14742081ca..c6d641c52b 100644
--- a/tests/checkasm/checkasm.c
+++ b/tests/checkasm/checkasm.c
@@ -1081,8 +1081,9 @@ int checkasm_bench_func(void)
            !wildstrcmp(state.current_func->name, state.bench_pattern);
 }
 
-/* Indicate that the current test has failed */
-void checkasm_fail_func(const char *msg, ...)
+/* Indicate that the current test has failed, return whether verbose printing
+ * is requested. */
+int checkasm_fail_func(const char *msg, ...)
 {
     if (state.current_func_ver && state.current_func_ver->cpu &&
         state.current_func_ver->ok)
@@ -1099,6 +1100,7 @@ void checkasm_fail_func(const char *msg, ...)
         state.current_func_ver->ok = 0;
         state.num_failed++;
     }
+    return state.verbose;
 }
 
 void checkasm_set_signal_handler_state(int enabled) {
@@ -1180,8 +1182,7 @@ int checkasm_check_##type(const char *file, int line, \
             break; \
     if (y == h) \
         return 0; \
-    checkasm_fail_func("%s:%d", file, line); \
-    if (!state.verbose) \
+    if (!checkasm_fail_func("%s:%d", file, line)) \
         return 1; \
     fprintf(stderr, "%s:\n", name); \
     while (h--) { \
diff --git a/tests/checkasm/checkasm.h b/tests/checkasm/checkasm.h
index 75c81317ba..852d6fca64 100644
--- a/tests/checkasm/checkasm.h
+++ b/tests/checkasm/checkasm.h
@@ -152,7 +152,7 @@ struct CheckasmPerf;
 
 void *checkasm_check_func(void *func, const char *name, ...) av_printf_format(2, 3);
 int checkasm_bench_func(void);
-void checkasm_fail_func(const char *msg, ...) av_printf_format(1, 2);
+int checkasm_fail_func(const char *msg, ...) av_printf_format(1, 2);
 struct CheckasmPerf *checkasm_get_perf_context(void);
 void checkasm_report(const char *name, ...) av_printf_format(1, 2);
 void checkasm_set_signal_handler_state(int enabled);
-- 
2.39.5 (Apple Git-154)

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

[FFmpeg-devel] [PATCH 2/4] checkasm: Implement helpers for defining and checking padded rects

[Martin Storsjö]

This backports similar functionality from dav1d, from commits
35d1d011fda4a92bcaf42d30ed137583b27d7f6d and
d130da9c315d5a1d3968d278bbee2238ad9051e7.

This allows detecting writes out of bounds, on all 4 sides of
the intended destination rectangle.

The bounds checking also can optionally allow small overwrites
(up to a specified alignment), while still checking for larger
overwrites past the intended allowed region.
---
 tests/checkasm/checkasm.c | 89 ++++++++++++++++++++++++++++++---------
 tests/checkasm/checkasm.h | 55 ++++++++++++++++++++----
 2 files changed, 116 insertions(+), 28 deletions(-)

diff --git a/tests/checkasm/checkasm.c b/tests/checkasm/checkasm.c
index c6d641c52b..a5b862fe52 100644
--- a/tests/checkasm/checkasm.c
+++ b/tests/checkasm/checkasm.c
@@ -1168,37 +1168,88 @@ void checkasm_report(const char *name, ...)
     }
 }
 
+static int check_err(const char *file, int line,
+                     const char *name, int w, int h,
+                     int *err)
+{
+    if (*err)
+        return 0;
+    if (!checkasm_fail_func("%s:%d", file, line))
+        return 1;
+    *err = 1;
+    fprintf(stderr, "%s (%dx%d):\n", name, w, h);
+    return 0;
+}
+
 #define DEF_CHECKASM_CHECK_FUNC(type, fmt) \
 int checkasm_check_##type(const char *file, int line, \
                           const type *buf1, ptrdiff_t stride1, \
                           const type *buf2, ptrdiff_t stride2, \
-                          int w, int h, const char *name) \
+                          int w, int h, const char *name, \
+                          int align_w, int align_h, \
+                          int padding) \
 { \
+    int aligned_w = (w + align_w - 1) & ~(align_w - 1); \
+    int aligned_h = (h + align_h - 1) & ~(align_h - 1); \
+    int err = 0; \
     int y = 0; \
     stride1 /= sizeof(*buf1); \
     stride2 /= sizeof(*buf2); \
     for (y = 0; y < h; y++) \
         if (memcmp(&buf1[y*stride1], &buf2[y*stride2], w*sizeof(*buf1))) \
             break; \
-    if (y == h) \
-        return 0; \
-    if (!checkasm_fail_func("%s:%d", file, line)) \
-        return 1; \
-    fprintf(stderr, "%s:\n", name); \
-    while (h--) { \
-        for (int x = 0; x < w; x++) \
-            fprintf(stderr, " " fmt, buf1[x]); \
-        fprintf(stderr, "    "); \
-        for (int x = 0; x < w; x++) \
-            fprintf(stderr, " " fmt, buf2[x]); \
-        fprintf(stderr, "    "); \
-        for (int x = 0; x < w; x++) \
-            fprintf(stderr, "%c", buf1[x] != buf2[x] ? 'x' : '.'); \
-        buf1 += stride1; \
-        buf2 += stride2; \
-        fprintf(stderr, "\n"); \
+    if (y != h) { \
+        if (check_err(file, line, name, w, h, &err)) \
+            return 1; \
+        for (y = 0; y < h; y++) { \
+            for (int x = 0; x < w; x++) \
+                fprintf(stderr, " " fmt, buf1[x]); \
+            fprintf(stderr, "    "); \
+            for (int x = 0; x < w; x++) \
+                fprintf(stderr, " " fmt, buf2[x]); \
+            fprintf(stderr, "    "); \
+            for (int x = 0; x < w; x++) \
+                fprintf(stderr, "%c", buf1[x] != buf2[x] ? 'x' : '.'); \
+            buf1 += stride1; \
+            buf2 += stride2; \
+            fprintf(stderr, "\n"); \
+        } \
+        buf1 -= h*stride1; \
+        buf2 -= h*stride2; \
     } \
-    return 1; \
+    for (y = -padding; y < 0; y++) \
+        if (memcmp(&buf1[y*stride1 - padding], &buf2[y*stride2 - padding], \
+                   (w + 2*padding)*sizeof(*buf1))) { \
+            if (check_err(file, line, name, w, h, &err)) \
+                return 1; \
+            fprintf(stderr, " overwrite above\n"); \
+            break; \
+        } \
+    for (y = aligned_h; y < aligned_h + padding; y++) \
+        if (memcmp(&buf1[y*stride1 - padding], &buf2[y*stride2 - padding], \
+                   (w + 2*padding)*sizeof(*buf1))) { \
+            if (check_err(file, line, name, w, h, &err)) \
+                return 1; \
+            fprintf(stderr, " overwrite below\n"); \
+            break; \
+        } \
+    for (y = 0; y < h; y++) \
+        if (memcmp(&buf1[y*stride1 - padding], &buf2[y*stride2 - padding], \
+                   padding*sizeof(*buf1))) { \
+            if (check_err(file, line, name, w, h, &err)) \
+                return 1; \
+            fprintf(stderr, " overwrite left\n"); \
+            break; \
+        } \
+    for (y = 0; y < h; y++) \
+        if (memcmp(&buf1[y*stride1 + aligned_w], &buf2[y*stride2 + aligned_w], \
+                   padding*sizeof(*buf1))) { \
+            if (check_err(file, line, name, w, h, &err)) \
+                return 1; \
+            fprintf(stderr, " overwrite right\n"); \
+            break; \
+        } \
+    return err; \
 }
 
 DEF_CHECKASM_CHECK_FUNC(uint8_t,  "%02x")
diff --git a/tests/checkasm/checkasm.h b/tests/checkasm/checkasm.h
index 852d6fca64..ca7cfac3f7 100644
--- a/tests/checkasm/checkasm.h
+++ b/tests/checkasm/checkasm.h
@@ -375,11 +375,30 @@ typedef struct CheckasmPerf {
 #define PERF_STOP(t)   while(0)
 #endif
 
+#define BUF_RECT(type, name, w, h) \
+    LOCAL_ALIGNED_32(type, name##_buf, [((h)+32)*(FFALIGN(w,64)+64) + 64]); \
+    av_unused ptrdiff_t name##_stride = sizeof(type)*(FFALIGN(w,64)+64); \
+    av_unused int name##_buf_h = (h)+32; \
+    type *name = name##_buf + (FFALIGN(w,64)+64)*16 + 64
+
+#define PIXEL_RECT(name, w, h) \
+    LOCAL_ALIGNED_32(uint8_t, name##_buf, [sizeof(uint16_t) * (((h)+32)*(FFALIGN(w,64)+64) + 64)],); \
+    av_unused ptrdiff_t name##_stride = sizeof(uint16_t) * (FFALIGN(w,64)+64); \
+    av_unused int name##_buf_h = (h)+32; \
+    uint8_t *name = name##_buf + (FFALIGN(w,64)+64)*16 + 64
+
+#define CLEAR_BUF_RECT(name) \
+    memset(name##_buf, 0x99, name##_stride * name##_buf_h + 64)
+#define CLEAR_PIXEL_RECT(name) \
+    CLEAR_BUF_RECT(name)
+
 #define DECL_CHECKASM_CHECK_FUNC(type) \
 int checkasm_check_##type(const char *file, int line, \
                           const type *buf1, ptrdiff_t stride1, \
                           const type *buf2, ptrdiff_t stride2, \
-                          int w, int h, const char *name)
+                          int w, int h, const char *name, \
+                          int align_w, int align_h, \
+                          int padding)
 
 DECL_CHECKASM_CHECK_FUNC(uint8_t);
 DECL_CHECKASM_CHECK_FUNC(uint16_t);
@@ -390,18 +409,36 @@ DECL_CHECKASM_CHECK_FUNC(int32_t);
 #define PASTE(a,b) a ## b
 #define CONCAT(a,b) PASTE(a,b)
 
-#define checkasm_check(prefix, ...) CONCAT(checkasm_check_, prefix)(__FILE__, __LINE__, __VA_ARGS__)
+#define checkasm_check2(prefix, ...) CONCAT(checkasm_check_, prefix)(__FILE__, __LINE__, __VA_ARGS__)
+#define checkasm_check(prefix, ...) checkasm_check2(prefix, __VA_ARGS__, 0, 0, 0)
+/* Check a pointer from BUF_RECT, checking whether there have been
+ * writes outside of the designated area. */
+#define checkasm_check_padded(...) \
+    checkasm_check2(__VA_ARGS__, 1, 1, 8)
+/* Check a pointer from BUF_RECT, checking whether there have been
+ * writes outside of the designated area. Allow writing slightly past the
+ * end of the buffer, by aligning w/h to align_w/align_h, and checking
+ * for overwrites outside of that. */
+#define checkasm_check_padded_align(...) \
+    checkasm_check2(__VA_ARGS__, 8)
 
 /* This assumes that there is a local variable named "bit_depth".
  * For tests that don't have that and only operate on a single
  * bitdepth, just call checkasm_check(uint8_t, ...) directly. */
-#define checkasm_check_pixel(buf1, stride1, buf2, stride2, ...) \
+#define checkasm_check_pixel2(buf1, stride1, buf2, stride2, ...) \
     ((bit_depth > 8) ?                                          \
-     checkasm_check(uint16_t, (const uint16_t*)buf1, stride1,   \
-                              (const uint16_t*)buf2, stride2,   \
-                              __VA_ARGS__) :                    \
-     checkasm_check(uint8_t,  (const uint8_t*) buf1, stride1,   \
-                              (const uint8_t*) buf2, stride2,   \
-                              __VA_ARGS__))
+     checkasm_check2(uint16_t, (const uint16_t*)buf1, stride1,   \
+                               (const uint16_t*)buf2, stride2,   \
+                               __VA_ARGS__) :                    \
+     checkasm_check2(uint8_t,  (const uint8_t*) buf1, stride1,   \
+                               (const uint8_t*) buf2, stride2,   \
+                               __VA_ARGS__))
+#define checkasm_check_pixel(...) \
+    checkasm_check_pixel2(__VA_ARGS__, 0, 0, 0)
+#define checkasm_check_pixel_padded(...) \
+    checkasm_check_pixel2(__VA_ARGS__, 1, 1, 8)
+#define checkasm_check_pixel_padded_align(...) \
+    checkasm_check_pixel2(__VA_ARGS__, 8)
+
 
 #endif /* TESTS_CHECKASM_CHECKASM_H */
-- 
2.39.5 (Apple Git-154)

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

Re: [FFmpeg-devel] [PATCH 2/4] checkasm: Implement helpers for defining and checking padded rects

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 2254 bytes --]

On Wed, Mar 26, 2025 at 12:30:13PM +0200, Martin Storsjö wrote:
> This backports similar functionality from dav1d, from commits
> 35d1d011fda4a92bcaf42d30ed137583b27d7f6d and
> d130da9c315d5a1d3968d278bbee2238ad9051e7.
> 
> This allows detecting writes out of bounds, on all 4 sides of
> the intended destination rectangle.
> 
> The bounds checking also can optionally allow small overwrites
> (up to a specified alignment), while still checking for larger
> overwrites past the intended allowed region.
> ---
>  tests/checkasm/checkasm.c | 89 ++++++++++++++++++++++++++++++---------
>  tests/checkasm/checkasm.h | 55 ++++++++++++++++++++----
>  2 files changed, 116 insertions(+), 28 deletions(-)
> 
> diff --git a/tests/checkasm/checkasm.c b/tests/checkasm/checkasm.c
> index c6d641c52b..a5b862fe52 100644
> --- a/tests/checkasm/checkasm.c
> +++ b/tests/checkasm/checkasm.c
> @@ -1168,37 +1168,88 @@ void checkasm_report(const char *name, ...)
>      }
>  }
>  
> +static int check_err(const char *file, int line,
> +                     const char *name, int w, int h,
> +                     int *err)
> +{
> +    if (*err)
> +        return 0;
> +    if (!checkasm_fail_func("%s:%d", file, line))
> +        return 1;
> +    *err = 1;
> +    fprintf(stderr, "%s (%dx%d):\n", name, w, h);
> +    return 0;
> +}
> +
>  #define DEF_CHECKASM_CHECK_FUNC(type, fmt) \
>  int checkasm_check_##type(const char *file, int line, \
>                            const type *buf1, ptrdiff_t stride1, \
>                            const type *buf2, ptrdiff_t stride2, \
> -                          int w, int h, const char *name) \
> +                          int w, int h, const char *name, \
> +                          int align_w, int align_h, \
> +                          int padding) \
>  { \

> +    int aligned_w = (w + align_w - 1) & ~(align_w - 1); \
> +    int aligned_h = (h + align_h - 1) & ~(align_h - 1); \

this can overflow
feel free to fix in a seperate patch

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- Epicurus

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

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

Re: [FFmpeg-devel] [PATCH 2/4] checkasm: Implement helpers for defining and checking padded rects

[Martin Storsjö]

On Sat, 29 Mar 2025, Michael Niedermayer wrote:

> On Wed, Mar 26, 2025 at 12:30:13PM +0200, Martin Storsjö wrote:
>> This backports similar functionality from dav1d, from commits
>> 35d1d011fda4a92bcaf42d30ed137583b27d7f6d and
>> d130da9c315d5a1d3968d278bbee2238ad9051e7.
>>
>> This allows detecting writes out of bounds, on all 4 sides of
>> the intended destination rectangle.
>>
>> The bounds checking also can optionally allow small overwrites
>> (up to a specified alignment), while still checking for larger
>> overwrites past the intended allowed region.
>> ---
>>  tests/checkasm/checkasm.c | 89 ++++++++++++++++++++++++++++++---------
>>  tests/checkasm/checkasm.h | 55 ++++++++++++++++++++----
>>  2 files changed, 116 insertions(+), 28 deletions(-)
>>
>> diff --git a/tests/checkasm/checkasm.c b/tests/checkasm/checkasm.c
>> index c6d641c52b..a5b862fe52 100644
>> --- a/tests/checkasm/checkasm.c
>> +++ b/tests/checkasm/checkasm.c
>> @@ -1168,37 +1168,88 @@ void checkasm_report(const char *name, ...)
>>      }
>>  }
>>
>> +static int check_err(const char *file, int line,
>> +                     const char *name, int w, int h,
>> +                     int *err)
>> +{
>> +    if (*err)
>> +        return 0;
>> +    if (!checkasm_fail_func("%s:%d", file, line))
>> +        return 1;
>> +    *err = 1;
>> +    fprintf(stderr, "%s (%dx%d):\n", name, w, h);
>> +    return 0;
>> +}
>> +
>>  #define DEF_CHECKASM_CHECK_FUNC(type, fmt) \
>>  int checkasm_check_##type(const char *file, int line, \
>>                            const type *buf1, ptrdiff_t stride1, \
>>                            const type *buf2, ptrdiff_t stride2, \
>> -                          int w, int h, const char *name) \
>> +                          int w, int h, const char *name, \
>> +                          int align_w, int align_h, \
>> +                          int padding) \
>>  { \
>
>> +    int aligned_w = (w + align_w - 1) & ~(align_w - 1); \
>> +    int aligned_h = (h + align_h - 1) & ~(align_h - 1); \
>
> this can overflow
> feel free to fix in a seperate patch

Feel free to propose a patch for how you'd prefer to have it fixed then... 
I don't see this as a real world problem - w and h are bounded by the 
tests themselves, and likewise the alignments - I don't see us having 
tests using buffers with a width near INT32_MAX?

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

Re: [FFmpeg-devel] [PATCH 2/4] checkasm: Implement helpers for defining and checking padded rects

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 3176 bytes --]

Hi Martin

On Mon, Mar 31, 2025 at 04:05:27PM +0300, Martin Storsjö wrote:
> On Sat, 29 Mar 2025, Michael Niedermayer wrote:
> 
> > On Wed, Mar 26, 2025 at 12:30:13PM +0200, Martin Storsjö wrote:
> > > This backports similar functionality from dav1d, from commits
> > > 35d1d011fda4a92bcaf42d30ed137583b27d7f6d and
> > > d130da9c315d5a1d3968d278bbee2238ad9051e7.
> > > 
> > > This allows detecting writes out of bounds, on all 4 sides of
> > > the intended destination rectangle.
> > > 
> > > The bounds checking also can optionally allow small overwrites
> > > (up to a specified alignment), while still checking for larger
> > > overwrites past the intended allowed region.
> > > ---
> > >  tests/checkasm/checkasm.c | 89 ++++++++++++++++++++++++++++++---------
> > >  tests/checkasm/checkasm.h | 55 ++++++++++++++++++++----
> > >  2 files changed, 116 insertions(+), 28 deletions(-)
> > > 
> > > diff --git a/tests/checkasm/checkasm.c b/tests/checkasm/checkasm.c
> > > index c6d641c52b..a5b862fe52 100644
> > > --- a/tests/checkasm/checkasm.c
> > > +++ b/tests/checkasm/checkasm.c
> > > @@ -1168,37 +1168,88 @@ void checkasm_report(const char *name, ...)
> > >      }
> > >  }
> > > 
> > > +static int check_err(const char *file, int line,
> > > +                     const char *name, int w, int h,
> > > +                     int *err)
> > > +{
> > > +    if (*err)
> > > +        return 0;
> > > +    if (!checkasm_fail_func("%s:%d", file, line))
> > > +        return 1;
> > > +    *err = 1;
> > > +    fprintf(stderr, "%s (%dx%d):\n", name, w, h);
> > > +    return 0;
> > > +}
> > > +
> > >  #define DEF_CHECKASM_CHECK_FUNC(type, fmt) \
> > >  int checkasm_check_##type(const char *file, int line, \
> > >                            const type *buf1, ptrdiff_t stride1, \
> > >                            const type *buf2, ptrdiff_t stride2, \
> > > -                          int w, int h, const char *name) \
> > > +                          int w, int h, const char *name, \
> > > +                          int align_w, int align_h, \
> > > +                          int padding) \
> > >  { \
> > 
> > > +    int aligned_w = (w + align_w - 1) & ~(align_w - 1); \
> > > +    int aligned_h = (h + align_h - 1) & ~(align_h - 1); \
> > 
> > this can overflow
> > feel free to fix in a seperate patch
> 
> Feel free to propose a patch for how you'd prefer to have it fixed then... I
> don't see this as a real world problem - w and h are bounded by the tests
> themselves, and likewise the alignments - I don't see us having tests using
> buffers with a width near INT32_MAX?

maybe but then
if we want our asm code to handle such extrem cases, something needs to
test it

ill send a patch based on this: (once your patches are in / minus in case
i forget)

int64_t aligned_w = (w - 1LL + align_w) & ~(align_w - 1); \
if (aligned_w != (int32_t)aligned_w)
    return AVERROR(EINVAL)

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

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

Re: [FFmpeg-devel] [PATCH 2/4] checkasm: Implement helpers for defining and checking padded rects

[Martin Storsjö]

On Tue, 1 Apr 2025, Michael Niedermayer wrote:

>>>>  #define DEF_CHECKASM_CHECK_FUNC(type, fmt) \
>>>>  int checkasm_check_##type(const char *file, int line, \
>>>>                            const type *buf1, ptrdiff_t stride1, \
>>>>                            const type *buf2, ptrdiff_t stride2, \
>>>> -                          int w, int h, const char *name) \
>>>> +                          int w, int h, const char *name, \
>>>> +                          int align_w, int align_h, \
>>>> +                          int padding) \
>>>>  { \
>>>
>>>> +    int aligned_w = (w + align_w - 1) & ~(align_w - 1); \
>>>> +    int aligned_h = (h + align_h - 1) & ~(align_h - 1); \
>>>
>>> this can overflow
>>> feel free to fix in a seperate patch
>>
>> Feel free to propose a patch for how you'd prefer to have it fixed then... I
>> don't see this as a real world problem - w and h are bounded by the tests
>> themselves, and likewise the alignments - I don't see us having tests using
>> buffers with a width near INT32_MAX?
>
> maybe but then
> if we want our asm code to handle such extrem cases, something needs to
> test it
>
> ill send a patch based on this: (once your patches are in / minus in case
> i forget)
>
> int64_t aligned_w = (w - 1LL + align_w) & ~(align_w - 1); \
> if (aligned_w != (int32_t)aligned_w)
>    return AVERROR(EINVAL)

Ok, if you find that important.

I'll go ahead and push patches 1-2 and 4 from this set, soon, holding off 
of patch 3/4 while waiting for someone to fix the Loongarch HEVC SIMD.

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

Re: [FFmpeg-devel] [PATCH 2/4] checkasm: Implement helpers for defining and checking padded rects

[Martin Storsjö]

On Tue, 1 Apr 2025, Martin Storsjö wrote:

> On Tue, 1 Apr 2025, Michael Niedermayer wrote:
>
>>>>>  #define DEF_CHECKASM_CHECK_FUNC(type, fmt) \
>>>>>  int checkasm_check_##type(const char *file, int line, \
>>>>>                            const type *buf1, ptrdiff_t stride1, \
>>>>>                            const type *buf2, ptrdiff_t stride2, \
>>>>> -                          int w, int h, const char *name) \
>>>>> +                          int w, int h, const char *name, \
>>>>> +                          int align_w, int align_h, \
>>>>> +                          int padding) \
>>>>>  { \
>>>> 
>>>>> +    int aligned_w = (w + align_w - 1) & ~(align_w - 1); \
>>>>> +    int aligned_h = (h + align_h - 1) & ~(align_h - 1); \
>>>> 
>>>> this can overflow
>>>> feel free to fix in a seperate patch
>>> 
>>> Feel free to propose a patch for how you'd prefer to have it fixed then... 
>>> I
>>> don't see this as a real world problem - w and h are bounded by the tests
>>> themselves, and likewise the alignments - I don't see us having tests 
>>> using
>>> buffers with a width near INT32_MAX?
>> 
>> maybe but then
>> if we want our asm code to handle such extrem cases, something needs to
>> test it
>> 
>> ill send a patch based on this: (once your patches are in / minus in case
>> i forget)
>> 
>> int64_t aligned_w = (w - 1LL + align_w) & ~(align_w - 1); \
>> if (aligned_w != (int32_t)aligned_w)
>>    return AVERROR(EINVAL)
>
> Ok, if you find that important.
>
> I'll go ahead and push patches 1-2 and 4 from this set, soon, holding off of 
> patch 3/4 while waiting for someone to fix the Loongarch HEVC SIMD.

Pushed.

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

[FFmpeg-devel] [PATCH 3/4] checkasm: hevc_pel: Use helpers for checking for writes out of bounds

[Martin Storsjö]

This allows catching whether the functions write outside of
the designated rectangle, and if run with "checkasm -v", it also
prints out on which side of the rectangle the overwrite was.
---
 tests/checkasm/hevc_pel.c | 157 ++++++++++++++++++++++----------------
 1 file changed, 90 insertions(+), 67 deletions(-)

diff --git a/tests/checkasm/hevc_pel.c b/tests/checkasm/hevc_pel.c
index b9417832e5..9a83613915 100644
--- a/tests/checkasm/hevc_pel.c
+++ b/tests/checkasm/hevc_pel.c
@@ -44,9 +44,14 @@ static const int offsets[] = {0, 255, -1 };
             uint32_t r = rnd() & mask;               \
             AV_WN32A(buf0 + k, r);                   \
             AV_WN32A(buf1 + k, r);                   \
-            if (k >= BUF_SIZE)                       \
-                continue;                            \
-            r = rnd();                               \
+        }                                            \
+    } while (0)
+
+#define randomize_buffers_dst()                      \
+    do {                                             \
+        int k;                                       \
+        for (k = 0; k < BUF_SIZE; k += 4) {          \
+            uint32_t r = rnd();                      \
             AV_WN32A(dst0 + k, r);                   \
             AV_WN32A(dst1 + k, r);                   \
         }                                            \
@@ -100,6 +105,7 @@ static void checkasm_check_hevc_qpel(void)
                                    "put_hevc_%s%d_%d", type, sizes[size], bit_depth)) {
                         int16_t *dstw0 = (int16_t *) dst0, *dstw1 = (int16_t *) dst1;
                         randomize_buffers();
+                        randomize_buffers_dst();
                         call_ref(dstw0, src0, sizes[size] * SIZEOF_PIXEL, sizes[size], i, j, sizes[size]);
                         call_new(dstw1, src1, sizes[size] * SIZEOF_PIXEL, sizes[size], i, j, sizes[size]);
                         checkasm_check(int16_t, dstw0, MAX_PB_SIZE * sizeof(int16_t),
@@ -118,8 +124,8 @@ static void checkasm_check_hevc_qpel_uni(void)
 {
     LOCAL_ALIGNED_32(uint8_t, buf0, [BUF_SIZE + SRC_EXTRA]);
     LOCAL_ALIGNED_32(uint8_t, buf1, [BUF_SIZE + SRC_EXTRA]);
-    LOCAL_ALIGNED_32(uint8_t, dst0, [BUF_SIZE]);
-    LOCAL_ALIGNED_32(uint8_t, dst1, [BUF_SIZE]);
+    PIXEL_RECT(dst0, 64, 64);
+    PIXEL_RECT(dst1, 64, 64);
 
     HEVCDSPContext h;
     int size, bit_depth, i, j;
@@ -143,16 +149,18 @@ static void checkasm_check_hevc_qpel_uni(void)
                     if (check_func(h.put_hevc_qpel_uni[size][j][i],
                                    "put_hevc_%s%d_%d", type, sizes[size], bit_depth)) {
                         randomize_buffers();
-                        call_ref(dst0, sizes[size] * SIZEOF_PIXEL,
+                        CLEAR_PIXEL_RECT(dst0);
+                        CLEAR_PIXEL_RECT(dst1);
+                        call_ref(dst0, dst0_stride,
                                  src0, sizes[size] * SIZEOF_PIXEL,
                                  sizes[size], i, j, sizes[size]);
-                        call_new(dst1, sizes[size] * SIZEOF_PIXEL,
+                        call_new(dst1, dst1_stride,
                                  src1, sizes[size] * SIZEOF_PIXEL,
                                  sizes[size], i, j, sizes[size]);
-                        checkasm_check_pixel(dst0, sizes[size] * SIZEOF_PIXEL,
-                                             dst1, sizes[size] * SIZEOF_PIXEL,
-                                             size[sizes], size[sizes], "dst");
-                        bench_new(dst1, sizes[size] * SIZEOF_PIXEL,
+                        checkasm_check_pixel_padded(dst0, dst0_stride,
+                                                    dst1, dst1_stride,
+                                                    size[sizes], size[sizes], "dst");
+                        bench_new(dst1, dst1_stride,
                                   src1, sizes[size] * SIZEOF_PIXEL,
                                   sizes[size], i, j, sizes[size]);
                     }
@@ -167,8 +175,8 @@ static void checkasm_check_hevc_qpel_uni_w(void)
 {
     LOCAL_ALIGNED_32(uint8_t, buf0, [BUF_SIZE + SRC_EXTRA]);
     LOCAL_ALIGNED_32(uint8_t, buf1, [BUF_SIZE + SRC_EXTRA]);
-    LOCAL_ALIGNED_32(uint8_t, dst0, [BUF_SIZE]);
-    LOCAL_ALIGNED_32(uint8_t, dst1, [BUF_SIZE]);
+    PIXEL_RECT(dst0, 64, 64);
+    PIXEL_RECT(dst1, 64, 64);
 
     HEVCDSPContext h;
     int size, bit_depth, i, j;
@@ -196,16 +204,18 @@ static void checkasm_check_hevc_qpel_uni_w(void)
                             for (wx = weights; *wx >= 0; wx++) {
                                 for (ox = offsets; *ox >= 0; ox++) {
                                     randomize_buffers();
-                                    call_ref(dst0, sizes[size] * SIZEOF_PIXEL,
+                                    CLEAR_PIXEL_RECT(dst0);
+                                    CLEAR_PIXEL_RECT(dst1);
+                                    call_ref(dst0, dst0_stride,
                                              src0, sizes[size] * SIZEOF_PIXEL,
                                              sizes[size], *denom, *wx, *ox, i, j, sizes[size]);
-                                    call_new(dst1, sizes[size] * SIZEOF_PIXEL,
+                                    call_new(dst1, dst1_stride,
                                              src1, sizes[size] * SIZEOF_PIXEL,
                                              sizes[size], *denom, *wx, *ox, i, j, sizes[size]);
-                                    checkasm_check_pixel(dst0, sizes[size] * SIZEOF_PIXEL,
-                                                         dst1, sizes[size] * SIZEOF_PIXEL,
-                                                         size[sizes], size[sizes], "dst");
-                                    bench_new(dst1, sizes[size] * SIZEOF_PIXEL,
+                                    checkasm_check_pixel_padded(dst0, dst0_stride,
+                                                                dst1, dst1_stride,
+                                                                size[sizes], size[sizes], "dst");
+                                    bench_new(dst1, dst1_stride,
                                               src1, sizes[size] * SIZEOF_PIXEL,
                                               sizes[size], *denom, *wx, *ox, i, j, sizes[size]);
                                 }
@@ -223,8 +233,8 @@ static void checkasm_check_hevc_qpel_bi(void)
 {
     LOCAL_ALIGNED_32(uint8_t, buf0, [BUF_SIZE + SRC_EXTRA]);
     LOCAL_ALIGNED_32(uint8_t, buf1, [BUF_SIZE + SRC_EXTRA]);
-    LOCAL_ALIGNED_32(uint8_t, dst0, [BUF_SIZE]);
-    LOCAL_ALIGNED_32(uint8_t, dst1, [BUF_SIZE]);
+    PIXEL_RECT(dst0, 64, 64);
+    PIXEL_RECT(dst1, 64, 64);
     LOCAL_ALIGNED_32(int16_t, ref0, [BUF_SIZE]);
     LOCAL_ALIGNED_32(int16_t, ref1, [BUF_SIZE]);
 
@@ -251,16 +261,18 @@ static void checkasm_check_hevc_qpel_bi(void)
                     if (check_func(h.put_hevc_qpel_bi[size][j][i],
                                    "put_hevc_%s%d_%d", type, sizes[size], bit_depth)) {
                         randomize_buffers_ref();
-                        call_ref(dst0, sizes[size] * SIZEOF_PIXEL,
+                        CLEAR_PIXEL_RECT(dst0);
+                        CLEAR_PIXEL_RECT(dst1);
+                        call_ref(dst0, dst0_stride,
                                  src0, sizes[size] * SIZEOF_PIXEL,
                                  ref0, sizes[size], i, j, sizes[size]);
-                        call_new(dst1, sizes[size] * SIZEOF_PIXEL,
+                        call_new(dst1, dst1_stride,
                                  src1, sizes[size] * SIZEOF_PIXEL,
                                  ref1, sizes[size], i, j, sizes[size]);
-                        checkasm_check_pixel(dst0, sizes[size] * SIZEOF_PIXEL,
-                                             dst1, sizes[size] * SIZEOF_PIXEL,
-                                             size[sizes], size[sizes], "dst");
-                        bench_new(dst1, sizes[size] * SIZEOF_PIXEL,
+                        checkasm_check_pixel_padded(dst0, dst0_stride,
+                                                    dst1, dst1_stride,
+                                                    size[sizes], size[sizes], "dst");
+                        bench_new(dst1, dst1_stride,
                                   src1, sizes[size] * SIZEOF_PIXEL,
                                   ref1, sizes[size], i, j, sizes[size]);
                     }
@@ -275,8 +287,8 @@ static void checkasm_check_hevc_qpel_bi_w(void)
 {
     LOCAL_ALIGNED_32(uint8_t, buf0, [BUF_SIZE + SRC_EXTRA]);
     LOCAL_ALIGNED_32(uint8_t, buf1, [BUF_SIZE + SRC_EXTRA]);
-    LOCAL_ALIGNED_32(uint8_t, dst0, [BUF_SIZE]);
-    LOCAL_ALIGNED_32(uint8_t, dst1, [BUF_SIZE]);
+    PIXEL_RECT(dst0, 64, 64);
+    PIXEL_RECT(dst1, 64, 64);
     LOCAL_ALIGNED_32(int16_t, ref0, [BUF_SIZE]);
     LOCAL_ALIGNED_32(int16_t, ref1, [BUF_SIZE]);
 
@@ -308,16 +320,18 @@ static void checkasm_check_hevc_qpel_bi_w(void)
                             for (wx = weights; *wx >= 0; wx++) {
                                 for (ox = offsets; *ox >= 0; ox++) {
                                     randomize_buffers_ref();
-                                    call_ref(dst0, sizes[size] * SIZEOF_PIXEL,
+                                    CLEAR_PIXEL_RECT(dst0);
+                                    CLEAR_PIXEL_RECT(dst1);
+                                    call_ref(dst0, dst0_stride,
                                              src0, sizes[size] * SIZEOF_PIXEL,
                                              ref0, sizes[size], *denom, *wx, *wx, *ox, *ox, i, j, sizes[size]);
-                                    call_new(dst1, sizes[size] * SIZEOF_PIXEL,
+                                    call_new(dst1, dst1_stride,
                                              src1, sizes[size] * SIZEOF_PIXEL,
                                              ref1, sizes[size], *denom, *wx, *wx, *ox, *ox, i, j, sizes[size]);
-                                    checkasm_check_pixel(dst0, sizes[size] * SIZEOF_PIXEL,
-                                                         dst1, sizes[size] * SIZEOF_PIXEL,
-                                                         size[sizes], size[sizes], "dst");
-                                    bench_new(dst1, sizes[size] * SIZEOF_PIXEL,
+                                    checkasm_check_pixel_padded(dst0, dst0_stride,
+                                                                dst1, dst1_stride,
+                                                                size[sizes], size[sizes], "dst");
+                                    bench_new(dst1, dst1_stride,
                                               src1, sizes[size] * SIZEOF_PIXEL,
                                               ref1, sizes[size], *denom, *wx, *wx, *ox, *ox, i, j, sizes[size]);
                                 }
@@ -364,6 +378,7 @@ static void checkasm_check_hevc_epel(void)
                                    "put_hevc_%s%d_%d", type, sizes[size], bit_depth)) {
                         int16_t *dstw0 = (int16_t *) dst0, *dstw1 = (int16_t *) dst1;
                         randomize_buffers();
+                        randomize_buffers_dst();
                         call_ref(dstw0, src0, sizes[size] * SIZEOF_PIXEL, sizes[size], i, j, sizes[size]);
                         call_new(dstw1, src1, sizes[size] * SIZEOF_PIXEL, sizes[size], i, j, sizes[size]);
                         checkasm_check(int16_t, dstw0, MAX_PB_SIZE * sizeof(int16_t),
@@ -382,8 +397,8 @@ static void checkasm_check_hevc_epel_uni(void)
 {
     LOCAL_ALIGNED_32(uint8_t, buf0, [BUF_SIZE]);
     LOCAL_ALIGNED_32(uint8_t, buf1, [BUF_SIZE]);
-    LOCAL_ALIGNED_32(uint8_t, dst0, [BUF_SIZE]);
-    LOCAL_ALIGNED_32(uint8_t, dst1, [BUF_SIZE]);
+    PIXEL_RECT(dst0, 64, 64);
+    PIXEL_RECT(dst1, 64, 64);
 
     HEVCDSPContext h;
     int size, bit_depth, i, j;
@@ -407,16 +422,18 @@ static void checkasm_check_hevc_epel_uni(void)
                     if (check_func(h.put_hevc_epel_uni[size][j][i],
                                    "put_hevc_%s%d_%d", type, sizes[size], bit_depth)) {
                         randomize_buffers();
-                        call_ref(dst0, sizes[size] * SIZEOF_PIXEL,
+                        CLEAR_PIXEL_RECT(dst0);
+                        CLEAR_PIXEL_RECT(dst1);
+                        call_ref(dst0, dst0_stride,
                                  src0, sizes[size] * SIZEOF_PIXEL,
                                  sizes[size], i, j, sizes[size]);
-                        call_new(dst1, sizes[size] * SIZEOF_PIXEL,
+                        call_new(dst1, dst1_stride,
                                  src1, sizes[size] * SIZEOF_PIXEL,
                                  sizes[size], i, j, sizes[size]);
-                        checkasm_check_pixel(dst0, sizes[size] * SIZEOF_PIXEL,
-                                             dst1, sizes[size] * SIZEOF_PIXEL,
-                                             size[sizes], size[sizes], "dst");
-                        bench_new(dst1, sizes[size] * SIZEOF_PIXEL,
+                        checkasm_check_pixel_padded(dst0, dst0_stride,
+                                                    dst1, dst1_stride,
+                                                    size[sizes], size[sizes], "dst");
+                        bench_new(dst1, dst1_stride,
                                   src1, sizes[size] * SIZEOF_PIXEL,
                                   sizes[size], i, j, sizes[size]);
                     }
@@ -431,8 +448,8 @@ static void checkasm_check_hevc_epel_uni_w(void)
 {
     LOCAL_ALIGNED_32(uint8_t, buf0, [BUF_SIZE]);
     LOCAL_ALIGNED_32(uint8_t, buf1, [BUF_SIZE]);
-    LOCAL_ALIGNED_32(uint8_t, dst0, [BUF_SIZE]);
-    LOCAL_ALIGNED_32(uint8_t, dst1, [BUF_SIZE]);
+    PIXEL_RECT(dst0, 64, 64);
+    PIXEL_RECT(dst1, 64, 64);
 
     HEVCDSPContext h;
     int size, bit_depth, i, j;
@@ -460,16 +477,18 @@ static void checkasm_check_hevc_epel_uni_w(void)
                             for (wx = weights; *wx >= 0; wx++) {
                                 for (ox = offsets; *ox >= 0; ox++) {
                                     randomize_buffers();
-                                    call_ref(dst0, sizes[size] * SIZEOF_PIXEL,
+                                    CLEAR_PIXEL_RECT(dst0);
+                                    CLEAR_PIXEL_RECT(dst1);
+                                    call_ref(dst0, dst0_stride,
                                              src0, sizes[size] * SIZEOF_PIXEL,
                                              sizes[size], *denom, *wx, *ox, i, j, sizes[size]);
-                                    call_new(dst1, sizes[size] * SIZEOF_PIXEL,
+                                    call_new(dst1, dst1_stride,
                                              src1, sizes[size] * SIZEOF_PIXEL,
                                              sizes[size], *denom, *wx, *ox, i, j, sizes[size]);
-                                    checkasm_check_pixel(dst0, sizes[size] * SIZEOF_PIXEL,
-                                                         dst1, sizes[size] * SIZEOF_PIXEL,
-                                                         size[sizes], size[sizes], "dst");
-                                    bench_new(dst1, sizes[size] * SIZEOF_PIXEL,
+                                    checkasm_check_pixel_padded(dst0, dst0_stride,
+                                                                dst1, dst1_stride,
+                                                                size[sizes], size[sizes], "dst");
+                                    bench_new(dst1, dst1_stride,
                                               src1, sizes[size] * SIZEOF_PIXEL,
                                               sizes[size], *denom, *wx, *ox, i, j, sizes[size]);
                                 }
@@ -487,8 +506,8 @@ static void checkasm_check_hevc_epel_bi(void)
 {
     LOCAL_ALIGNED_32(uint8_t, buf0, [BUF_SIZE]);
     LOCAL_ALIGNED_32(uint8_t, buf1, [BUF_SIZE]);
-    LOCAL_ALIGNED_32(uint8_t, dst0, [BUF_SIZE]);
-    LOCAL_ALIGNED_32(uint8_t, dst1, [BUF_SIZE]);
+    PIXEL_RECT(dst0, 64, 64);
+    PIXEL_RECT(dst1, 64, 64);
     LOCAL_ALIGNED_32(int16_t, ref0, [BUF_SIZE]);
     LOCAL_ALIGNED_32(int16_t, ref1, [BUF_SIZE]);
 
@@ -515,16 +534,18 @@ static void checkasm_check_hevc_epel_bi(void)
                     if (check_func(h.put_hevc_epel_bi[size][j][i],
                                    "put_hevc_%s%d_%d", type, sizes[size], bit_depth)) {
                         randomize_buffers_ref();
-                        call_ref(dst0, sizes[size] * SIZEOF_PIXEL,
+                        CLEAR_PIXEL_RECT(dst0);
+                        CLEAR_PIXEL_RECT(dst1);
+                        call_ref(dst0, dst0_stride,
                                  src0, sizes[size] * SIZEOF_PIXEL,
                                  ref0, sizes[size], i, j, sizes[size]);
-                        call_new(dst1, sizes[size] * SIZEOF_PIXEL,
+                        call_new(dst1, dst1_stride,
                                  src1, sizes[size] * SIZEOF_PIXEL,
                                  ref1, sizes[size], i, j, sizes[size]);
-                        checkasm_check_pixel(dst0, sizes[size] * SIZEOF_PIXEL,
-                                             dst1, sizes[size] * SIZEOF_PIXEL,
-                                             size[sizes], size[sizes], "dst");
-                        bench_new(dst1, sizes[size] * SIZEOF_PIXEL,
+                        checkasm_check_pixel_padded(dst0, dst0_stride,
+                                                    dst1, dst1_stride,
+                                                    size[sizes], size[sizes], "dst");
+                        bench_new(dst1, dst1_stride,
                                   src1, sizes[size] * SIZEOF_PIXEL,
                                   ref1, sizes[size], i, j, sizes[size]);
                     }
@@ -539,8 +560,8 @@ static void checkasm_check_hevc_epel_bi_w(void)
 {
     LOCAL_ALIGNED_32(uint8_t, buf0, [BUF_SIZE]);
     LOCAL_ALIGNED_32(uint8_t, buf1, [BUF_SIZE]);
-    LOCAL_ALIGNED_32(uint8_t, dst0, [BUF_SIZE]);
-    LOCAL_ALIGNED_32(uint8_t, dst1, [BUF_SIZE]);
+    PIXEL_RECT(dst0, 64, 64);
+    PIXEL_RECT(dst1, 64, 64);
     LOCAL_ALIGNED_32(int16_t, ref0, [BUF_SIZE]);
     LOCAL_ALIGNED_32(int16_t, ref1, [BUF_SIZE]);
 
@@ -572,16 +593,18 @@ static void checkasm_check_hevc_epel_bi_w(void)
                             for (wx = weights; *wx >= 0; wx++) {
                                 for (ox = offsets; *ox >= 0; ox++) {
                                     randomize_buffers_ref();
-                                    call_ref(dst0, sizes[size] * SIZEOF_PIXEL,
+                                    CLEAR_PIXEL_RECT(dst0);
+                                    CLEAR_PIXEL_RECT(dst1);
+                                    call_ref(dst0, dst0_stride,
                                              src0, sizes[size] * SIZEOF_PIXEL,
                                              ref0, sizes[size], *denom, *wx, *wx, *ox, *ox, i, j, sizes[size]);
-                                    call_new(dst1, sizes[size] * SIZEOF_PIXEL,
+                                    call_new(dst1, dst1_stride,
                                              src1, sizes[size] * SIZEOF_PIXEL,
                                              ref1, sizes[size], *denom, *wx, *wx, *ox, *ox, i, j, sizes[size]);
-                                    checkasm_check_pixel(dst0, sizes[size] * SIZEOF_PIXEL,
-                                                         dst1, sizes[size] * SIZEOF_PIXEL,
-                                                         size[sizes], size[sizes], "dst");
-                                    bench_new(dst1, sizes[size] * SIZEOF_PIXEL,
+                                    checkasm_check_pixel_padded(dst0, dst0_stride,
+                                                                dst1, dst1_stride,
+                                                                size[sizes], size[sizes], "dst");
+                                    bench_new(dst1, dst1_stride,
                                               src1, sizes[size] * SIZEOF_PIXEL,
                                               ref1, sizes[size], *denom, *wx, *wx, *ox, *ox, i, j, sizes[size]);
                                 }
-- 
2.39.5 (Apple Git-154)

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

Re: [FFmpeg-devel] [PATCH 3/4] checkasm: hevc_pel: Use helpers for checking for writes out of bounds

[Martin Storsjö]

On Wed, 26 Mar 2025, Martin Storsjö wrote:

> This allows catching whether the functions write outside of
> the designated rectangle, and if run with "checkasm -v", it also
> prints out on which side of the rectangle the overwrite was.
> ---
> tests/checkasm/hevc_pel.c | 157 ++++++++++++++++++++++----------------
> 1 file changed, 90 insertions(+), 67 deletions(-)

This patch exposes existing issues in the Loongson HEVC simd; running with 
"checkasm --test=hevc_pel -v" after applying these patches, shows the 
following errors:

LSX:
  - hevc_pel.qpel       [OK]
  - hevc_pel.qpel_uni   [OK]
  - hevc_pel.qpel_uni_w [OK]
    put_hevc_pel_bi_pixels12_8_lsx (src/tests/checkasm/hevc_pel.c:272)
dst (12x12):
  overwrite below
  - hevc_pel.qpel_bi    [FAILED]
  - hevc_pel.epel       [OK]
    put_hevc_epel_uni_v24_8_lsx (src/tests/checkasm/hevc_pel.c:433)
dst (24x24):
  overwrite below
    put_hevc_epel_uni_hv12_8_lsx (src/tests/checkasm/hevc_pel.c:433)
dst (12x12):
  overwrite below
  - hevc_pel.epel_uni   [FAILED]
  - hevc_pel.epel_uni_w [OK]
    put_hevc_epel_bi_hv6_8_lsx (src/tests/checkasm/hevc_pel.c:545)
dst (6x6):
  overwrite below
  - hevc_pel.epel_bi    [FAILED]
LASX:
  - hevc_pel.qpel_uni   [OK]
  - hevc_pel.qpel_uni_w [OK]
  - hevc_pel.epel_uni_w [OK]
  - hevc_pel.epel_bi    [OK]
checkasm: 4 of 231 tests have failed


So the functions seem to produce more output rows than requested, for 
"uneven" heights like 6, 12 or 24 pixels.

Can someone from Loongson please have a look at this?

We don't seem to have any Loongson instances on fate, so can I go ahead 
and push this, without waiting for the Loongson assembly to be fixed? We 
do have some on patchwork though, so I guess that'll make patchwork noisy 
for everybody else until it is fixed, so I guess I'll have to hold off of 
this patch until it is fixed.

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

Re: [FFmpeg-devel] [PATCH 3/4] checkasm: hevc_pel: Use helpers for checking for writes out of bounds

[yinshiyou-hf]

> -----原始邮件-----
> 发件人: "Martin Storsjö" <martin@martin.st>
> 发送时间:2025-04-01 17:35:16 (星期二)
> 收件人: ffmpeg-devel@ffmpeg.org
> 抄送: jinbo <jinbo@loongson.cn>, yinshiyou-hf@loongson.cn, "Lu Wang" <wanglu@loongson.cn>
> 主题: Re: [PATCH 3/4] checkasm: hevc_pel: Use helpers for checking for writes out of bounds
> 
> On Wed, 26 Mar 2025, Martin Storsjö wrote:
> 
> > This allows catching whether the functions write outside of
> > the designated rectangle, and if run with "checkasm -v", it also
> > prints out on which side of the rectangle the overwrite was.
> > ---
> > tests/checkasm/hevc_pel.c | 157 ++++++++++++++++++++++----------------
> > 1 file changed, 90 insertions(+), 67 deletions(-)
> 
> This patch exposes existing issues in the Loongson HEVC simd; running with 
> "checkasm --test=hevc_pel -v" after applying these patches, shows the 
> following errors:
> 
> LSX:
>   - hevc_pel.qpel       [OK]
>   - hevc_pel.qpel_uni   [OK]
>   - hevc_pel.qpel_uni_w [OK]
>     put_hevc_pel_bi_pixels12_8_lsx (src/tests/checkasm/hevc_pel.c:272)
> dst (12x12):
>   overwrite below
>   - hevc_pel.qpel_bi    [FAILED]
>   - hevc_pel.epel       [OK]
>     put_hevc_epel_uni_v24_8_lsx (src/tests/checkasm/hevc_pel.c:433)
> dst (24x24):
>   overwrite below
>     put_hevc_epel_uni_hv12_8_lsx (src/tests/checkasm/hevc_pel.c:433)
> dst (12x12):
>   overwrite below
>   - hevc_pel.epel_uni   [FAILED]
>   - hevc_pel.epel_uni_w [OK]
>     put_hevc_epel_bi_hv6_8_lsx (src/tests/checkasm/hevc_pel.c:545)
> dst (6x6):
>   overwrite below
>   - hevc_pel.epel_bi    [FAILED]
> LASX:
>   - hevc_pel.qpel_uni   [OK]
>   - hevc_pel.qpel_uni_w [OK]
>   - hevc_pel.epel_uni_w [OK]
>   - hevc_pel.epel_bi    [OK]
> checkasm: 4 of 231 tests have failed
> 
> 
> So the functions seem to produce more output rows than requested, for 
> "uneven" heights like 6, 12 or 24 pixels.
> 
> Can someone from Loongson please have a look at this?
> 
> We don't seem to have any Loongson instances on fate, so can I go ahead 
> and push this, without waiting for the Loongson assembly to be fixed? We 
> do have some on patchwork though, so I guess that'll make patchwork noisy 
> for everybody else until it is fixed, so I guess I'll have to hold off of 
> this patch until it is fixed.
> 
> // Martin

OK，go head.
I will try to fix it in few days.

本邮件及其附件含有龙芯中科的商业秘密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制或散发）本邮件及其附件中的信息。如果您错收本邮件，请您立即电话或邮件通知发件人并删除本邮件。 
This email and its attachments contain confidential information from Loongson Technology , which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this email in error, please notify the sender by phone or email immediately and delete it. 


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

[FFmpeg-devel] [PATCH 4/4] checkasm: vp8dsp: Use checkasm_check_padded in check_mc

[Martin Storsjö]

---
This is mostly a small example for using the versions of the
helpers that don't rely on a "bit_depth" local variable.
---
 tests/checkasm/vp8dsp.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/tests/checkasm/vp8dsp.c b/tests/checkasm/vp8dsp.c
index e448fe89fc..a12c295a2a 100644
--- a/tests/checkasm/vp8dsp.c
+++ b/tests/checkasm/vp8dsp.c
@@ -259,8 +259,8 @@ static void check_luma_dc_wht(VP8DSPContext *d, bool is_vp7)
 static void check_mc(VP8DSPContext *d)
 {
     LOCAL_ALIGNED_16(uint8_t, buf, [32 * 32]);
-    LOCAL_ALIGNED_16(uint8_t, dst0, [16 * 16]);
-    LOCAL_ALIGNED_16(uint8_t, dst1, [16 * 16]);
+    BUF_RECT(uint8_t, dst0, 16, 16);
+    BUF_RECT(uint8_t, dst1, 16, 16);
     int type, k, dx, dy;
     declare_func_emms(AV_CPU_FLAG_MMX, void, uint8_t *, ptrdiff_t,
                       const uint8_t *, ptrdiff_t, int, int, int);
@@ -305,10 +305,11 @@ static void check_mc(VP8DSPContext *d)
                             src[i                 ] = val;
                             src[i * SRC_BUF_STRIDE] = val;
                         }
-                        call_ref(dst0, size, src, SRC_BUF_STRIDE, height, mx, my);
-                        call_new(dst1, size, src, SRC_BUF_STRIDE, height, mx, my);
-                        if (memcmp(dst0, dst1, size * height))
-                            fail();
+                        CLEAR_BUF_RECT(dst0);
+                        CLEAR_BUF_RECT(dst1);
+                        call_ref(dst0, dst0_stride, src, SRC_BUF_STRIDE, height, mx, my);
+                        call_new(dst1, dst1_stride, src, SRC_BUF_STRIDE, height, mx, my);
+                        checkasm_check_padded(uint8_t, dst0, dst0_stride, dst1, dst1_stride, size, height, "dst");
                         bench_new(dst1, size, src, SRC_BUF_STRIDE, height, mx, my);
                     }
                 }
-- 
2.39.5 (Apple Git-154)

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