On Fri, 25 Mar 2022, Ben Avison wrote:

void ff_vc1dsp_init(VC1DSPContext* c);
diff --git a/tests/checkasm/vc1dsp.c b/tests/checkasm/vc1dsp.c
index 0823ccad31..0ab5892403 100644
--- a/tests/checkasm/vc1dsp.c
+++ b/tests/checkasm/vc1dsp.c
@@ -286,6 +286,20 @@ static matrix 
*generate_inverse_quantized_transform_coefficients(size_t width, s
        }                                                                   \
    } while (0)

+#define TEST_UNESCAPE                                                          
                         \
+    do {                                                                       
                     \
+        for (int count = 100; count > 0; --count) {                            
                     \
+            escaped_offset = rnd() & 7;                                        
                     \
+            unescaped_offset = rnd() & 7;                                      
                     \
+            escaped_len = (1u << (rnd() % 8) + 3) - (rnd() & 7);               
                     \
+            RANDOMIZE_BUFFER8(unescaped, UNESCAPE_BUF_SIZE);                   
                     \

The output buffer will be overwritten in the end, but I guess this initialization is useful for making sure that the test doesn't accidentally rely on the output from the previous iteration, right?

+            len0 = call_ref(escaped0 + escaped_offset, escaped_len, unescaped0 
+ unescaped_offset); \
+            len1 = call_new(escaped1 + escaped_offset, escaped_len, unescaped1 
+ unescaped_offset); \
+            if (len0 != len1 || memcmp(unescaped0, unescaped1, len0))          
                     \

Don't you need to include unescaped_offset here too? Otherwise you're just checking areas of the buffer that wasn't necessarily written.


+                fail();                                                        
                     \
+        }                                                                      
                     \
+    } while (0)
+

As with the rest of the checkasm tests - please unmacro most things where possible (except for the RANDOMIZE_* macros, those are ok to keep macroed if you want to). And sorry for leading you down a path with a bad example in that respect.

void checkasm_check_vc1dsp(void)
{
    /* Inverse transform input coefficients are stored in a 16-bit buffer
@@ -309,6 +323,14 @@ void checkasm_check_vc1dsp(void)
    LOCAL_ALIGNED_4(uint8_t, filter_buf0, [24 * 24]);
    LOCAL_ALIGNED_4(uint8_t, filter_buf1, [24 * 24]);

+    /* This appears to be a typical length of buffer in use */
+#define LOG2_UNESCAPE_BUF_SIZE 17
+#define UNESCAPE_BUF_SIZE (1u<<LOG2_UNESCAPE_BUF_SIZE)
+    LOCAL_ALIGNED_8(uint8_t, escaped0, [UNESCAPE_BUF_SIZE]);
+    LOCAL_ALIGNED_8(uint8_t, escaped1, [UNESCAPE_BUF_SIZE]);
+    LOCAL_ALIGNED_8(uint8_t, unescaped0, [UNESCAPE_BUF_SIZE]);
+    LOCAL_ALIGNED_8(uint8_t, unescaped1, [UNESCAPE_BUF_SIZE]);
+
    VC1DSPContext h;

    ff_vc1dsp_init(&h);
@@ -349,4 +371,41 @@ void checkasm_check_vc1dsp(void)
    CHECK_LOOP_FILTER(vc1_h_loop_filter16);

    report("loop_filter");
+
+    if (check_func(h.vc1_unescape_buffer, "vc1dsp.vc1_unescape_buffer")) {
+        int len0, len1, escaped_offset, unescaped_offset, escaped_len;
+        declare_func_emms(AV_CPU_FLAG_MMX, int, const uint8_t *, int, uint8_t 
*);
+
+        /* Test data which consists of escapes sequences packed as tightly as 
possible */
+        for (int x = 0; x < UNESCAPE_BUF_SIZE; ++x)
+            escaped1[x] = escaped0[x] = 3 * (x % 3 == 0);
+        TEST_UNESCAPE;
+
+        /* Test random data */
+        RANDOMIZE_BUFFER8(escaped, UNESCAPE_BUF_SIZE);
+        TEST_UNESCAPE;
+
+        /* Test data with escape sequences at random intervals */
+        for (int x = 0; x <= UNESCAPE_BUF_SIZE - 4;) {
+            int gap, gap_msb;
+            escaped1[x+0] = escaped0[x+0] = 0;
+            escaped1[x+1] = escaped0[x+1] = 0;
+            escaped1[x+2] = escaped0[x+2] = 3;
+            escaped1[x+3] = escaped0[x+3] = rnd() & 3;
+            gap_msb = 2u << (rnd() % 8);
+            gap = (rnd() &~ -gap_msb) | gap_msb;
+            x += gap;
+        }
+        TEST_UNESCAPE;
+
+        /* Test data which is known to contain no escape sequences */
+        memset(escaped0, 0xFF, UNESCAPE_BUF_SIZE);
+        memset(escaped1, 0xFF, UNESCAPE_BUF_SIZE);
+        TEST_UNESCAPE;
+
+        /* Benchmark the no-escape-sequences case */
+        bench_new(escaped1, UNESCAPE_BUF_SIZE, unescaped1);
+    }
+
+    report("unescape_buffer");
}

The test looks great otherwise! But please split the code for it into a standalonef unction, e.g. check_unescape(), so the main checkasm_check_vc1dsp() just is a list of calls to check_loopfilter(), check_idct(), check_unescape() etc.

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

Reply via email to