fix(gpu): apply motion_fps_weight exactly once on the CUDA/SYCL/HIP motion3 twins

CHANGELOG.md

@@ -18385,6 +18385,20 @@ VMAF_FEATURE_EXTRACTOR_HIP`; all 8 `test_pic_preallocation` sub-tests pass.
   [ADR-1203](docs/adr/1203-cuda-psnr-hvs-enable-chroma-default.md).
 
 
+- `motion_fps_weight` is now applied exactly once on the CUDA, SYCL and
+  HIP `motion` twins. Their host-side `motion3` post-process re-applied
+  the weight to an input every caller had already weighted and clipped,
+  so `VMAF_integer_feature_motion3_score` carried `motion_fps_weight`
+  **squared** whenever the option was set away from its `1.0` default
+  (`motion2_score` was unaffected, and at the default weight the
+  squaring is invisible — which is why every existing parity test
+  passed). Measured on an RTX 4090 at `motion_fps_weight = 0.6`:
+  `cpu = 14.48987751` vs `cuda = 8.69392654`, a `5.8` absolute drift
+  against a `1e-4` gate. The three `test_<backend>_motion3_parity`
+  tests now pin a non-default weight and assert CPU/GPU parity on the
+  derived `integer_motion3_mfw_0.6` key. See ADR-1216.
+
+
 - `vmafx-tune predict --use-saliency` now wires saliency moments into
   feature extraction via `pkg/saliency.ComputeMap` rather than returning an
   unimplemented error directing users to the retired Python binary (#1272).

changelog.d/fixed/1216-gpu-motion3-fps-weight.md

@@ -0,0 +1,12 @@
+- `motion_fps_weight` is now applied exactly once on the CUDA, SYCL and
+  HIP `motion` twins. Their host-side `motion3` post-process re-applied
+  the weight to an input every caller had already weighted and clipped,
+  so `VMAF_integer_feature_motion3_score` carried `motion_fps_weight`
+  **squared** whenever the option was set away from its `1.0` default
+  (`motion2_score` was unaffected, and at the default weight the
+  squaring is invisible — which is why every existing parity test
+  passed). Measured on an RTX 4090 at `motion_fps_weight = 0.6`:
+  `cpu = 14.48987751` vs `cuda = 8.69392654`, a `5.8` absolute drift
+  against a `1e-4` gate. The three `test_<backend>_motion3_parity`
+  tests now pin a non-default weight and assert CPU/GPU parity on the
+  derived `integer_motion3_mfw_0.6` key. See ADR-1216.

core/src/feature/cuda/AGENTS.md

@@ -342,6 +342,24 @@ HIP / Metal motion twins listed in the Twin-update table below) in the same PR.
   `float_motion_vulkan.c`, `float_motion_hip.c`,
   `float_motion_metal.mm`. PR #863 initially wired this option.
 
+- **`motion_fps_weight` is applied EXACTLY ONCE on the v1
+  `integer_motion_*` twins** (ADR-1216) — the CPU reference
+  (`integer_motion.c`) scales the SAD-derived score by the weight in
+  `extract()`, stores the *weighted* value as `motion_sad_score`, and
+  then blends that already-weighted value into `motion2` / `motion3`
+  in `flush()` without touching the weight again. The GPU twins mirror
+  this: every caller of `motion3_postprocess_{cuda,sycl,hip}()` hands
+  it a value that is already fps-weighted and `motion_max_val`-clipped,
+  so **`motion3_postprocess_*` must not multiply by
+  `motion_fps_weight`**. It did until ADR-1216, squaring the weight in
+  `motion3_score`. Because the default is `1.0` and `1.0² = 1.0`, no
+  default-options parity test can see this — the guard is the
+  `test_{cuda,sycl,hip}_motion3_parity`
+  `test_motion3_fps_weight_applied_once` variant, which pins
+  `motion_fps_weight = 0.6` and reads the derived
+  `integer_motion3_mfw_0.6` key. Keep that variant when touching these
+  twins; deleting it re-opens the blind spot.
+
 - **`integer_motion_v2_*` mirror contract** (ADR-0662) — CPU
   `integer_motion_v2.c::mirror` maps `idx >= size` to
   `2 * size - idx - 2`. The CUDA, SYCL, and Vulkan `motion_v2`

core/src/feature/cuda/integer_motion_cuda.c

@@ -226,8 +226,12 @@ static int extract_force_zero(VmafFeatureExtractor *fex, VmafPicture *ref_pic,
 /* ------------------------------------------------------------------ */
 static double motion3_postprocess_cuda(MotionStateCuda *s, double score2)
 {
-    double const weighted = score2 * s->motion_fps_weight;
-    double const blended = motion_blend(weighted, s->motion_blend_factor, s->motion_blend_offset);
+    /* ``score2`` already carries ``motion_fps_weight`` and the
+     * ``motion_max_val`` clip: every caller applies both before handing the
+     * value over, exactly as the CPU reference does once in extract()
+     * (integer_motion.c:372).  Re-weighting here would square the factor
+     * whenever ``motion_fps_weight != 1.0``.  ADR-1216. */
+    double const blended = motion_blend(score2, s->motion_blend_factor, s->motion_blend_offset);
     double const clipped = MIN(blended, s->motion_max_val);
     double const previous_unaveraged = s->prev_motion3_blended;
     s->prev_motion3_blended = clipped;

core/src/feature/hip/integer_motion_hip.c

@@ -213,8 +213,12 @@ static const VmafOption options[] = {
 /* ------------------------------------------------------------------ */
 static double motion3_postprocess_hip(MotionStateHip *s, double score2)
 {
-    const double weighted = score2 * s->motion_fps_weight;
-    const double blended = motion_blend(weighted, s->motion_blend_factor, s->motion_blend_offset);
+    /* ``score2`` already carries ``motion_fps_weight`` and the
+     * ``motion_max_val`` clip: every caller applies both before handing the
+     * value over, exactly as the CPU reference does once in extract()
+     * (integer_motion.c:372).  Re-weighting here would square the factor
+     * whenever ``motion_fps_weight != 1.0``.  ADR-1216. */
+    const double blended = motion_blend(score2, s->motion_blend_factor, s->motion_blend_offset);
     const double clipped = blended < s->motion_max_val ? blended : s->motion_max_val;
     const double prev_una = s->prev_motion3_blended;
     s->prev_motion3_blended = clipped;

core/src/feature/sycl/AGENTS.md

@@ -219,7 +219,10 @@ HIP / Metal motion twins listed in the Twin-update table above) in the same PR.
   the `motion_fps_weight` option and apply it in `flush()` /
   `collect()` exactly as documented there. Any future change to the
   weight application math must span all motion-family GPU twins in
-  the same PR.
+  the same PR. The v1 `integer_motion_sycl.cpp` twin is covered by the
+  same canonical note's **applied exactly once** clause (ADR-1216):
+  `motion3_postprocess_sycl()` must not re-apply the weight its callers
+  have already applied.
 
 - **VAAPI / dmabuf zero-copy import** — the FFmpeg `libvmaf_sycl`
   filter (`ffmpeg-patches/0005-*.patch`) consumes

core/src/feature/sycl/integer_motion_sycl.cpp

@@ -611,8 +611,12 @@ static int extract_force_zero(VmafFeatureExtractor *fex, VmafPicture *ref, VmafP
 /* ------------------------------------------------------------------ */
 static double motion3_postprocess_sycl(MotionStateSycl *s, double score2)
 {
-    double const weighted = score2 * s->motion_fps_weight;
-    double const blended = motion_blend(weighted, s->motion_blend_factor, s->motion_blend_offset);
+    /* ``score2`` already carries ``motion_fps_weight`` and the
+     * ``motion_max_val`` clip: every caller applies both before handing the
+     * value over, exactly as the CPU reference does once in extract()
+     * (integer_motion.c:372).  Re-weighting here would square the factor
+     * whenever ``motion_fps_weight != 1.0``.  ADR-1216. */
+    double const blended = motion_blend(score2, s->motion_blend_factor, s->motion_blend_offset);
     double const clipped = MIN(blended, s->motion_max_val);
     double const previous_unaveraged = s->prev_motion3_blended;
     s->prev_motion3_blended = clipped;

core/test/test_cuda_motion3_parity.c

@@ -92,7 +92,7 @@ static int fill_fixture(VmafPicture *pic, unsigned frame_idx)
 /* CPU path — run the "motion" extractor for NUM_FRAMES frames.        */
 /* Returns the motion3_score at frame index 1 via *out_score.         */
 /* ------------------------------------------------------------------ */
-static char *run_cpu_motion3(double *out_score)
+static char *run_cpu_motion3(const char *fps_weight, const char *key, double *out_score)
 {
     int err = 0;
 
@@ -101,7 +101,15 @@ static char *run_cpu_motion3(double *out_score)
     err = vmaf_init(&vmaf, cfg);
     mu_assert("CPU: vmaf_init failed", !err);
 
-    err = vmaf_use_feature(vmaf, "motion", NULL);
+    VmafFeatureDictionary *opts = NULL;
+    if (fps_weight) {
+        err = vmaf_feature_dictionary_set(&opts, "motion_fps_weight", fps_weight);
+        mu_assert("CPU: vmaf_feature_dictionary_set(motion_fps_weight) failed", !err);
+    }
+
+    err = vmaf_use_feature(vmaf, "motion", opts);
+    if (err)
+        (void)vmaf_feature_dictionary_free(&opts);
     mu_assert("CPU: vmaf_use_feature(motion) failed", !err);
 
     for (unsigned i = 0; i < NUM_FRAMES; i++) {
@@ -119,7 +127,7 @@ static char *run_cpu_motion3(double *out_score)
     err = vmaf_read_pictures(vmaf, NULL, NULL, 0);
     mu_assert("CPU: vmaf_read_pictures(EOS) failed", !err);
 
-    err = vmaf_feature_score_at_index(vmaf, "VMAF_integer_feature_motion3_score", out_score, 1u);
+    err = vmaf_feature_score_at_index(vmaf, key, out_score, 1u);
     mu_assert("CPU: vmaf_feature_score_at_index(motion3, idx=1) failed", !err);
 
     err = vmaf_close(vmaf);
@@ -132,7 +140,7 @@ static char *run_cpu_motion3(double *out_score)
 /* Returns the motion3_score at frame index 1 via *out_score.         */
 /* Returns a skip sentinel (out_score = NaN) if no CUDA device.      */
 /* ------------------------------------------------------------------ */
-static char *run_cuda_motion3(double *out_score)
+static char *run_cuda_motion3(const char *fps_weight, const char *key, double *out_score)
 {
     *out_score = NAN;
     int err = 0;
@@ -154,7 +162,15 @@ static char *run_cuda_motion3(double *out_score)
     err = vmaf_cuda_import_state(vmaf, cu_state);
     mu_assert("CUDA: vmaf_cuda_import_state failed", !err);
 
-    err = vmaf_use_feature(vmaf, "motion_cuda", NULL);
+    VmafFeatureDictionary *opts = NULL;
+    if (fps_weight) {
+        err = vmaf_feature_dictionary_set(&opts, "motion_fps_weight", fps_weight);
+        mu_assert("CUDA: vmaf_feature_dictionary_set(motion_fps_weight) failed", !err);
+    }
+
+    err = vmaf_use_feature(vmaf, "motion_cuda", opts);
+    if (err)
+        (void)vmaf_feature_dictionary_free(&opts);
     mu_assert("CUDA: vmaf_use_feature(motion_cuda) failed", !err);
 
     for (unsigned i = 0; i < NUM_FRAMES; i++) {
@@ -171,7 +187,7 @@ static char *run_cuda_motion3(double *out_score)
     err = vmaf_read_pictures(vmaf, NULL, NULL, 0);
     mu_assert("CUDA: vmaf_read_pictures(EOS) failed", !err);
 
-    err = vmaf_feature_score_at_index(vmaf, "VMAF_integer_feature_motion3_score", out_score, 1u);
+    err = vmaf_feature_score_at_index(vmaf, key, out_score, 1u);
     mu_assert("CUDA: vmaf_feature_score_at_index(motion3, idx=1) failed", !err);
 
     err = vmaf_close(vmaf);
@@ -190,11 +206,11 @@ static char *test_motion3_cpu_cuda_parity(void)
     double cpu_score = 0.0;
     double cuda_score = NAN;
 
-    char *msg = run_cpu_motion3(&cpu_score);
+    char *msg = run_cpu_motion3(NULL, "VMAF_integer_feature_motion3_score", &cpu_score);
     if (msg)
         return msg;
 
-    msg = run_cuda_motion3(&cuda_score);
+    msg = run_cuda_motion3(NULL, "VMAF_integer_feature_motion3_score", &cuda_score);
     if (msg)
         return msg;
 
@@ -211,8 +227,51 @@ static char *test_motion3_cpu_cuda_parity(void)
     return NULL;
 }
 
+/* ------------------------------------------------------------------ */
+/* ADR-1216 — motion_fps_weight must be applied exactly once.          */
+/*                                                                     */
+/* The CPU reference weights the SAD-derived score a single time in    */
+/* extract() (integer_motion.c:372) and then blends the already-       */
+/* weighted motion2 into motion3 without touching the weight again.    */
+/* The CUDA twin used to re-apply motion_fps_weight inside its host-side */
+/* motion3 post-process, squaring the factor.  With the default        */
+/* weight of 1.0 the squaring is invisible, which is why the parity    */
+/* test above never caught it; this variant pins a non-default weight  */
+/* so the two paths only agree when the weight is applied once.        */
+/* ------------------------------------------------------------------ */
+#define FPS_WEIGHT_VAL "0.6"
+#define FPS_WEIGHT_KEY "integer_motion3_mfw_0.6"
+
+static char *test_motion3_fps_weight_applied_once(void)
+{
+    double cpu_score = 0.0;
+    double gpu_score = NAN;
+
+    char *msg = run_cpu_motion3(FPS_WEIGHT_VAL, FPS_WEIGHT_KEY, &cpu_score);
+    if (msg)
+        return msg;
+
+    msg = run_cuda_motion3(FPS_WEIGHT_VAL, FPS_WEIGHT_KEY, &gpu_score);
+    if (msg)
+        return msg;
+
+    if (isnan(gpu_score))
+        return NULL;
+
+    const double delta = fabs(cpu_score - gpu_score);
+    if (delta > PARITY_TOL) {
+        (void)fprintf(stderr,
+                      "\nmotion3 mfw=%s parity FAIL: cpu=%.8f gpu=%.8f delta=%.2e tol=%.2e\n",
+                      FPS_WEIGHT_VAL, cpu_score, gpu_score, delta, PARITY_TOL);
+    }
+    mu_assert("motion3 with motion_fps_weight != 1.0 drifts from the CPU reference",
+              delta <= PARITY_TOL);
+    return NULL;
+}
+
 char *run_tests(void)
 {
     mu_run_test(test_motion3_cpu_cuda_parity);
+    mu_run_test(test_motion3_fps_weight_applied_once);
     return NULL;
 }

core/test/test_hip_motion3_parity.c

@@ -112,7 +112,7 @@ static int fill_fixture(VmafPicture *pic, unsigned frame_idx)
 /* CPU path — run the "motion" extractor for NUM_FRAMES frames.           */
 /* Returns the motion3_score at frame index 1 via *out_score.            */
 /* ---------------------------------------------------------------------- */
-static char *run_cpu_motion3(double *out_score)
+static char *run_cpu_motion3(const char *fps_weight, const char *key, double *out_score)
 {
     int err = 0;
 
@@ -121,7 +121,15 @@ static char *run_cpu_motion3(double *out_score)
     err = vmaf_init(&vmaf, cfg);
     mu_assert("CPU: vmaf_init failed", !err);
 
-    err = vmaf_use_feature(vmaf, "motion", NULL);
+    VmafFeatureDictionary *opts = NULL;
+    if (fps_weight) {
+        err = vmaf_feature_dictionary_set(&opts, "motion_fps_weight", fps_weight);
+        mu_assert("CPU: vmaf_feature_dictionary_set(motion_fps_weight) failed", !err);
+    }
+
+    err = vmaf_use_feature(vmaf, "motion", opts);
+    if (err)
+        (void)vmaf_feature_dictionary_free(&opts);
     mu_assert("CPU: vmaf_use_feature(motion) failed", !err);
 
     for (unsigned i = 0; i < NUM_FRAMES; i++) {
@@ -139,7 +147,7 @@ static char *run_cpu_motion3(double *out_score)
     err = vmaf_read_pictures(vmaf, NULL, NULL, 0);
     mu_assert("CPU: vmaf_read_pictures(EOS) failed", !err);
 
-    err = vmaf_feature_score_at_index(vmaf, "VMAF_integer_feature_motion3_score", out_score, 1u);
+    err = vmaf_feature_score_at_index(vmaf, key, out_score, 1u);
     mu_assert("CPU: vmaf_feature_score_at_index(motion3, idx=1) failed", !err);
 
     err = vmaf_close(vmaf);
@@ -184,7 +192,7 @@ static char *hip_submit_one_frame(VmafContext *vmaf, unsigned i, int *enosys_ski
 /* Returns the motion3_score at frame index 1 via *out_score.            */
 /* Returns a skip sentinel (out_score = NaN) if no HIP device.           */
 /* ---------------------------------------------------------------------- */
-static char *run_hip_motion3(double *out_score)
+static char *run_hip_motion3(const char *fps_weight, const char *key, double *out_score)
 {
     *out_score = NAN;
     int err = 0;
@@ -206,7 +214,15 @@ static char *run_hip_motion3(double *out_score)
     err = vmaf_hip_import_state(vmaf, hip_state);
     mu_assert("HIP: vmaf_hip_import_state failed", !err);
 
-    err = vmaf_use_feature(vmaf, "motion_hip", NULL);
+    VmafFeatureDictionary *opts = NULL;
+    if (fps_weight) {
+        err = vmaf_feature_dictionary_set(&opts, "motion_fps_weight", fps_weight);
+        mu_assert("HIP: vmaf_feature_dictionary_set(motion_fps_weight) failed", !err);
+    }
+
+    err = vmaf_use_feature(vmaf, "motion_hip", opts);
+    if (err)
+        (void)vmaf_feature_dictionary_free(&opts);
     mu_assert("HIP: vmaf_use_feature(motion_hip) failed", !err);
 
     for (unsigned i = 0; i < NUM_FRAMES; i++) {
@@ -226,7 +242,7 @@ static char *run_hip_motion3(double *out_score)
     err = vmaf_read_pictures(vmaf, NULL, NULL, 0);
     mu_assert("HIP: vmaf_read_pictures(EOS) failed", !err);
 
-    err = vmaf_feature_score_at_index(vmaf, "VMAF_integer_feature_motion3_score", out_score, 1u);
+    err = vmaf_feature_score_at_index(vmaf, key, out_score, 1u);
     mu_assert("HIP: vmaf_feature_score_at_index(motion3, idx=1) failed", !err);
 
     err = vmaf_close(vmaf);
@@ -244,11 +260,11 @@ static char *test_motion3_cpu_hip_parity(void)
     double cpu_score = 0.0;
     double hip_score = NAN;
 
-    char *msg = run_cpu_motion3(&cpu_score);
+    char *msg = run_cpu_motion3(NULL, "VMAF_integer_feature_motion3_score", &cpu_score);
     if (msg)
         return msg;
 
-    msg = run_hip_motion3(&hip_score);
+    msg = run_hip_motion3(NULL, "VMAF_integer_feature_motion3_score", &hip_score);
     if (msg)
         return msg;
 
@@ -265,8 +281,51 @@ static char *test_motion3_cpu_hip_parity(void)
     return NULL;
 }
 
+/* ------------------------------------------------------------------ */
+/* ADR-1216 — motion_fps_weight must be applied exactly once.          */
+/*                                                                     */
+/* The CPU reference weights the SAD-derived score a single time in    */
+/* extract() (integer_motion.c:372) and then blends the already-       */
+/* weighted motion2 into motion3 without touching the weight again.    */
+/* The HIP twin used to re-apply motion_fps_weight inside its host-side */
+/* motion3 post-process, squaring the factor.  With the default        */
+/* weight of 1.0 the squaring is invisible, which is why the parity    */
+/* test above never caught it; this variant pins a non-default weight  */
+/* so the two paths only agree when the weight is applied once.        */
+/* ------------------------------------------------------------------ */
+#define FPS_WEIGHT_VAL "0.6"
+#define FPS_WEIGHT_KEY "integer_motion3_mfw_0.6"
+
+static char *test_motion3_fps_weight_applied_once(void)
+{
+    double cpu_score = 0.0;
+    double gpu_score = NAN;
+
+    char *msg = run_cpu_motion3(FPS_WEIGHT_VAL, FPS_WEIGHT_KEY, &cpu_score);
+    if (msg)
+        return msg;
+
+    msg = run_hip_motion3(FPS_WEIGHT_VAL, FPS_WEIGHT_KEY, &gpu_score);
+    if (msg)
+        return msg;
+
+    if (isnan(gpu_score))
+        return NULL;
+
+    const double delta = fabs(cpu_score - gpu_score);
+    if (delta > PARITY_TOL) {
+        (void)fprintf(stderr,
+                      "\nmotion3 mfw=%s parity FAIL: cpu=%.8f gpu=%.8f delta=%.2e tol=%.2e\n",
+                      FPS_WEIGHT_VAL, cpu_score, gpu_score, delta, PARITY_TOL);
+    }
+    mu_assert("motion3 with motion_fps_weight != 1.0 drifts from the CPU reference",
+              delta <= PARITY_TOL);
+    return NULL;
+}
+
 char *run_tests(void)
 {
     mu_run_test(test_motion3_cpu_hip_parity);
+    mu_run_test(test_motion3_fps_weight_applied_once);
     return NULL;
 }