netplay: one CRC check for both paths, decided anew after each state load

A client compares the host's CRC for a frame either when it arrives or
once the client reaches that frame, and both now go through one check.
The first check at or after a state loaded from the host decides
whether this core's CRCs can be compared: that state is the host's, so
a mismatch there turns desync checks off with a warning, as a core
whose states carry a timestamp needs. Any later mismatch asks for the
host's state. A client logs each state it loads from the host.

samples/network/netplay_e2e gains a rollback desync lane with input
latency, so some checks wait for the client to reach their frame, and
a lane whose core states never hash alike between peers, which must
stop at one resync past the join with its checks turned off.
NETPLAY_E2E_JUNK puts a per-peer serialize count in the test core's
state.

network/netplay/netplay_frontend.c

@@ -3898,6 +3898,41 @@ static bool netplay_resolve_input(netplay_t *netplay,
    return ret;
 }
 
+/* Compare the host's CRC for a frame with ours. The first check at or
+ * after a state we loaded from the host decides whether CRCs can be
+ * compared at all: that state is the host's, so a mismatch there means
+ * this core's hashes differ between peers whatever they run, and a
+ * resync could never mend it. Any later mismatch is a desync, and asks
+ * for the host's state. */
+static void netplay_check_frame_crc(netplay_t *netplay,
+      struct delta_frame *delta, uint32_t remote_crc)
+{
+   uint32_t local_crc;
+
+   if (     !(netplay->flags & NETPLAY_FLAG_CRCS_VALID)
+         || !remote_crc
+         || (int32_t)(delta->frame - netplay->crc_load_frame) < 0)
+      return;
+
+   local_crc = netplay_frame_local_crc(netplay, delta);
+   /* lockstep: this frame was not hashed */
+   if (!local_crc && (netplay->flags & NETPLAY_FLAG_LOCKSTEP))
+      return;
+
+   if (local_crc == remote_crc)
+      netplay->flags |= NETPLAY_FLAG_CRC_VALIDITY_CHECKED;
+   else if (!(netplay->flags & NETPLAY_FLAG_CRC_VALIDITY_CHECKED))
+   {
+      RARCH_WARN("[Netplay] CRCs differ from the host's right after its "
+            "state was loaded; desync checks are off for this core.\n");
+      netplay->flags &= ~NETPLAY_FLAG_CRCS_VALID;
+   }
+   else if (netplay->check_frames)
+      netplay_cmd_request_savestate(netplay);
+   else
+      RARCH_WARN("[Netplay] Netplay CRCs mismatch!\n");
+}
+
 static void netplay_handle_frame_hash(netplay_t *netplay,
       struct delta_frame *delta)
 {
@@ -3913,34 +3948,7 @@ static void netplay_handle_frame_hash(netplay_t *netplay,
       }
    }
    else
-   {
-      if ((netplay->flags & NETPLAY_FLAG_CRCS_VALID) && delta->crc)
-      {
-         /* We have a remote CRC, so check it. */
-         uint32_t local_crc = netplay_frame_local_crc(netplay, delta);
-
-         if (!local_crc && (netplay->flags & NETPLAY_FLAG_LOCKSTEP))
-            return;
-
-         if (local_crc != delta->crc)
-         {
-            /* If the very first check frame is wrong,
-               they probably just don't work. */
-            if (!(netplay->flags & NETPLAY_FLAG_CRC_VALIDITY_CHECKED))
-            {
-               netplay->flags &= ~NETPLAY_FLAG_CRCS_VALID;
-               return;
-            }
-
-            if (netplay->check_frames)
-               netplay_cmd_request_savestate(netplay);
-            else
-               RARCH_WARN("[Netplay] Netplay CRCs mismatch!\n");
-         }
-         else
-            netplay->flags |= NETPLAY_FLAG_CRC_VALIDITY_CHECKED;
-      }
-   }
+      netplay_check_frame_crc(netplay, delta, delta->crc);
 }
 
 /**
@@ -6784,18 +6792,11 @@ static bool netplay_get_cmd(netplay_t *netplay,
             if (!found)
                break;
 
+            /* We've already replayed up to this frame, so we can check it
+             * directly */
             if (buffer[0] <= netplay->other_frame_count)
