Sync to Exact Content Framerate: keep vsync within the skew tolerance

A content rate slightly past what the display presents at the chosen
interval - a 60.0988 Hz core at swap interval 2 on a 120 Hz panel, or
60 Hz content on a 59.94 Hz VRR ceiling - is paced by vsync with audio
skewed as it is without VRR, rather than forcing the loop nonblocking.
Only a content rate beyond audio_max_timing_skew keeps its own rate
with vsync off.

The decision is runloop_sync_plan_for() in runloop.h, pinned by a
table and an invariant sweep in samples/runloop/pacing. Fixes #19600.

retroarch.c

@@ -1398,42 +1398,6 @@ static float audio_driver_monitor_adjust_system_rates(
    return inp_sample_rate;
 }
 
-static bool video_driver_monitor_adjust_system_rates(
-      float timing_skew_hz,
-      float video_refresh_rate,
-      bool vrr_runloop_enable,
-      float audio_max_timing_skew,
-      unsigned video_swap_interval,
-      unsigned black_frame_insertion,
-      unsigned shader_subframes,
-      double input_fps)
-{
-   float target_video_sync_rate = timing_skew_hz;
-
-   /* Same concept as for audio driver adjust. */
-   float refresh_ratio                   = target_video_sync_rate/input_fps;
-   unsigned refresh_closest_multiple     = (unsigned)(refresh_ratio + 0.5f);
-   float timing_skew                     = 0.0f;
-
-   if (refresh_closest_multiple > 1)
-      target_video_sync_rate /= (((float)black_frame_insertion + 1.0f) * (float)video_swap_interval * (float)shader_subframes);
-
-   if (!vrr_runloop_enable)
-   {
-      timing_skew         =
-         fabs(1.0f - input_fps / target_video_sync_rate);
-      /* We don't want to adjust pitch too much. If we have extreme cases,
-       * just don't readjust at all. */
-      if (timing_skew <= audio_max_timing_skew)
-         return true;
-      RARCH_LOG("[Video] Timings deviate too much. Will not adjust."
-            " (Target = %.2f Hz, Game = %.2f Hz)\n",
-            target_video_sync_rate,
-            (float)input_fps);
-   }
-   return input_fps <= target_video_sync_rate;
-}
-
 static void driver_adjust_system_rates(
       runloop_state_t *runloop_st,
       video_driver_state_t *video_st,
@@ -1448,16 +1412,42 @@ static void driver_adjust_system_rates(
    unsigned shader_subframes              = settings->uints.video_shader_subframes;
    bool vrr_runloop_enable                = settings->bools.vrr_runloop_enable;
    bool video_adaptive_vsync              = settings->bools.video_adaptive_vsync;
+   unsigned sync_plan                     = RUNLOOP_SYNC_VSYNC_HOLDS
+         | (vrr_runloop_enable ? RUNLOOP_SYNC_EXACT_RATE : 0);
 
    /* Update video swap interval if automatic
     * switching is enabled */
    runloop_set_video_swap_interval(settings);
    video_swap_interval = runloop_get_video_swap_interval(video_swap_interval);
 
+   if (input_fps > 0.0)
+   {
+      float timing_skew_hz          = video_refresh_rate;
+
+      if (retro_atomic_load_acquire_int(&video_st->crt_switching_active))
+         timing_skew_hz             = input_fps;
+      video_st->core_hz             = input_fps;
+
+      sync_plan = runloop_sync_plan_for(timing_skew_hz, (float)input_fps,
+            ((float)black_frame_insertion + 1.0f)
+            * (float)video_swap_interval * (float)shader_subframes,
+            audio_max_timing_skew, vrr_runloop_enable);
+
+      if (!(sync_plan & RUNLOOP_SYNC_WITHIN_SKEW) && !vrr_runloop_enable)
+         RARCH_LOG("[Video] Timings deviate too much. Will not adjust."
+               " (Display = %.2f Hz, Game = %.2f Hz)\n",
+               timing_skew_hz, (float)input_fps);
+      else if (vrr_runloop_enable
+            && (sync_plan & RUNLOOP_SYNC_VSYNC_HOLDS)
+            && !(sync_plan & RUNLOOP_SYNC_EXACT_RATE))
+         RARCH_LOG("[Video] Game FPS above what the display presents at "
+               "this interval; VSync paces it instead of the exact rate.\n");
+   }
+
    if (input_sample_rate > 0.0)
    {
       audio_driver_state_t *audio_st      = audio_state_get_ptr();
-      if (vrr_runloop_enable)
+      if (sync_plan & RUNLOOP_SYNC_EXACT_RATE)
          audio_st->input = input_sample_rate;
       else
          audio_st->input =
@@ -1476,40 +1466,23 @@ static void driver_adjust_system_rates(
 
    runloop_st->flags &= ~RUNLOOP_FLAG_FORCE_NONBLOCK;
 
-   if (input_fps > 0.0)
+   if (!(sync_plan & RUNLOOP_SYNC_VSYNC_HOLDS))
    {
-      float timing_skew_hz          = video_refresh_rate;
+      /* We won't be able to do VSync reliably
+         when game FPS > monitor FPS. */
+      runloop_st->flags |= RUNLOOP_FLAG_FORCE_NONBLOCK;
+      RARCH_LOG("[Video] Game FPS > Monitor FPS. Cannot rely on VSync.\n");
 
-      if (retro_atomic_load_acquire_int(&video_st->crt_switching_active))
-         timing_skew_hz             = input_fps;
-      video_st->core_hz             = input_fps;
-
-      if (!video_driver_monitor_adjust_system_rates(
-               timing_skew_hz,
-               video_refresh_rate,
-               vrr_runloop_enable,
-               audio_max_timing_skew,
-               video_swap_interval,
-               black_frame_insertion,
-               shader_subframes,
-               input_fps))
+      if (video_st->data)
       {
-         /* We won't be able to do VSync reliably
-            when game FPS > monitor FPS. */
-         runloop_st->flags |= RUNLOOP_FLAG_FORCE_NONBLOCK;
-         RARCH_LOG("[Video] Game FPS > Monitor FPS. Cannot rely on VSync.\n");
-
-         if (video_st->data)
-         {
-            if (video_st->current_video->set_nonblock_state)
-               video_st->current_video->set_nonblock_state(
-                     video_st->data, true,
-                     video_driver_test_all_flags(GFX_CTX_FLAGS_ADAPTIVE_VSYNC)
-                     && video_adaptive_vsync,
-                     video_swap_interval);
-         }
-         return;
+         if (video_st->current_video->set_nonblock_state)
+            video_st->current_video->set_nonblock_state(
+                  video_st->data, true,
+                  video_driver_test_all_flags(GFX_CTX_FLAGS_ADAPTIVE_VSYNC)
+                  && video_adaptive_vsync,
+                  video_swap_interval);
       }
+      return;
    }
 
    if (video_st->data)

runloop.h

@@ -562,6 +562,62 @@ static INLINE unsigned runloop_video_swap_interval_for(float timing_fps,
    return (timing_skew <= max_timing_skew) ? swap_integer : 1;
 }
 
+/* How a display/content pair is synced, decided whenever the rates are
+ * set. VSYNC_HOLDS: vsync can pace the content without the loop being
+ * forced nonblocking. EXACT_RATE: the content runs at its own rate, so
+ * audio takes the core's sample rate unskewed. WITHIN_SKEW: the content
+ * rate is within @max_timing_skew of the rate the display presents it
+ * at.
+ *
+ * @multiple is how many display frames one content frame occupies -
+ * black frame insertion, swap interval and shader subframes multiplied
+ * - and applies only when the display is near a whole multiple of the
+ * content rate, as the audio skew does.
+ *
+ * With Sync to Exact Content Framerate (@vrr) the content keeps its own
+ * rate while the display can present it. Past that but within the skew
+ * tolerance - a 60.0988 Hz core at swap interval 2 on a 120 Hz panel -
+ * vsync paces it and audio is skewed as it is without VRR: dropping
+ * vsync over a fraction of a percent tears, and where a driver emulates
+ * the interval by presenting a frame again, presents unpaced. Only a
+ * content rate beyond the tolerance keeps its own rate with vsync off. */
+enum runloop_sync_plan
+{
+   RUNLOOP_SYNC_VSYNC_HOLDS = (1 << 0),
+   RUNLOOP_SYNC_EXACT_RATE  = (1 << 1),
+   RUNLOOP_SYNC_WITHIN_SKEW = (1 << 2)
+};
+
+static INLINE unsigned runloop_sync_plan_for(float display_hz,
+      float input_fps, float multiple, float max_timing_skew, bool vrr)
+{
+   float    target = display_hz;
+   float    timing_skew;
+   unsigned plan   = 0;
+
+   if ((input_fps <= 0.0f) || (display_hz <= 0.0f) || (multiple <= 0.0f))
+      return RUNLOOP_SYNC_VSYNC_HOLDS | (vrr ? RUNLOOP_SYNC_EXACT_RATE : 0);
+
+   if ((unsigned)(display_hz / input_fps + 0.5f) > 1)
+      target /= multiple;
+
+   timing_skew = 1.0f - input_fps / target;
+   if (timing_skew < 0.0f)
+      timing_skew = -timing_skew;
+   if (timing_skew <= max_timing_skew)
+      plan |= RUNLOOP_SYNC_WITHIN_SKEW;
+
+   if (input_fps <= target)
+      plan |= RUNLOOP_SYNC_VSYNC_HOLDS
+            | (vrr ? RUNLOOP_SYNC_EXACT_RATE : 0);
+   else if (plan & RUNLOOP_SYNC_WITHIN_SKEW)
+      plan |= RUNLOOP_SYNC_VSYNC_HOLDS;
+   else if (vrr)
+      plan |= RUNLOOP_SYNC_EXACT_RATE;
+
+   return plan;
+}
+
 /* Everything the pace decision reads, gathered once per iteration into
  * one word. Each bit is one fact about this iteration, named for what
  * it means rather than where it lives: fast-forward is NONBLOCKING

samples/runloop/pacing/pacing_test.c

@@ -462,6 +462,117 @@ static void test_swap_interval(void)
                 * PACING_TEST_CEILING));
 }
 
+/* --- how a display/content pair is synced ------------------------ */
+
+#define SP_V RUNLOOP_SYNC_VSYNC_HOLDS
+#define SP_E RUNLOOP_SYNC_EXACT_RATE
+#define SP_W RUNLOOP_SYNC_WITHIN_SKEW
+
+static void check_plan(float display_hz, float input_fps, float multiple,
+      bool vrr, unsigned want, const char *what)
+{
+   char     msg[192];
+   unsigned got = runloop_sync_plan_for(display_hz, input_fps, multiple,
+         PACING_TEST_MAX_SKEW, vrr);
+
+   snprintf(msg, sizeof(msg), "%.4f Hz / %.4f fps x%.0f%s -> %s%s%s, "
+         "wanted %s%s%s (%s)", display_hz, input_fps, multiple,
+         vrr ? " VRR" : "",
+         (got  & SP_V) ? "V" : "-", (got  & SP_E) ? "E" : "-",
+         (got  & SP_W) ? "W" : "-",
+         (want & SP_V) ? "V" : "-", (want & SP_E) ? "E" : "-",
+         (want & SP_W) ? "W" : "-", what);
+   check(got == want, msg);
+}
+
+static void test_sync_plan(void)
+{
+   static const float displays[] =
+   {
+      0.0f, 50.0f, 59.94f, 60.0f, 75.0f, 100.0f, 119.88f, 120.0f,
+      144.0f, 165.0f, 240.0f, 360.0f
+   };
+   static const float contents[] =
+   {
+      0.0f, 24.0f, 30.0f, 49.7f, 50.0f, 53.7f, 57.5f, 59.8261f,
+      59.94f, 60.0f, 60.0988f, 61.0f, 75.0f, 120.0f
+   };
+   static const float multiples[] = { 1.0f, 2.0f, 3.0f, 4.0f };
+   unsigned i, j, k;
+
+   /* Issue 19600: a 60.0988 Hz core at swap interval 2 on a 120 Hz
+    * panel is 0.16% past what the panel presents. VSync paces it,
+    * audio is skewed; the loop is never forced nonblocking over it. */
+   check_plan(120.0f, 60.0988f, 2.0f, true,  SP_V | SP_W,
+         "SNES, interval 2, 120 Hz, VRR: vsync holds, rate skewed");
+   check_plan(120.0f, 59.8261f, 2.0f, true,  SP_V | SP_E | SP_W,
+         "TG16, interval 2, 120 Hz, VRR: exact rate");
+   check_plan(120.0f, 60.0988f, 2.0f, false, SP_V | SP_W,
+         "SNES, interval 2, 120 Hz: vsync holds, rate skewed");
+   check_plan(60.0f,  60.0988f, 1.0f, true,  SP_V | SP_W,
+         "SNES on a 60 Hz VRR ceiling: vsync holds, rate skewed");
+   check_plan(120.0f, 60.0988f, 2.0f * 1.0f, true, SP_V | SP_W,
+         "SNES, one black frame, 120 Hz, VRR");
+
+   /* Past the tolerance the content keeps its own rate under VRR,
+    * with vsync off; without VRR vsync is simply not relied on. */
+   check_plan(120.0f, 75.0f, 2.0f, true,  SP_E,
+         "75 Hz content, interval 2, 120 Hz, VRR: own rate, no vsync");
+   check_plan(60.0f,  75.0f, 1.0f, true,  SP_E,
+         "75 Hz content on 60 Hz, VRR: own rate, no vsync");
+   check_plan(60.0f,  75.0f, 1.0f, false, 0,
+         "75 Hz content on 60 Hz: no vsync");
+
+   /* Under the panel's rate: vsync holds either way, VRR keeps the
+    * exact rate however far the skew. */
+   check_plan(144.0f, 60.0988f, 1.0f, true,  SP_V | SP_E,
+         "60 Hz content on 144 Hz, VRR");
+   check_plan(75.0f,  60.0f,    1.0f, false, SP_V,
+         "60 Hz content on 75 Hz: vsync, rate not skewed");
+
+   /* Degenerate rates change nothing. */
+   check_plan(120.0f, 0.0f,  2.0f, true,  SP_V | SP_E, "no content rate, VRR");
+   check_plan(120.0f, 0.0f,  2.0f, false, SP_V,        "no content rate");
+   check_plan(0.0f,   60.0f, 2.0f, true,  SP_V | SP_E, "no display rate, VRR");
+   check_plan(120.0f, 60.0f, 0.0f, false, SP_V,        "no multiple");
+
+   /* Over every pair: VRR never decides whether vsync holds, only
+    * whether the content keeps its own rate - and it does so exactly
+    * when vsync can present it or the skew is too far to follow. */
+   for (i = 0; i < ARRAY_SIZE(displays); i++)
+      for (j = 0; j < ARRAY_SIZE(contents); j++)
+         for (k = 0; k < ARRAY_SIZE(multiples); k++)
+         {
+            char     msg[160];
+            unsigned off = runloop_sync_plan_for(displays[i], contents[j],
+                  multiples[k], PACING_TEST_MAX_SKEW, false);
+            unsigned on  = runloop_sync_plan_for(displays[i], contents[j],
+                  multiples[k], PACING_TEST_MAX_SKEW, true);
+
+            snprintf(msg, sizeof(msg), "%.4f Hz / %.4f fps x%.0f: "
+                  "VRR moved vsync or the skew (%u vs %u)",
+                  displays[i], contents[j], multiples[k], on, off);
+            check((on & (SP_V | SP_W)) == (off & (SP_V | SP_W)), msg);
+
+            snprintf(msg, sizeof(msg), "%.4f Hz / %.4f fps x%.0f: "
+                  "VRR plan %u holds neither vsync nor the rate",
+                  displays[i], contents[j], multiples[k], on);
+            check((on & (SP_V | SP_E)) != 0, msg);
+
+            snprintf(msg, sizeof(msg), "%.4f Hz / %.4f fps x%.0f: "
+                  "within skew but vsync dropped (%u)",
+                  displays[i], contents[j], multiples[k], on);
+            check(!(on & SP_W) || (on & SP_V), msg);
+
+            check(!(off & SP_E), "exact rate without VRR");
+         }
+
+   printf("   sync plan: %u display/content/multiple triples, VRR "
+          "drops vsync only past the skew tolerance\n",
+          (unsigned)(ARRAY_SIZE(displays) * ARRAY_SIZE(contents)
+                * ARRAY_SIZE(multiples)));
+}
+
 static void test_menu_table(void)
 {
    {
@@ -507,6 +618,7 @@ int main(void)
    test_schedule();
    test_margin();
    test_swap_interval();
+   test_sync_plan();
    test_menu_table();
 
    if (failures)
@@ -520,6 +632,7 @@ int main(void)
           "stall never moves the measured rate, an overshooting sleep "
           "never slows the loop, the margin follows the overshoot, and "
           "the swap interval is the multiple the display actually is of "
-          "the content, within what the driver can present\n");
+          "the content, within what the driver can present, and VRR drops "
+          "vsync only for a content rate past the skew tolerance\n");
    return 0;
 }