From f81f70ed5d759fc15e2d7def08f8f0bdac8da9eb Mon Sep 17 00:00:00 2001 From: saphid <4596216+saphid@users.noreply.github.com> Date: Tue, 29 Sep 2026 09:22:22 +1000 Subject: [PATCH] Fix media stop, upload cleanup and review findings - Stop is a no-op when the collected player unit is already gone (raw systemctl stop exits 5 on the Frame; verified 2026-09-29). - Surface systemd-run stderr when the player can't start. - Keep the copy error if the cleanup ssh also fails; reject upload names that the play path can never accept. - Allow 60 s for play (ffprobe 30 s + systemd-run 15 s remote). - Docs: four-hour cap is unconditional; no delete action yet; fix a garbled timing sentence. Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/vr-video.md | 11 ++++++----- tests/test_media.py | 19 ++++++++++++++++++- ui/frame_media_remote.py | 8 ++++++-- ui/server.py | 14 ++++++++++---- 4 files changed, 40 insertions(+), 12 deletions(-) diff --git a/docs/vr-video.md b/docs/vr-video.md index 8da4563..1e242a6 100644 --- a/docs/vr-video.md +++ b/docs/vr-video.md @@ -53,9 +53,9 @@ All test media were generated by us. No paid content or DRM was involved. test in 0.128 s (decode only); converting all frames to RGBA took 0.242 s. - Our ffmpeg → Python → OpenVR path submitted all 150 frames and exited 0. The 1280×720 prototype took 4.775 s; the full 1920×1080 player took - 4.618 s. These are wall times, not in-headset frame-rate measurements; - These initial probes exposed an early finish. The final player explicitly - paces 30 fps output: all 150 frames then completed in **5.025 s**. + 4.618 s. These are wall times, not in-headset frame-rate measurements. + Finishing a 5 s clip early showed those probes weren't paced, so the final + player paces output at 30 fps: all 150 frames then completed in **5.025 s**. - The H.265 hardware decoder also completed the generated 1080p clip (60 frames, exit 0); this is a short compatibility check, not a 4K/8K benchmark. - Our actual player, launched through Frame Control's media API, displayed @@ -101,8 +101,9 @@ The ffmpeg path above completed instead. version rejects files above 20,000 records. It is a stereo preview, not an immersive walk-through. - A single player can run at a time. It stops at EOF, on **Stop**, or after - four hours if the computer disconnects. Still images remain until Stop - or that timeout. The log is + four hours in any case, so longer movies are cut off. Still images remain + until Stop or that timeout. There is no delete action yet: remove old + uploads from `~/Videos/FrameControl/` over SSH. The log is `~/.local/share/frame-control/media/player.log` on the Frame. - **Panel/stream theatre remains outside this media slice:** SteamVR's `vrcmd --dock-overlay` accepts `theater`, but docking an existing panel diff --git a/tests/test_media.py b/tests/test_media.py index 3434b70..29ea4a9 100644 --- a/tests/test_media.py +++ b/tests/test_media.py @@ -83,9 +83,26 @@ class Media(unittest.TestCase): ssh.assert_not_called() def test_fake_frame_stop_only_owns_our_unit(self): - with patch.object(remote.subprocess, 'run') as run, patch.object(remote, 'status', return_value={}): + with patch.object(remote.subprocess, 'run') as run, patch.object(remote, 'status', return_value={}), \ + patch.object(remote, 'active', return_value=True): remote.run({'action': 'stop'}) self.assertEqual(run.call_args.args[0], ['systemctl', '--user', 'stop', 'frame-control-media.service']) + # A finished player is already collected; Stop is then a no-op, not an error. + with patch.object(remote.subprocess, 'run') as run, patch.object(remote, 'status', return_value={}), \ + patch.object(remote, 'active', return_value=False): + remote.run({'action': 'stop'}) + run.assert_not_called() + + def test_upload_rejects_unplayable_names_and_keeps_copy_error(self): + with patch.object(server, 'ssh') as ssh, patch.object(server, 'push_file') as push: + for name in ('.hidden.mp4', 'a\\b_SBS.mp4'): + with self.assertRaises(server.Failure): + server.push_media(Path('/tmp')/name) + ssh.assert_not_called() + push.side_effect = server.Failure('copy failed') + ssh.side_effect = [None, server.Failure('link down')] + with self.assertRaisesRegex(server.Failure, 'copy failed'): + server.push_media(Path('/tmp/film_SBS.mp4')) def test_splat_invalid_records_and_stereo_parallax(self): with tempfile.TemporaryDirectory() as d: diff --git a/ui/frame_media_remote.py b/ui/frame_media_remote.py index acd7b90..b60b295 100644 --- a/ui/frame_media_remote.py +++ b/ui/frame_media_remote.py @@ -63,7 +63,9 @@ def run(body): if action == 'status': return status() if action == 'stop': - subprocess.run(['systemctl', '--user', 'stop', UNIT], check=True, timeout=15) + # --collect unloads the unit after it exits; stopping it then is a no-op, not an error. + if active(): + subprocess.run(['systemctl', '--user', 'stop', UNIT], check=True, timeout=15) return {'message': 'Media player stopped', **status()} if action != 'play': raise ValueError('Media action must be list, status, play or stop') @@ -87,7 +89,9 @@ def run(body): '--layout', plan['layout'], '--status', str(STATUS)] if body.get('theatre'): command.append('--theatre') - subprocess.run(command, check=True, capture_output=True, text=True, timeout=15) + started = subprocess.run(command, capture_output=True, text=True, timeout=15) + if started.returncode: + raise RuntimeError('Could not start the media player: ' + (started.stderr.strip() or 'systemd-run exited %s' % started.returncode)) return {'message': 'Starting Frame Control media', 'plan': plan} diff --git a/ui/server.py b/ui/server.py index 59353e5..6109d39 100755 --- a/ui/server.py +++ b/ui/server.py @@ -1262,7 +1262,7 @@ for name, source in json.load(sys.stdin).items(): ssh("python3 -c " + shlex.quote(installer), stdin=json.dumps(sources)) try: out = ssh("python3 ~/.local/share/frame-control/media/frame_media_remote.py", - stdin=json.dumps(body), timeout=45) + stdin=json.dumps(body), timeout=60) except Failure as e: for line in reversed(getattr(e, "stdout", "").splitlines()): try: @@ -1278,17 +1278,23 @@ for name, source in json.load(sys.stdin).items(): def push_media(path): # Validate the format, but leave layout selection until playback (ffprobe # can then read metadata on the Frame, where it is installed). - frame_media.plan(Path(path).name, "mono") + name = Path(path).name + frame_media.plan(name, "mono") + if name.startswith(".") or "\\" in name: + raise Failure("Rename the file: media names can't start with a dot or contain a backslash", 400) token = secrets.token_hex(16) dest = "Videos/FrameControl/" + token + "/" ssh("mkdir -p ~/" + dest) try: push_file(path, dest) except Exception: - ssh("rm -rf ~/" + dest) + try: + ssh("rm -rf ~/" + dest) + except Failure: + pass # keep the copy error; an empty folder isn't listed as media raise return {"message": "Media sent. Choose its layout and press Play.", - "id": token + "/" + Path(path).name} + "id": token + "/" + name} POST = {"/api/media": media, "/api/android/display": android_display, "/api/android": android, "/api/titles": titles, "/api/launch": launch, "/api/steam": steam, "/api/volume": set_volume, "/api/clipboard": clipboard,