-            {
-               /* We've already replayed up to this frame, so we can check it
-                * directly */
-               uint32_t local_crc = netplay_frame_local_crc(
-                     netplay, &netplay->buffer[tmp_ptr]);
-
-               /* Problem! (lockstep: if this frame was hashed at all) */
-               if (     buffer[1] != local_crc
-                     && (local_crc || !(netplay->flags & NETPLAY_FLAG_LOCKSTEP)))
-                  netplay_cmd_request_savestate(netplay);
-            }
+               netplay_check_frame_crc(netplay, &netplay->buffer[tmp_ptr],
+                     buffer[1]);
             /* We'll have to check it when we catch up */
             else
                netplay->buffer[tmp_ptr].crc = buffer[1];
@@ -7058,9 +7059,13 @@ static bool netplay_get_cmd(netplay_t *netplay,
             }
 
             /* Make sure our states are correct. */
-            netplay->flags &= ~NETPLAY_FLAG_SAVESTATE_REQUEST_OUTSTANDING;
+            netplay->flags &= ~(NETPLAY_FLAG_SAVESTATE_REQUEST_OUTSTANDING
+                              | NETPLAY_FLAG_CRC_VALIDITY_CHECKED);
+            netplay->crc_load_frame                = load_frame_count;
             netplay->other_ptr                     = load_ptr;
             netplay->other_frame_count             = load_frame_count;
+            RARCH_LOG("[Netplay] Loaded the host's state at frame %u.\n",
+                  load_frame_count);
 
             break;
          }

network/netplay/netplay_private.h

@@ -523,7 +523,7 @@ enum netplay_flags
    /* Are we the server? */
    NETPLAY_FLAG_IS_SERVER                     = (1 << 1),
    NETPLAY_FLAG_NAT_TRAVERSAL                 = (1 << 2),
-   /* Have we checked whether CRCs are valid at all? */
+   /* Has a check matched since the last state we loaded? */
    NETPLAY_FLAG_CRC_VALIDITY_CHECKED          = (1 << 3),
    /* Are they valid? */
    NETPLAY_FLAG_CRCS_VALID                    = (1 << 4),
@@ -687,6 +687,8 @@ struct netplay
 
    /* Frequency with which to check CRCs */
    uint32_t check_frames;
+   /* The frame of the last state loaded from the host */
+   uint32_t crc_load_frame;
 
    /* How far behind did we fall? */
    uint32_t catch_up_behind;

samples/network/netplay_e2e/netplay_e2e.py

@@ -14,10 +14,14 @@ port's stick. Passes when:
   - a lockstep peer never re-runs a frame, but after a resync: lockstep
     asked for, lockstep for a state too large to roll back, and a
     lockstep host with a rollback client
