diff --git a/CHANGELOG.md b/CHANGELOG.md index 25e71ed..c9786f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ ## Unreleased +- Keep Steam clip exports separate for each headset, Steam account and nested + recording session. Manual export and automatic copying use the same folders; + existing flat exports stay untouched and do not mark unrelated clips saved. + [Recording guide](docs/recording.md#steam-clips). + - Import a base APK with supported native-library or asset splits from a local Android backup or XAPK/APKS/APKM bundle. Check package and version before install; refuse resource, code and feature splits and device-variant sets. diff --git a/docs/evidence/steam-clip-export-collisions-2026-10-25.md b/docs/evidence/steam-clip-export-collisions-2026-10-25.md new file mode 100644 index 0000000..444be24 --- /dev/null +++ b/docs/evidence/steam-clip-export-collisions-2026-10-25.md @@ -0,0 +1,70 @@ +# Steam clip export collision fixtures, 25 October 2026 + +Base: `d79efb4` (Frame Control v0.4.30). Issue: +[#212](https://github.com/saphid/frame-control/issues/212). +Host: Linux 6.8.0-139-generic, Python 3.12.3, ffmpeg 6.1.1. + +## Baseline reproduction + +Loaded the unmodified `ui/frame_clips.py` from `d79efb4` into a temporary +Python module and exported three fake sources into a temporary directory: + +``` +1/clips/clip_620/video/bg +2/clips/clip_620/video/bg +1/clips/clip_620/video/other +``` + +Each source's payload was its own ID. All three exports returned the same +destination, `clip_620.mp4`; listing marked all three saved. The final file +contained only the last source ID. No SSH or headset was used. + +## Regression coverage + +`tests/test_clips.py` exports five IDs on each of two fake headsets, including +equal clip names across accounts, nested sessions with equal leaf names, +different leaf names under one clip and the background/clip distinction. +All ten payloads survive at distinct destinations. Listing marks only the +identities already exported. A pre-existing flat MP4 retains its original +bytes and does not mark any identity saved. Normal export still returns +`message` and `file`, removes its remote temporary export and clears local +staging; a failed re-export preserves the completed file. + +`tests/test_clip_copy.py` exercises the server's list and manual export +functions and its actual auto-sync callbacks with fake transport. A manual +copy is recognised by auto-sync, all other account/session identities copy, +and switching to a second headset copies its own sources without overwriting +the first. A delayed manual job refuses a changed headset with status 409. +Both local phone mode and local headset runtime reject Steam clip list, +export and auto-sync requests with status 400. The separate `Recorder` path +is unchanged. + +## Validation + +- `PYTHONPATH=tests python3 -m unittest test_clips test_clip_copy test_autosync test_shot_copy test_headset_actions -v` + — 66 tests passed in 17.288 seconds, no skips. Includes real ffmpeg/ffprobe + export of a generated 65-second DASH fixture and recording stop/cleanup. +- `PYTHONPATH=tests python3 -m unittest test_server -v` + — 44 tests passed in 12.315 seconds, no skips. +- `node --test site/test/*.test.mjs` — 2 tests passed. +- `python3 -m unittest discover -s tests -v` — 1,518 tests run in + 181.025 seconds; OK, 16 skipped for unavailable platform/tool prerequisites. +- `python3 -m py_compile ui/frame_clips.py ui/server.py tests/test_clips.py tests/test_clip_copy.py` + and `git diff --check` — passed. + +The server and full Python suites require localhost sockets and were run +outside the network sandbox. Tests import `tests/sandbox.py` to isolate app +data, SSH settings, home directories and service URLs. + +## Limits and upgrade behaviour + +No real headset was accessed: the shared device lock is held by other work. +This is Linux fixture and server coverage, not a fresh packaged-app or +Windows/macOS verification. Existing real-headset export evidence in the +recording guide predates this destination change. + +Old flat exports cannot prove headset/account/session identity and are left +untouched. Previously remembered auto-sync IDs retain their existing +watermark/history behaviour; **Copy everything now** or manual **Export** +creates an identified copy. No release, version bump, merge or social post +is included. PR #113 is outside this task. diff --git a/docs/recording.md b/docs/recording.md index ea54db1..13ba209 100644 --- a/docs/recording.md +++ b/docs/recording.md @@ -69,11 +69,31 @@ yes-or-no answer about passthrough pixels. See the **Steam clips** lists what Steam's game recording has kept on the Frame: saved clips and background recordings, newest first, with the game, length and size. -**Export** joins one into a single `.mp4` in `~/Videos/SteamFrame`, named after -its folder (for example `clip_620_20261020_202134.mp4`), however long it is. +**Export** joins one into a single `.mp4` under `~/Videos/SteamFrame`, named after +its clip folder (for example `clip_620_20261020_202134.mp4`), however long it is. Steam's own export is reported to stop at about 60 seconds. Exports run in the background; ones already on your computer are marked. +Each export keeps its headset, Steam account and complete recording session ID: + +``` +~/Videos/SteamFrame/headset-//clips//video//.mp4 +~/Videos/SteamFrame/headset-//video//.mp4 +``` + +The headset folder uses the same short device-identity hash as screenshots. +All folders in the Steam clip ID are kept, including nested sessions, so equal +clip names across accounts, headsets or sessions keep separate files. Manual +**Export** and automatic copying use the same path and saved marker. Exporting +the same identified session again replaces only that session's export. + +Existing exports in the old flat `~/Videos/SteamFrame/.mp4` layout stay +untouched. Their source identity cannot be proved, so they do not mark a clip +as saved. Export again or use **Copy everything now** to make an identified +copy. Automatic copying still remembers IDs copied before the upgrade; use +**Copy everything now** to copy those again. This destination change has local +fixture coverage; it has not been tested on a real headset. + How it works: Steam keeps each recording as MPEG-DASH segments, one folder per recording session. @@ -147,8 +167,10 @@ and the last problem it had. The screenshot and clip lists say when it's on. `ui/frame_clips.py` holds the logic, so other features can reuse it (automatic copying, in `ui/frame_autosync.py`, does): -- `list_clips(run_frame)` and `export_clip(id, run_frame, run, fetch)` are the - computer's side; the server passes how to reach the Frame. +- `list_clips(run_frame, device_id=...)` and + `export_clip(id, run_frame, run, fetch, device_id=...)` are the computer's + side; the server passes how to reach the Frame and its pinned device identity. + Both use `clip_local_path(id, device_id, folder=None)` for the destination. - `list_recordings(userdata)` and `export(userdata, id)` are the Frame's side, run as `python3 - list` / `python3 - export ID` with the file on stdin. - `Recorder` runs one recording from Start to the copy. @@ -171,7 +193,8 @@ The API, with the same guards as every other request: They aren't available when the server runs on the Frame itself (the iPhone app), since they save to a computer. -Tests: `tests/test_clips.py`, and `tests/test_autosync.py` for automatic +Tests: `tests/test_clips.py`, `tests/test_clip_copy.py` for server destinations +and collision coverage, and `tests/test_autosync.py` for automatic copying (with a fake headset). They run the recording's shell under bash with a stand-in `ffmpeg`, so Stop, the time limit and the clean-up are tested as they run on the Frame. Where `ffmpeg` is installed, they also make a 65-second DASH diff --git a/tests/test_clip_copy.py b/tests/test_clip_copy.py new file mode 100644 index 0000000..75f9f89 --- /dev/null +++ b/tests/test_clip_copy.py @@ -0,0 +1,115 @@ +"""Clip destinations through the server and automatic copying, without a headset.""" +import sandbox # noqa: F401 +import shutil +import sys +import tempfile +import unittest +from pathlib import Path +from unittest import mock + +sys.path.insert(0, str(Path(__file__).resolve().parents[1] / "ui")) +import frame_autosync +import frame_clips +import server + + +IDS = ["1/clips/clip_620/video/bg", "2/clips/clip_620/video/bg", + "1/clips/clip_620/video/other", "1/clips/other/video/bg"] +NOW = 1800000000 + + +class ClipCopyTest(unittest.TestCase): + def setUp(self): + self.folder = Path(tempfile.mkdtemp(prefix="frame-clip-copy-")) + self.addCleanup(shutil.rmtree, self.folder, ignore_errors=True) + self.device = "frame-a" + self.link = mock.Mock() + self.link.active_device.side_effect = lambda: {"id": self.device, "name": self.device} + self.link.snapshot.return_value = {"phase": "connected"} + self.exports = [] + self.payload = None + for name, value in (("LINK", self.link), ("LOCAL", False)): + patch = mock.patch.object(server, name, value) + patch.start() + self.addCleanup(patch.stop) + patch = mock.patch.object(frame_clips, "VIDEOS_DIR", self.folder) + patch.start() + self.addCleanup(patch.stop) + for name, fn in (("_clips_frame", self.frame), ("fetch_file", self.fetch), + ("ssh", lambda *a, **k: "")): + patch = mock.patch.object(server, name, side_effect=fn) + patch.start() + self.addCleanup(patch.stop) + (self.folder / "clip_620.mp4").write_bytes(b"legacy export") + + def frame(self, args, timeout): + if args == ["list"]: + return {"clips": [{"id": cid, "kind": "clip", "time": NOW + 5, "idle": 60} for cid in IDS]} + self.exports.append((self.device, args[1])) + self.payload = f"{self.device}: {args[1]}".encode() + return {"path": f"{frame_clips.WORK}/export-0123456789abcdef/clip_620.mp4", + "size": len(self.payload)} + + def fetch(self, remote, target): + target.write_bytes(self.payload) + + def test_manual_export_lists_only_matching_identity_and_keeps_job_shape(self): + with mock.patch.object(server, "start_job", side_effect=lambda label, work: work()) as job: + for device in ("frame-a", "frame-b"): + self.device = device + self.assertFalse(any(c["saved"] for c in server.list_clips()["clips"])) + result = server.export_clip({"id": IDS[0]}) + self.assertEqual(set(result), {"message", "file"}) + self.assertEqual(Path(result["file"]).read_bytes(), f"{device}: {IDS[0]}".encode()) + self.assertEqual([c["saved"] for c in server.list_clips()["clips"]], [True, False, False, False]) + self.assertEqual(job.call_args.args[0], "Export clip_620.mp4") + first = frame_clips.clip_local_path(IDS[0], "frame-a", self.folder) + self.assertEqual(first.read_bytes(), f"frame-a: {IDS[0]}".encode()) + self.assertEqual((self.folder / "clip_620.mp4").read_bytes(), b"legacy export") + + def test_auto_sync_uses_same_identity_as_manual_exports(self): + with mock.patch.object(server, "start_job", side_effect=lambda label, work: work()): + server.export_clip({"id": IDS[0]}) + sync = frame_autosync.AutoSync( + server._sync_list(server.list_shots), server._sync_shots, + server._sync_list(server.list_clips), server._sync_clip, + server._sync_device, lambda: False, shots_folder=self.folder / "shots", + clips_folder=self.folder, path=self.folder / "auto-sync.json", clock=lambda: NOW, + space=lambda folder: 100 * 1024**3) + sync.update({"clips": True}) + sync.tick() + self.assertEqual(self.exports, [("frame-a", cid) for cid in IDS]) + self.device = "frame-b" + sync.tick() + self.assertEqual(self.exports, [(device, cid) for device in ("frame-a", "frame-b") for cid in IDS]) + sync.tick() + self.assertEqual(len(self.exports), 8) + for device in ("frame-a", "frame-b"): + for cid in IDS: + target = frame_clips.clip_local_path(cid, device, self.folder) + self.assertEqual(target.read_bytes(), f"{device}: {cid}".encode()) + self.assertEqual((self.folder / "clip_620.mp4").read_bytes(), b"legacy export") + + def test_delayed_job_refuses_a_different_headset(self): + with mock.patch.object(server, "start_job", return_value={"job": "queued"}) as job: + self.assertEqual(server.export_clip({"id": IDS[0]}), {"job": "queued"}) + self.device = "frame-b" + with self.assertRaises(server.Failure) as error: + job.call_args.args[1]() + self.assertEqual(error.exception.status, 409) + self.assertEqual(self.exports, []) + + def test_local_and_headset_modes_reject_steam_clip_exports(self): + for headset in (False, True): + with self.subTest(headset=headset), mock.patch.object(server, "LOCAL", True), \ + mock.patch.object(server, "HEADSET_RUNTIME", headset): + for action in (server.list_clips, lambda: server.export_clip({"id": IDS[0]}), + lambda: server.auto_sync({"action": "now"})): + with self.assertRaises(server.Failure) as error: + action() + self.assertEqual(error.exception.status, 400) + self.assertEqual(self.exports, []) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_clips.py b/tests/test_clips.py index fba81b8..9d4be0f 100644 --- a/tests/test_clips.py +++ b/tests/test_clips.py @@ -71,6 +71,17 @@ class Ids(unittest.TestCase): with self.subTest(bad=bad), self.assertRaises(ValueError): clips.remove_command(bad) + def test_local_path_keeps_validated_identity_and_hashes_device_names(self): + folder = Path("exports") + target = clips.clip_local_path("123/clips/clip/video/session", "../../device:name", folder) + self.assertRegex(target.parts[1], r"^headset-[0-9a-f]{12}$") + self.assertEqual(target.parts[2:], ("123", "clips", "clip", "video", "session", "clip.mp4")) + for bad in (None, "", 42): + with self.subTest(device=bad), self.assertRaises(ValueError): + clips.clip_local_path("123/video/bg", bad, folder) + with self.assertRaises(ValueError): + clips.clip_local_path("../clips/clip", "frame-a", folder) + def test_recording_names_checked(self): for fn in (clips.finish_command, clips.cleanup_command, lambda n: clips.record_command(n, True)): for bad in ("rec-1", "rec-20261020-201950; rm -rf ~", "../rec-20261020-201950", "*"): @@ -156,10 +167,10 @@ class Sessions(unittest.TestCase): def test_listing_marks_what_is_already_here(self): here = self.dir / "here" - write(here / "clip_620_20261020_202134.mp4") + write(clips.clip_local_path("1/clips/clip_620_20261020_202134/video/bg", "frame-a", here)) frame = lambda args, timeout: {"clips": [{"id": "1/clips/clip_620_20261020_202134/video/bg"}, {"id": "1/video/bg_620_20261020_201950"}]} - got = clips.list_clips(frame, folder=here) + got = clips.list_clips(frame, folder=here, device_id="frame-a") self.assertEqual([c["saved"] for c in got["clips"]], [True, False]) self.assertEqual(got["folder"], str(here)) @@ -261,12 +272,16 @@ class ComputerSide(unittest.TestCase): frame = lambda args, timeout: calls.append(("frame", args)) or {"path": path, "size": 3} run = lambda cmd, timeout: calls.append(("run", cmd)) fetch = lambda remote, local: calls.append(("fetch", remote)) or Path(local).write_bytes(b"mp4") - got = clips.export_clip("1/clips/clip_620/video/bg", frame, run, fetch, folder=self.dir) + got = clips.export_clip("1/clips/clip_620/video/bg", frame, run, fetch, folder=self.dir, device_id="frame-a") self.assertEqual(got["message"], "Saved clip_620.mp4 to ~/Videos/SteamFrame") - self.assertEqual((self.dir / "clip_620.mp4").read_bytes(), b"mp4") + target = clips.clip_local_path("1/clips/clip_620/video/bg", "frame-a", self.dir) + self.assertEqual(got["file"], str(target)) + self.assertEqual(set(got), {"message", "file"}) + self.assertEqual(target.read_bytes(), b"mp4") self.assertEqual([c[0] for c in calls], ["frame", "fetch", "run"]) self.assertEqual(calls[2][1], clips.remove_command(path)) - self.assertEqual([p.name for p in self.dir.iterdir()], ["clip_620.mp4"]) + self.assertEqual(list(self.dir.rglob("*.mp4")), [target]) + self.assertFalse(list(self.dir.glob(".incoming-*"))) def test_failed_copy_leaves_no_partial_file_and_still_cleans_up(self): path = f"{clips.WORK}/export-0123456789abcdef/clip_620.mp4" @@ -277,20 +292,62 @@ class ComputerSide(unittest.TestCase): raise RuntimeError("connection lost") with self.assertRaisesRegex(RuntimeError, "connection lost"): clips.export_clip("1/clips/clip_620/video/bg", lambda a, t: {"path": path, "size": 4}, - lambda cmd, timeout: ran.append(cmd), fetch, folder=self.dir) + lambda cmd, timeout: ran.append(cmd), fetch, folder=self.dir, device_id="frame-a") self.assertEqual(list(self.dir.iterdir()), []) self.assertEqual(ran, [clips.remove_command(path)]) + def test_same_names_keep_each_account_headset_kind_and_nested_session(self): + first = "1/clips/clip_620/video/bg" + ids = [first, "2/clips/clip_620/video/bg", "1/clips/clip_620/video/other", + "1/clips/clip_620/other/bg", "1/video/clip_620"] + legacy = write(self.dir / "clip_620.mp4", b"legacy export") + remote = f"{clips.WORK}/export-0123456789abcdef/clip_620.mp4" + frame = lambda args, timeout: {"clips": [{"id": cid} for cid in ids]} + stored = {} + for device in ("frame-a", "frame-b"): + self.assertFalse(any(c["saved"] for c in clips.list_clips( + frame, folder=self.dir, device_id=device)["clips"])) + for index, cid in enumerate(ids): + payload = f"{device} session {index}".encode() + result = clips.export_clip(cid, lambda a, t: {"path": remote, "size": len(payload)}, + lambda c, t: None, lambda r, p: p.write_bytes(payload), + folder=self.dir, device_id=device) + stored[Path(result["file"])] = payload + listing = clips.list_clips(frame, folder=self.dir, device_id=device) + self.assertEqual([c["saved"] for c in listing["clips"]], + [n <= index for n in range(len(ids))]) + self.assertEqual(listing["folder"], str(self.dir)) + self.assertEqual(len(stored), len(ids) * 2) + for path, payload in stored.items(): + self.assertEqual(path.read_bytes(), payload) + self.assertEqual(legacy.read_bytes(), b"legacy export") + self.assertFalse(list(self.dir.glob(".incoming-*"))) + + def test_failed_reexport_preserves_completed_export(self): + cid = "1/clips/clip_620/video/bg" + target = write(clips.clip_local_path(cid, "frame-a", self.dir), b"completed") + remote = f"{clips.WORK}/export-0123456789abcdef/clip_620.mp4" + + def fetch(remote, local): + local.write_bytes(b"partial") + raise RuntimeError("connection lost") + + with self.assertRaisesRegex(RuntimeError, "connection lost"): + clips.export_clip(cid, lambda a, t: {"path": remote}, lambda c, t: None, fetch, + folder=self.dir, device_id="frame-a") + self.assertEqual(target.read_bytes(), b"completed") + self.assertFalse(list(self.dir.glob(".incoming-*"))) + def test_bad_id_never_reaches_the_frame(self): never = mock.Mock(side_effect=AssertionError("reached the Frame")) with self.assertRaises(ValueError): - clips.export_clip("1/video/../../.ssh", never, never, never, folder=self.dir) + clips.export_clip("1/video/../../.ssh", never, never, never, folder=self.dir, device_id="frame-a") def test_unexpected_path_from_the_frame_refused(self): never = mock.Mock(side_effect=AssertionError("deleted or copied")) with self.assertRaises(ValueError): clips.export_clip("1/video/bg", lambda a, t: {"path": ".ssh/id_ed25519", "size": 1}, never, never, - folder=self.dir) + folder=self.dir, device_id="frame-a") class Commands(unittest.TestCase): diff --git a/ui/frame_clips.py b/ui/frame_clips.py index 748443e..6ecdee5 100644 --- a/ui/frame_clips.py +++ b/ui/frame_clips.py @@ -26,6 +26,7 @@ that stream. The folder names are as users and Steam's own files describe them; listing doesn't depend on them: any folder under clips/ or video/ (up to four levels) holding init-stream*.m4s counts as a session. """ +import hashlib import json import os import re @@ -363,10 +364,20 @@ def parse_id(clip_id): def export_name(clip_id): - """The .mp4's name on the computer: the clip's (or background session's) own folder name.""" + """The .mp4's basename; it alone does not identify a saved export.""" return parse_id(clip_id)[2] + ".mp4" +def clip_local_path(clip_id, device_id, folder=None): + """Keep the headset, account and full Steam session identity in the destination.""" + parts = parse_id(clip_id) + if not isinstance(device_id, str) or not device_id: + raise ValueError("missing clip headset identity") + headset = hashlib.sha256(device_id.encode("utf-8")).hexdigest()[:12] + folder = Path(folder) if folder else VIDEOS_DIR + return folder / f"headset-{headset}" / Path(*parts) / export_name(clip_id) + + def read_session(folder): """{stream number: [init, chunk, chunk...]} for one session folder, chunks in number order. A stream without its init segment or without chunks can't be played, so it's left out.""" @@ -552,19 +563,20 @@ def remove_command(path): # run_frame(args, timeout) runs this file on the Frame with `args` and returns its JSON; # run(command, timeout) and fetch(remote, local) as for Recorder. -def list_clips(run_frame, folder=None): +def list_clips(run_frame, folder=None, *, device_id): """Steam's recordings on the Frame, each marked saved if its .mp4 is already here.""" folder = Path(folder) if folder else VIDEOS_DIR clips = run_frame(["list"], 60)["clips"] for c in clips: - c["saved"] = (folder / export_name(c["id"])).exists() + c["saved"] = clip_local_path(c["id"], device_id, folder).is_file() return {"clips": clips, "folder": str(folder)} -def export_clip(clip_id, run_frame, run, fetch, folder=None): +def export_clip(clip_id, run_frame, run, fetch, folder=None, *, device_id): """Stitch a Steam recording on the Frame and copy the .mp4 here. -> {"message", "file"}.""" folder = Path(folder) if folder else VIDEOS_DIR - name = export_name(clip_id) # checks the id before anything reaches the Frame + target = clip_local_path(clip_id, device_id, folder) # validate before reaching the Frame + name = target.name made = run_frame(["export", clip_id], 1900) path = made.get("path") remove = remove_command(path) # refuses a path that isn't one of our exports @@ -573,7 +585,8 @@ def export_clip(clip_id, run_frame, run, fetch, folder=None): incoming = Path(tempfile.mkdtemp(prefix=".incoming-", dir=folder)) try: fetch(path, incoming / name) - os.replace(incoming / name, folder / name) + target.parent.mkdir(parents=True, exist_ok=True) + os.replace(incoming / name, target) finally: shutil.rmtree(incoming, ignore_errors=True) finally: @@ -581,7 +594,7 @@ def export_clip(clip_id, run_frame, run, fetch, folder=None): run(remove, 30) except Exception: pass # the next export sweeps it - return {"message": f"Saved {name} to ~/Videos/SteamFrame", "file": str(folder / name)} + return {"message": f"Saved {name} to ~/Videos/SteamFrame", "file": str(target)} # ---- on the Frame ----------------------------------------------------------------- diff --git a/ui/server.py b/ui/server.py index 09f2f83..d10b7e4 100755 --- a/ui/server.py +++ b/ui/server.py @@ -916,7 +916,9 @@ def _clips_frame(args, timeout): def list_clips(): _clips_here() try: - return frame_clips.list_clips(_clips_frame) + device = agent_target()["id"] + with working(device): + return frame_clips.list_clips(_clips_frame, device_id=device) except frame_clips.ClipError as e: raise Failure(str(e), e.status) @@ -926,11 +928,13 @@ def export_clip(body): _clips_here() clip_id = body.get("id") name = frame_clips.export_name(clip_id) # ValueError, so 400, for a bad id + device = agent_target()["id"] def work(): try: - return frame_clips.export_clip(clip_id, _clips_frame, lambda cmd, timeout: ssh(cmd, timeout=timeout), - fetch_file) + with working(device): + return frame_clips.export_clip(clip_id, _clips_frame, lambda cmd, timeout: ssh(cmd, timeout=timeout), + fetch_file, device_id=device) except frame_clips.ClipError as e: raise Failure(str(e), e.status) return start_job(f"Export {name}", work) @@ -953,7 +957,7 @@ def _sync_shots(device, ids): def _sync_clip(device, clip_id): with working(device): return frame_clips.export_clip(clip_id, _clips_frame, lambda cmd, timeout: ssh(cmd, timeout=timeout), - fetch_file) + fetch_file, device_id=device) def _sync_list(listing):