Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Ben Avison <bavison@riscosopen.org>
To: ffmpeg-devel@ffmpeg.org
Cc: Ben Avison <bavison@riscosopen.org>
Subject: [FFmpeg-devel] [PATCH 02/10] checkasm: Add vc1dsp inverse transform tests
Date: Fri, 25 Mar 2022 18:52:49 +0000
Message-ID: <20220325185257.513933-3-bavison@riscosopen.org> (raw)
In-Reply-To: <20220325185257.513933-1-bavison@riscosopen.org>

This test deliberately doesn't exercise the full range of inputs described in
the committee draft VC-1 standard. It says:

input coefficients in frequency domain, D, satisfy   -2048 <= D < 2047
intermediate coefficients, E, satisfy                -4096 <= E < 4095
fully inverse-transformed coefficients, R, satisfy    -512 <= R <  511

For one thing, the inequalities look odd. Did they mean them to go the
other way round? That would make more sense because the equations generally
both add and subtract coefficients multiplied by constants, including powers
of 2. Requiring the most-negative values to be valid extends the number of
bits to represent the intermediate values just for the sake of that one case!

For another thing, the extreme values don't look to occur in real streams -
both in my experience and supported by the following comment in the AArch32
decoder:

    tNhalf is half of the value of tN (as described in vc1_inv_trans_8x8_c).
    This is done because sometimes files have input that causes tN + tM to
    overflow. To avoid this overflow, we compute tNhalf, then compute
    tNhalf + tM (which doesn't overflow), and then we use vhadd to compute
    (tNhalf + (tNhalf + tM)) >> 1 which does not overflow because it is
    one instruction.

My AArch64 decoder goes further than this. It calculates tNhalf and tM
then does an SRA (essentially a fused halve and add) to compute
(tN + tM) >> 1 without ever having to hold (tNhalf + tM) in a 16-bit element
without overflowing. It only encounters difficulties if either tNhalf or
tM overflow in isolation.

I haven't had sight of the final standard, so it's possible that these
issues were dealt with during finalisation, which could explain the lack
of usage of extreme inputs in real streams. Or a preponderance of decoders
that only support 16-bit intermediate values in their inverse transforms
might have caused encoders to steer clear of such cases.

I have effectively followed this approach in the test, and limited the
scale of the coefficients sufficient that both the existing AArch32 decoder
and my new AArch64 decoder both pass.

Signed-off-by: Ben Avison <bavison@riscosopen.org>
---
 tests/checkasm/vc1dsp.c | 258 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 258 insertions(+)

diff --git a/tests/checkasm/vc1dsp.c b/tests/checkasm/vc1dsp.c
index db916d08f9..0823ccad31 100644
--- a/tests/checkasm/vc1dsp.c
+++ b/tests/checkasm/vc1dsp.c
@@ -29,6 +29,200 @@
 #include "libavutil/intreadwrite.h"
 #include "libavutil/mem_internal.h"
 
+typedef struct matrix {
+    size_t width;
+    size_t height;
+    float d[];
+} matrix;
+
+static const matrix T8 = { 8, 8, {
+        12,  12,  12,  12,  12,  12,  12,  12,
+        16,  15,   9,   4,  -4,  -9, -15, -16,
+        16,   6,  -6, -16, -16,  -6,   6,  16,
+        15,  -4, -16,  -9,   9,  16,   4, -15,
+        12, -12, -12,  12,  12, -12, -12,  12,
+         9, -16,   4,  15, -15,  -4,  16,  -9,
+         6, -16,  16,  -6,  -6,  16, -16,   6,
+         4,  -9,  15, -16,  16, -15,   9,  -4
+} };
+
+static const matrix T4 = { 4, 4, {
+        17,  17,  17,  17,
+        22,  10, -10, -22,
+        17, -17, -17,  17,
+        10, -22,  22, -10
+} };
+
+static const matrix T8t = { 8, 8, {
+        12,  16,  16,  15,  12,   9,   6,   4,
+        12,  15,   6,  -4, -12, -16, -16,  -9,
+        12,   9,  -6, -16, -12,   4,  16,  15,
+        12,   4, -16,  -9,  12,  15,  -6, -16,
+        12,  -4, -16,   9,  12, -15,  -6,  16,
+        12,  -9,  -6,  16, -12,  -4,  16, -15,
+        12, -15,   6,   4, -12,  16, -16,   9,
+        12, -16,  16, -15,  12,  -9,   6,  -4
+} };
+
+static const matrix T4t = { 4, 4, {
+        17,  22,  17,  10,
+        17,  10, -17, -22,
+        17, -10, -17,  22,
+        17, -22,  17, -10
+} };
+
+static matrix *new_matrix(size_t width, size_t height)
+{
+    matrix *out = av_mallocz(sizeof (matrix) + height * width * sizeof (float));
+    if (out == NULL) {
+        fprintf(stderr, "Memory allocation failure\n");
+        exit(EXIT_FAILURE);
+    }
+    out->width = width;
+    out->height = height;
+    return out;
+}
+
+static matrix *multiply(const matrix *a, const matrix *b)
+{
+    matrix *out;
+    if (a->width != b->height) {
+        fprintf(stderr, "Incompatible multiplication\n");
+        exit(EXIT_FAILURE);
+    }
+    out = new_matrix(b->width, a->height);
+    for (int j = 0; j < out->height; ++j)
+        for (int i = 0; i < out->width; ++i) {
+            float sum = 0;
+            for (int k = 0; k < a->width; ++k)
+                sum += a->d[j * a->width + k] * b->d[k * b->width + i];
+            out->d[j * out->width + i] = sum;
+        }
+    return out;
+}
+
+static void normalise(matrix *a)
+{
+    for (int j = 0; j < a->height; ++j)
+        for (int i = 0; i < a->width; ++i) {
+            float *p = a->d + j * a->width + i;
+            *p *= 64;
+            if (a->height == 4)
+                *p /= (const unsigned[]) { 289, 292, 289, 292 } [j];
+            else
+                *p /= (const unsigned[]) { 288, 289, 292, 289, 288, 289, 292, 289 } [j];
+            if (a->width == 4)
+                *p /= (const unsigned[]) { 289, 292, 289, 292 } [i];
+            else
+                *p /= (const unsigned[]) { 288, 289, 292, 289, 288, 289, 292, 289 } [i];
+        }
+}
+
+static void divide_and_round_nearest(matrix *a, float by)
+{
+    for (int j = 0; j < a->height; ++j)
+        for (int i = 0; i < a->width; ++i) {
+            float *p = a->d + j * a->width + i;
+            *p = rintf(*p / by);
+        }
+}
+
+static void tweak(matrix *a)
+{
+    for (int j = 4; j < a->height; ++j)
+        for (int i = 0; i < a->width; ++i) {
+            float *p = a->d + j * a->width + i;
+            *p += 1;
+        }
+}
+
+/* The VC-1 spec places restrictions on the values permitted at three
+ * different stages:
+ * - D: the input coefficients in frequency domain
+ * - E: the intermediate coefficients, inverse-transformed only horizontally
+ * - R: the fully inverse-transformed coefficients
+ *
+ * To fully cater for the ranges specified requires various intermediate
+ * values to be held to 17-bit precision; yet these conditions do not appear
+ * to be utilised in real-world streams. At least some assembly
+ * implementations have chosen to restrict these values to 16-bit precision,
+ * to accelerate the decoding of real-world streams at the cost of strict
+ * adherence to the spec. To avoid our test marking these as failures,
+ * reduce our random inputs.
+ */
+#define ATTENUATION 4
+
+static matrix *generate_inverse_quantized_transform_coefficients(size_t width, size_t height)
+{
+    matrix *raw, *tmp, *D, *E, *R;
+    raw = new_matrix(width, height);
+    for (int i = 0; i < width * height; ++i)
+        raw->d[i] = (int) (rnd() % (1024/ATTENUATION)) - 512/ATTENUATION;
+    tmp = multiply(height == 8 ? &T8 : &T4, raw);
+    D = multiply(tmp, width == 8 ? &T8t : &T4t);
+    normalise(D);
+    divide_and_round_nearest(D, 1);
+    for (int i = 0; i < width * height; ++i) {
+        if (D->d[i] < -2048/ATTENUATION || D->d[i] > 2048/ATTENUATION-1) {
+            /* Rare, so simply try again */
+            av_free(raw);
+            av_free(tmp);
+            av_free(D);
+            return generate_inverse_quantized_transform_coefficients(width, height);
+        }
+    }
+    E = multiply(D, width == 8 ? &T8 : &T4);
+    divide_and_round_nearest(E, 8);
+    for (int i = 0; i < width * height; ++i)
+        if (E->d[i] < -4096/ATTENUATION || E->d[i] > 4096/ATTENUATION-1) {
+            /* Rare, so simply try again */
+            av_free(raw);
+            av_free(tmp);
+            av_free(D);
+            av_free(E);
+            return generate_inverse_quantized_transform_coefficients(width, height);
+        }
+    R = multiply(height == 8 ? &T8t : &T4t, E);
+    tweak(R);
+    divide_and_round_nearest(R, 128);
+    for (int i = 0; i < width * height; ++i)
+        if (R->d[i] < -512/ATTENUATION || R->d[i] > 512/ATTENUATION-1) {
+            /* Rare, so simply try again */
+            av_free(raw);
+            av_free(tmp);
+            av_free(D);
+            av_free(E);
+            av_free(R);
+            return generate_inverse_quantized_transform_coefficients(width, height);
+        }
+    av_free(raw);
+    av_free(tmp);
+    av_free(E);
+    av_free(R);
+    return D;
+}
+
+#define RANDOMIZE_BUFFER16(name, size)        \
+    do {                                      \
+        int i;                                \
+        for (i = 0; i < size; ++i) {          \
+            uint16_t r = rnd();               \
+            AV_WN16A(name##0 + i, r);         \
+            AV_WN16A(name##1 + i, r);         \
+        }                                     \
+    } while (0)
+
+#define RANDOMIZE_BUFFER8(name, size)         \
+    do {                                      \
+        int i;                                \
+        for (i = 0; i < size; ++i) {          \
+            uint8_t r = rnd();                \
+            name##0[i] = r;                   \
+            name##1[i] = r;                   \
+        }                                     \
+    } while (0)
+
+
 #define RANDOMIZE_BUFFER8_MID_WEIGHTED(name, size)  \
     do {                                            \
         uint8_t *p##0 = name##0, *p##1 = name##1;   \
@@ -42,6 +236,28 @@
         }                                           \
     } while (0)
 
+#define CHECK_INV_TRANS(func, width, height)                                            \
+    do {                                                                                \
+        if (check_func(h.func, "vc1dsp." #func)) {                                      \
+            matrix *coeffs;                                                             \
+            declare_func_emms(AV_CPU_FLAG_MMX, void, uint8_t *, ptrdiff_t, int16_t *);  \
+            RANDOMIZE_BUFFER16(inv_trans_in, 10 * 8);                                   \
+            RANDOMIZE_BUFFER8(inv_trans_out, 10 * 24);                                  \
+            coeffs = generate_inverse_quantized_transform_coefficients(width, height);  \
+            for (int j = 0; j < height; ++j)                                            \
+                for (int i = 0; i < width; ++i) {                                       \
+                    int idx = 8 + j * 8 + i;                                            \
+                    inv_trans_in1[idx] = inv_trans_in0[idx] = coeffs->d[j * width + i]; \
+                }                                                                       \
+            call_ref(inv_trans_out0 + 24 + 8, 24, inv_trans_in0 + 8);                   \
+            call_new(inv_trans_out1 + 24 + 8, 24, inv_trans_in1 + 8);                   \
+            if (memcmp(inv_trans_out0, inv_trans_out1, 10 * 24))                        \
+                fail();                                                                 \
+            bench_new(inv_trans_out1 + 24 + 8, 24, inv_trans_in1 + 8);                  \
+            av_free(coeffs);                                                            \
+        }                                                                               \
+    } while (0)
+
 #define CHECK_LOOP_FILTER(func)                                             \
     do {                                                                    \
         if (check_func(h.func, "vc1dsp." #func)) {                          \
@@ -72,6 +288,20 @@
 
 void checkasm_check_vc1dsp(void)
 {
+    /* Inverse transform input coefficients are stored in a 16-bit buffer
+     * with row stride of 8 coefficients irrespective of transform size.
+     * vc1_inv_trans_8x8 differs from the others in two ways: coefficients
+     * are stored in column-major order, and the outputs are written back
+     * to the input buffer, so we oversize it slightly to catch overruns. */
+    LOCAL_ALIGNED_16(int16_t, inv_trans_in0, [10 * 8]);
+    LOCAL_ALIGNED_16(int16_t, inv_trans_in1, [10 * 8]);
+
+    /* For all but vc1_inv_trans_8x8, the inverse transform is narrowed and
+     * added with saturation to an array of unsigned 8-bit values. Oversize
+     * this by 8 samples left and right and one row above and below. */
+    LOCAL_ALIGNED_8(uint8_t, inv_trans_out0, [10 * 24]);
+    LOCAL_ALIGNED_8(uint8_t, inv_trans_out1, [10 * 24]);
+
     /* Deblocking filter buffers are big enough to hold a 16x16 block,
      * plus 4 rows/columns above/left to hold filter inputs (depending on
      * whether v or h neighbouring block edge) plus 4 rows/columns
@@ -83,6 +313,34 @@ void checkasm_check_vc1dsp(void)
 
     ff_vc1dsp_init(&h);
 
+    if (check_func(h.vc1_inv_trans_8x8, "vc1dsp.vc1_inv_trans_8x8")) {
+        matrix *coeffs;
+        declare_func_emms(AV_CPU_FLAG_MMX, void, int16_t *);
+        RANDOMIZE_BUFFER16(inv_trans_in, 10 * 8);
+        coeffs = generate_inverse_quantized_transform_coefficients(8, 8);
+        for (int j = 0; j < 8; ++j)
+            for (int i = 0; i < 8; ++i) {
+                int idx = 8 + i * 8 + j;
+                inv_trans_in1[idx] = inv_trans_in0[idx] = coeffs->d[j * 8 + i];
+            }
+        call_ref(inv_trans_in0 + 8);
+        call_new(inv_trans_in1 + 8);
+        if (memcmp(inv_trans_in0,  inv_trans_in1,  10 * 8 * sizeof (int16_t)))
+            fail();
+        bench_new(inv_trans_in1 + 8);
+        av_free(coeffs);
+    }
+
+    CHECK_INV_TRANS(vc1_inv_trans_8x4, 8, 4);
+    CHECK_INV_TRANS(vc1_inv_trans_4x8, 4, 8);
+    CHECK_INV_TRANS(vc1_inv_trans_4x4, 4, 4);
+    CHECK_INV_TRANS(vc1_inv_trans_8x8_dc, 8, 8);
+    CHECK_INV_TRANS(vc1_inv_trans_8x4_dc, 8, 4);
+    CHECK_INV_TRANS(vc1_inv_trans_4x8_dc, 4, 8);
+    CHECK_INV_TRANS(vc1_inv_trans_4x4_dc, 4, 4);
+
+    report("inv_trans");
+
     CHECK_LOOP_FILTER(vc1_v_loop_filter4);
     CHECK_LOOP_FILTER(vc1_h_loop_filter4);
     CHECK_LOOP_FILTER(vc1_v_loop_filter8);
-- 
2.25.1

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

  parent reply	other threads:[~2022-03-25 18:54 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-17 18:58 [FFmpeg-devel] [PATCH 0/6] avcodec/vc1: Arm optimisations Ben Avison
2022-03-17 18:58 ` [FFmpeg-devel] [PATCH 1/6] avcodec/vc1: Arm 64-bit NEON deblocking filter fast paths Ben Avison
2022-03-17 18:58 ` [FFmpeg-devel] [PATCH 2/6] avcodec/vc1: Arm 32-bit " Ben Avison
2022-03-17 18:58 ` [FFmpeg-devel] [PATCH 3/6] avcodec/vc1: Arm 64-bit NEON inverse transform " Ben Avison
2022-03-17 18:58 ` [FFmpeg-devel] [PATCH 4/6] avcodec/idctdsp: Arm 64-bit NEON block add and clamp " Ben Avison
2022-03-17 18:58 ` [FFmpeg-devel] [PATCH 5/6] avcodec/blockdsp: Arm 64-bit NEON block clear " Ben Avison
2022-03-17 18:58 ` [FFmpeg-devel] [PATCH 6/6] avcodec/vc1: Introduce fast path for unescaping bitstream buffer Ben Avison
2022-03-18 19:10   ` Andreas Rheinhardt
2022-03-21 15:51     ` Ben Avison
2022-03-21 20:44       ` Martin Storsjö
2022-03-19 23:06 ` [FFmpeg-devel] [PATCH 0/6] avcodec/vc1: Arm optimisations Martin Storsjö
2022-03-19 23:07   ` Martin Storsjö
2022-03-21 17:37   ` Ben Avison
2022-03-21 22:29     ` Martin Storsjö
2022-03-25 18:52 ` [FFmpeg-devel] [PATCH v2 00/10] " Ben Avison
2022-03-25 18:52   ` [FFmpeg-devel] [PATCH 01/10] checkasm: Add vc1dsp in-loop deblocking filter tests Ben Avison
2022-03-25 22:53     ` Martin Storsjö
2022-03-28 18:28       ` Ben Avison
2022-03-29 11:47         ` Martin Storsjö
2022-03-29 12:24     ` Martin Storsjö
2022-03-29 12:43     ` Martin Storsjö
2022-03-25 18:52   ` Ben Avison [this message]
2022-03-29 12:41     ` [FFmpeg-devel] [PATCH 02/10] checkasm: Add vc1dsp inverse transform tests Martin Storsjö
2022-03-25 18:52   ` [FFmpeg-devel] [PATCH 03/10] checkasm: Add idctdsp add/put-pixels-clamped tests Ben Avison
2022-03-29 13:13     ` Martin Storsjö
2022-03-29 19:56       ` Martin Storsjö
2022-03-29 20:22       ` Ben Avison
2022-03-29 20:30         ` Martin Storsjö
2022-03-25 18:52   ` [FFmpeg-devel] [PATCH 04/10] avcodec/vc1: Introduce fast path for unescaping bitstream buffer Ben Avison
2022-03-29 20:37     ` Martin Storsjö
2022-03-31 13:58       ` Ben Avison
2022-03-31 14:07         ` Martin Storsjö
2022-03-25 18:52   ` [FFmpeg-devel] [PATCH 05/10] avcodec/vc1: Arm 64-bit NEON deblocking filter fast paths Ben Avison
2022-03-30 12:35     ` Martin Storsjö
2022-03-31 15:15       ` Ben Avison
2022-03-31 21:21         ` Martin Storsjö
2022-03-25 18:52   ` [FFmpeg-devel] [PATCH 06/10] avcodec/vc1: Arm 32-bit " Ben Avison
2022-03-25 19:27     ` Lynne
2022-03-25 19:49       ` Martin Storsjö
2022-03-25 19:55         ` Lynne
2022-03-30 12:37     ` Martin Storsjö
2022-03-30 13:03     ` Martin Storsjö
2022-03-25 18:52   ` [FFmpeg-devel] [PATCH 07/10] avcodec/vc1: Arm 64-bit NEON inverse transform " Ben Avison
2022-03-30 13:49     ` Martin Storsjö
2022-03-30 14:01       ` Martin Storsjö
2022-03-31 15:37       ` Ben Avison
2022-03-31 21:32         ` Martin Storsjö
2022-03-25 18:52   ` [FFmpeg-devel] [PATCH 08/10] avcodec/idctdsp: Arm 64-bit NEON block add and clamp " Ben Avison
2022-03-30 14:14     ` Martin Storsjö
2022-03-31 16:47       ` Ben Avison
2022-03-31 21:42         ` Martin Storsjö
2022-03-25 18:52   ` [FFmpeg-devel] [PATCH 09/10] avcodec/vc1: Arm 64-bit NEON unescape fast path Ben Avison
2022-03-30 14:35     ` Martin Storsjö
2022-03-25 18:52   ` [FFmpeg-devel] [PATCH 10/10] avcodec/vc1: Arm 32-bit " Ben Avison
2022-03-30 14:35     ` Martin Storsjö

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220325185257.513933-3-bavison@riscosopen.org \
    --to=bavison@riscosopen.org \
    --cc=ffmpeg-devel@ffmpeg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git