-  - a lockstep client whose RAM is upset is caught at a check frame and
-    given the host's state: frames differ for a while, then never again,
-    also when the client lags the host and the state arrives labeled
-    with a frame it has not reached
+  - a client whose RAM is upset is caught at a check frame and given the
+    host's state: frames differ for a while, then never again; in
+    lockstep, also when the client lags the host and the state arrives
+    labeled with a frame it has not reached; in rollback, with input
+    latency, so some checks wait for the client to reach their frame
+  - a core whose states never hash alike (a count in them that differs
+    between peers) is resynced once at most past the join, then its
+    checks are turned off
 
 Usage: netplay_e2e.py /path/to/retroarch /path/to/netplay_e2e_libretro.so
 """
@@ -113,7 +117,7 @@ def replays(order, start):
 
 
 def run(retroarch, core, analog, label, host_extra=None, client_extra=None,
-        pad=0, lockstep=(False, False), desync=0, client_pace=4):
+        pad=0, lockstep=(False, False), desync=0, client_pace=4, junk=False):
     d     = tempfile.mkdtemp(prefix="netplay_e2e_")
     nport = free_port(socket.SOCK_STREAM)
     rport = free_port(socket.SOCK_DGRAM)
@@ -127,7 +131,8 @@ def run(retroarch, core, analog, label, host_extra=None, client_extra=None,
             "network_remote_enable": "true",
             "network_remote_enable_user_p1": "true",
             "network_remote_base_port": str(rport)}))
-        env = dict(os.environ, NETPLAY_E2E_PAD=str(pad))
+        env = dict(os.environ, NETPLAY_E2E_PAD=str(pad),
+                   NETPLAY_E2E_JUNK="1" if junk else "0")
         host_log, client_log = (os.path.join(d, n + ".log")
                                 for n in ("host", "client"))
 
@@ -216,6 +221,15 @@ def run(retroarch, core, analog, label, host_extra=None, client_extra=None,
                          % (label, moved, len(shared)))
         if not analog and moved:
             fails.append("%s: the stick crossed with sync off (%d frames)" % (label, moved))
+        if junk:
+            with open(os.path.join(d, "client.txt"), errors="replace") as f:
+                text  = f.read()
+            loads = text.count("Loaded the host's state")
+            if loads > 2:
+                fails.append("%s: the client loaded the host's state %d times"
+                             % (label, loads))
+            if "desync checks are off" not in text:
+                fails.append("%s: the CRC checks were never turned off" % label)
         for dev in host_dev + client_dev:
             if dev & ~0xff00 not in (0, 1):
                 fails.append("%s: the core was given device 0x%x" % (label, dev))
@@ -253,6 +267,12 @@ def main():
     # is labeled with when it arrives
     fails += run(retroarch, core, True, "lockstep, a desync mended, slow client",
                  lock, lock, lockstep=(True, True), desync=200, client_pace=12)
+    # the host's input latency makes the client lag the frames the host
+    # confirms, so some CRCs arrive before the client has run their frame
+    fails += run(retroarch, core, True, "rollback, a desync mended",
+                 {"netplay_input_latency_frames_min": "8"}, None, desync=200)
+    fails += run(retroarch, core, True, "states that never hash alike",
+                 junk=True)
     # past the size rollback keeps one of per frame: lockstep unasked
     fails += run(retroarch, core, True, "a 40 MB state", pad=40 << 20,
                  lockstep=(True, True))

samples/network/netplay_e2e/netplay_e2e_core.c

@@ -7,7 +7,9 @@
  * the RAM, to stand in for a large one. NETPLAY_E2E_DESYNC: frames run
  * after the first state load before the RAM is upset once.
  * NETPLAY_E2E_PACE_MS: milliseconds slept a frame (4 if unset), to make
- * one peer lag the other.
+ * one peer lag the other. NETPLAY_E2E_JUNK: the state carries a count of
+ * its own serializations, which differs between peers and never reaches
+ * the RAM, as a core that stores a timestamp would.
  *
  * Paced at about 250 frames a second by default: null video runs
  * unthrottled, and the logs would grow by the hundred thousand lines. */
@@ -43,6 +45,8 @@ static uint8_t  ram[RAM_SIZE];
 static uint32_t frame;
 static uint32_t pixels[64];
 static size_t   pad;
+static uint32_t junk;
+static uint32_t serials;
 static unsigned desync_after;
 static unsigned since_load;
 static unsigned pace_ms;
@@ -87,9 +91,12 @@ void retro_init(void)
    const char *l = getenv("NETPLAY_E2E_LOG");
    const char *u = getenv("NETPLAY_E2E_DESYNC");
    const char *m = getenv("NETPLAY_E2E_PACE_MS");
+   const char *j = getenv("NETPLAY_E2E_JUNK");
    pad      = p ? (size_t)strtoul(p, NULL, 10) : 0;
    desync_after = u ? (unsigned)strtoul(u, NULL, 10) : 0;
    pace_ms  = m ? (unsigned)strtoul(m, NULL, 10) : 4;
+   junk     = (j && *j == '1') ? 4 : 0;
+   serials  = 0;
    loaded   = false;
    since_load = 0;
    log_file = l ? fopen(l, "w") : NULL;
@@ -170,7 +177,7 @@ void retro_run(void)
    pace(pace_ms);
 }
 
-size_t retro_serialize_size(void) { return 4 + RAM_SIZE + pad; }
+size_t retro_serialize_size(void) { return 4 + RAM_SIZE + junk + pad; }
 
 bool retro_serialize(void *data, size_t size)
 {
@@ -180,10 +187,15 @@ bool retro_serialize(void *data, size_t size)
       return false;
    memcpy(d, &frame, 4);
    memcpy(d + 4, ram, RAM_SIZE);
+   if (junk)
+   {
+      serials++;
+      memcpy(d + 4 + RAM_SIZE, &serials, 4);
+   }
    /* the padding follows the RAM, so it compresses no better than a
     * real core's state */
    for (i = 0; i < pad; i++)
-      d[4 + RAM_SIZE + i] = (uint8_t)(ram[i % RAM_SIZE] ^ (i * 2654435761u >> 13));
+      d[4 + RAM_SIZE + junk + i] = (uint8_t)(ram[i % RAM_SIZE] ^ (i * 2654435761u >> 13));
    return true;
 }