From 53107fd91b6c4a70cfb0e6fa8bafaa0170ddc816 Mon Sep 17 00:00:00 2001 From: SAPHID Date: Sun, 25 Oct 2026 02:17:29 +0000 Subject: [PATCH 1/2] Keep saved Steam screenshots separate by headset and game --- CHANGELOG.md | 5 +++ docs/recording.md | 9 ++++ tests/test_shot_copy.py | 92 +++++++++++++++++++++++++++++++++++++++++ ui/server.py | 78 +++++++++++++++++++++++----------- 4 files changed, 161 insertions(+), 23 deletions(-) create mode 100644 tests/test_shot_copy.py diff --git a/CHANGELOG.md b/CHANGELOG.md index a9bc04f3..65c177c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ ## Unreleased +- Save Steam screenshots under a separate folder for each headset, Steam + account and game. Equal screenshot filenames no longer overwrite one another; + existing files in the old flat folder are left in place. + [Copying guide](docs/recording.md). + - Compare two explicit Windows Wireless Adapter doctor reads in local memory. Highlight exact PnP instances and known state changes after a dropout; incomplete inventories stay unknown. A Forget button clears the comparison. diff --git a/docs/recording.md b/docs/recording.md index fd4d7b6..4e1ebcc 100644 --- a/docs/recording.md +++ b/docs/recording.md @@ -107,6 +107,15 @@ about once a minute and copies what's new with the same code as **Save** and **Export**: screenshots to `~/Pictures/SteamFrame`, clips to `~/Videos/SteamFrame`. +Each newly saved screenshot lives under +`~/Pictures/SteamFrame/headset-///`. +The headset folder uses a short hash of Frame Control's device identity. +Steam can use the same screenshot filename for two games or accounts; these +folders keep both originals. Existing screenshots in the old flat +`~/Pictures/SteamFrame/` layout stay where they are. They cannot be +matched reliably to one headset and game, so **Copy everything now** can create +a new, correctly identified copy. It never overwrites the old file. + - **Only what's new.** It copies what you take after turning it on, so the shots and clips already on the headset don't all arrive at once. **Copy everything now** copies everything that isn't on this computer yet, once. diff --git a/tests/test_shot_copy.py b/tests/test_shot_copy.py new file mode 100644 index 0000000..f715371 --- /dev/null +++ b/tests/test_shot_copy.py @@ -0,0 +1,92 @@ +"""Screenshot copies retain the headset, Steam account and game identity.""" +import sandbox # noqa: F401 +import shutil +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path +from unittest import mock + +ROOT = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(ROOT / "ui")) +import server # noqa: E402 + + +NAME = "20261025020000_1.jpg" +FIRST = f"1/620/{NAME}" +SECOND = f"1/730/{NAME}" + + +class ShotCopyTest(unittest.TestCase): + def setUp(self): + self.folder = Path(tempfile.mkdtemp(prefix="frame-shot-copy-")) + self.addCleanup(shutil.rmtree, self.folder, ignore_errors=True) + patch = mock.patch.object(server, "SHOTS_DIR", self.folder) + patch.start() + self.addCleanup(patch.stop) + patch = mock.patch.object(server, "LINK", None) + patch.start() + self.addCleanup(patch.stop) + self.payloads = { + server.shot_path(FIRST): b"first game", + server.shot_path(SECOND): b"second game", + } + + def scp(self, argv, **_kwargs): + target = Path(argv[-1]) + for source in argv[1:-1]: + if not source.startswith("frame-a:") and not source.startswith("frame-b:"): + continue + remote = source.split(":", 1)[1] + (target / Path(remote).name).write_bytes(self.payloads[remote]) + return subprocess.CompletedProcess(argv, 0, "", "") + + def test_equal_names_survive_across_games_and_headsets(self): + # A pre-upgrade flat file has no provable source identity. Leave it alone. + legacy = self.folder / NAME + legacy.write_bytes(b"older screenshot") + with mock.patch.object(server, "FRAME", "frame-a"), \ + mock.patch.object(server, "ensure_master"), \ + mock.patch.object(server.frame_host, "run_ssh", side_effect=self.scp) as copy: + self.assertEqual(server.save_shots({"ids": [FIRST, SECOND]})["saved"], 2) + self.assertEqual(copy.call_count, 2) # scp cannot flatten equal names together + first = server.shot_local_path(FIRST) + second = server.shot_local_path(SECOND) + self.assertEqual(first.read_bytes(), b"first game") + self.assertEqual(second.read_bytes(), b"second game") + self.assertNotEqual(first, second) + self.assertEqual(legacy.read_bytes(), b"older screenshot") + listing = "\n".join([ + f"1/760/remote/620/screenshots/{NAME}\t10\t1792893600", + f"1/760/remote/730/screenshots/{NAME}\t11\t1792893601", + ]) + with mock.patch.object(server, "ssh", return_value=listing): + self.assertTrue(all(s["saved"] for s in server.list_shots()["shots"])) + with mock.patch.object(server.frame_host, "reveal_path") as reveal: + server.open_thing({"what": "shot", "id": SECOND}) + reveal.assert_called_once_with(second) + + self.payloads[server.shot_path(FIRST)] = b"another headset" + with mock.patch.object(server, "FRAME", "frame-b"), \ + mock.patch.object(server, "ensure_master"), \ + mock.patch.object(server.frame_host, "run_ssh", side_effect=self.scp): + self.assertEqual(server.save_shots({"ids": [FIRST]})["saved"], 1) + other = server.shot_local_path(FIRST) + self.assertNotEqual(other, first) + self.assertEqual(other.read_bytes(), b"another headset") + self.assertEqual(first.read_bytes(), b"first game") + + def test_missing_staged_file_does_not_claim_saved(self): + with mock.patch.object(server, "FRAME", "frame-a"), \ + mock.patch.object(server, "ensure_master"), \ + mock.patch.object(server.frame_host, "run_ssh", + return_value=subprocess.CompletedProcess([], 0, "", "")): + with self.assertRaisesRegex(server.Failure, "did not contain every"): + server.save_shots({"ids": [FIRST]}) + self.assertFalse(server.shot_local_path(FIRST).exists()) + self.assertFalse(list(self.folder.glob(".incoming-*"))) + + +if __name__ == "__main__": + unittest.main() diff --git a/ui/server.py b/ui/server.py index ec5671f..b7aa143 100755 --- a/ui/server.py +++ b/ui/server.py @@ -635,6 +635,7 @@ def _headset_view(): SHOT_ROOT = ".local/share/Steam/userdata" SHOT_ID = re.compile(r"(\d{1,12})/(\d{1,20})/(\d{14}_\d{1,4}\.(?:jpg|png))") SHOTS_DIR = Path.home() / "Pictures" / "SteamFrame" +_shots_copy_lock = threading.Lock() LIST_SHOTS = f"""cd ~/{SHOT_ROOT} 2>/dev/null || exit 0 find . -mindepth 6 -maxdepth 6 -path './*/760/remote/*/screenshots/*' -type f \\ \\( -name '*.jpg' -o -name '*.png' \\) -printf '%P\\t%s\\t%T@\\n'""" @@ -647,8 +648,19 @@ def shot_path(shot_id, thumb=False): return f"{SHOT_ROOT}/{m[1]}/760/remote/{m[2]}/screenshots/{'thumbnails/' if thumb else ''}{m[3]}" +def shot_local_path(shot_id, device_id=None): + """Keep equal Steam filenames from different headsets/accounts/games separate.""" + m = SHOT_ID.fullmatch(shot_id) if isinstance(shot_id, str) else None + if not m: + raise Failure("bad screenshot id", 400) + identity = device_id if device_id is not None else agent_target()["id"] + headset = hashlib.sha256(identity.encode("utf-8")).hexdigest()[:12] + return SHOTS_DIR / f"headset-{headset}" / m[1] / m[2] / m[3] + + def list_shots(): shots = [] + device_id = agent_target()["id"] for line in ssh(LIST_SHOTS, timeout=20).splitlines(): rel, _, rest = line.partition("\t") parts = rel.split("/") # account/760/remote/appid/screenshots/file @@ -660,7 +672,7 @@ def list_shots(): when = float(mtime) except ValueError: continue - local = SHOTS_DIR / parts[-1] + local = shot_local_path(shot_id, device_id) shots.append({"id": shot_id, "appid": parts[3], "file": parts[-1], "size": int(size), "time": when, "saved": local.exists() and local.stat().st_size == int(size)}) shots.sort(key=lambda s: s["time"], reverse=True) @@ -682,31 +694,51 @@ def shot_image(query): def save_shots(body): - """Copy screenshots to ~/Pictures/SteamFrame, skipping ones already there.""" + """Copy screenshots to per-headset/account/game folders without name collisions.""" ids = body.get("ids") if not isinstance(ids, list) or not 0 < len(ids) <= 1000: raise Failure("ids must be a list of 1-1000 screenshot ids", 400) - paths = [shot_path(i) for i in ids] - todo = [p for p in paths if not (SHOTS_DIR / p.rsplit("/", 1)[-1]).exists()] - if todo: - SHOTS_DIR.mkdir(parents=True, exist_ok=True) - ensure_master() - # Copy into a hidden folder and move complete files in, so a cut-off - # copy never looks saved. -p keeps the time the shot was taken. - incoming = Path(tempfile.mkdtemp(prefix=".incoming-", dir=SHOTS_DIR)) - try: + device_id = agent_target()["id"] + items = [(shot_path(i), shot_local_path(i, device_id)) for i in ids] + n = 0 + with _shots_copy_lock: + todo = [(remote, local) for remote, local in items if not local.exists()] + if todo: + SHOTS_DIR.mkdir(parents=True, exist_ok=True) + ensure_master() + # scp flattens source paths into one destination directory. Split equal + # basenames into separate transfers before assigning each file its ID path. + while todo: + names, group, rest = set(), [], [] + for item in todo: + if item[1].name in names: + rest.append(item) + else: + names.add(item[1].name) + group.append(item) + todo = rest + # Stage under the destination filesystem, then move complete files in. + # -p keeps Steam's screenshot timestamp. + incoming = Path(tempfile.mkdtemp(prefix=".incoming-", dir=SHOTS_DIR)) try: - r = frame_host.run_ssh(["scp", "-p", *SSH[1:], *(f"{FRAME}:{p}" for p in todo), str(incoming)], - capture_output=True, stdin=subprocess.DEVNULL, text=True, timeout=300) - except subprocess.TimeoutExpired: - raise Failure("Copying screenshots timed out") - if r.returncode != 0: - raise Failure(strip_ansi(r.stderr).strip() or f"scp exited {r.returncode}") - for f in incoming.iterdir(): - os.replace(f, SHOTS_DIR / f.name) - finally: - shutil.rmtree(incoming, ignore_errors=True) - n, skipped = len(todo), len(ids) - len(todo) + try: + r = frame_host.run_ssh(["scp", "-p", *SSH[1:], + *(f"{FRAME}:{remote}" for remote, _ in group), str(incoming)], + capture_output=True, stdin=subprocess.DEVNULL, text=True, timeout=300) + except subprocess.TimeoutExpired: + raise Failure("Copying screenshots timed out") + if r.returncode != 0: + raise Failure(strip_ansi(r.stderr).strip() or f"scp exited {r.returncode}") + if any(not (incoming / local.name).is_file() for _, local in group): + raise Failure("Screenshot copy did not contain every requested file") + for _, local in group: + local.parent.mkdir(parents=True, exist_ok=True) + if not local.exists(): + os.replace(incoming / local.name, local) + n += 1 + finally: + shutil.rmtree(incoming, ignore_errors=True) + skipped = len(ids) - n msg = f"Saved {n} screenshot{'s' * (n != 1)} to ~/Pictures/SteamFrame" return {"message": msg + (f" ({skipped} already there)" if skipped else ""), "saved": n} @@ -1936,7 +1968,7 @@ def open_thing(body): frame_host.open_path(frame_clips.VIDEOS_DIR) return {"message": f"Opened {frame_clips.VIDEOS_DIR} in {frame_host.FILE_MANAGER}"} if what == "shot": - saved = SHOTS_DIR / shot_path(body.get("id")).rsplit("/", 1)[-1] + saved = shot_local_path(body.get("id")) if not saved.exists(): raise Failure("That screenshot isn't saved on this computer yet", 404) frame_host.reveal_path(saved) From af2dca8365a6f087f9df6a6380dae300cd3453bb Mon Sep 17 00:00:00 2001 From: SAPHID Date: Sun, 25 Oct 2026 02:29:56 +0000 Subject: [PATCH 2/2] Document real Frame screenshot copy check --- docs/evidence/screenshot-copy-2026-10-25.md | 25 +++++++++++++++++++++ docs/recording.md | 3 +++ 2 files changed, 28 insertions(+) create mode 100644 docs/evidence/screenshot-copy-2026-10-25.md diff --git a/docs/evidence/screenshot-copy-2026-10-25.md b/docs/evidence/screenshot-copy-2026-10-25.md new file mode 100644 index 0000000..13dc061 --- /dev/null +++ b/docs/evidence/screenshot-copy-2026-10-25.md @@ -0,0 +1,25 @@ +# Screenshot Save on a real Frame, 25 October 2026 + +After PR #192 merged, Frame Control's production `list_shots()` and +`save_shots()` paths were run from lxso2 against the real Frame. The test held +the shared `/tmp/frame-test.lock` with an owner marker and removed only its own +lock. A separate locked check earlier in this round found SteamVR at activity +level 3 (standby). No new screenshot was created. + +- The Frame listed seven existing Steam screenshots. +- One existing screenshot was copied to a temporary folder on the PC. Its + local size matched the reported remote size: **294,885 bytes**. The new + destination had four components below the temporary root: headset, Steam + account, game ID and filename. +- Saving the same screenshot a second time returned zero new copies. A fresh + list then marked that screenshot as saved. +- The temporary PC folder and its private image were removed at the end. The + source screenshot on the Frame was only read. Battery was **100% Full** + before and after. + +This verifies one real screenshot transfer and the repeat-save behavior. It +does **not** reproduce two different real screenshots with the same filename; +the account/game/headset collision case is covered by the fake-source test in +`tests/test_shot_copy.py`. Windows and macOS physical-PC saves were not tested. +Active passthrough in the recording mirror remains unknown because the Frame +was in standby. diff --git a/docs/recording.md b/docs/recording.md index 4e1ebcc..ea54db1 100644 --- a/docs/recording.md +++ b/docs/recording.md @@ -115,6 +115,9 @@ folders keep both originals. Existing screenshots in the old flat `~/Pictures/SteamFrame/` layout stay where they are. They cannot be matched reliably to one headset and game, so **Copy everything now** can create a new, correctly identified copy. It never overwrites the old file. +One real Frame screenshot was copied through this path on 25 October 2026; +see the [test record](evidence/screenshot-copy-2026-10-25.md). The collision +case was tested with separate fake sources, not two real same-name screenshots. - **Only what's new.** It copies what you take after turning it on, so the shots and clips already on the headset don't all arrive at once. **Copy