feat: streaming library sync, memory cards, and save state history - #3856
Open
LoneAngelFayt wants to merge 18 commits into
Open
feat: streaming library sync, memory cards, and save state history#3856LoneAngelFayt wants to merge 18 commits into
LoneAngelFayt wants to merge 18 commits into
Conversation
Extend the streaming session layer with library-backed asset sync and per-user memory cards. State library sync: manual and autosave states are pulled from the container into the user's library, with PCSX2 screenshots extracted from the .p2s archive as state thumbnails. A session can be claimed with a state id to resume directly from a library state. Save file sync: save files are hydrated into the container at claim and pulled back on release, deduplicated by content hash. Memory cards: a new per-user, per-emulator whole-card model with version history (memory_cards, memory_card_versions). Opt-in per platform, currently PCSX2 only. Cards are hydrated at claim (wipe-then-replace) and evacuated before the session is released. Playtime: session start and end are ingested as play sessions, skipping runs shorter than five seconds. Session hardening: heartbeats mark liveness, and a session whose heartbeat has gone stale can be taken over after the previous container is torn down. Frontend: resume-from-state picker, memory card picker and manager, asset previews, and an admin streaming panel on the activity view.
…session An admin ending a session from the activity panel used to leave the player's tab stranded on a dead stream with no indication anything had happened. The player's tab now learns of this near-instantly via a socket push to a per-user room, with a slower REST poll (30s) as a fallback for a dropped event or a throttled background tab, exits fullscreen, and shows a dialog naming the admin and the reason given before returning to the game's details page. Backend: a 15-minute Redis tombstone records who ended the session and why, read by both the new side-effect-free status endpoint and the existing heartbeat, and pushed live over Socket.IO the moment it is written. The self-release guard is corrected to key off whether a reason was supplied rather than off user identity, since an admin can be logged into the same account they are evicting. The admin panel's release action now prompts for an optional reason that is shown to the displaced player alongside the admin's name.
Pulled states overwrote the slot they came from, so a player who saves often could only ever resume from the last capture in each slot. Every capture is now its own library asset, and the resume picker is the way back to any earlier point. The container filename encodes the emulator slot, so the library name and the container name have to diverge. A capture stamp is inserted directly before the slot token, which keeps both slot patterns matching at the end of the name: states written before this resolve unchanged, and the container-side name is recovered by dropping the stamp. Retention is capped per ROM, emulator and user by STREAMING_STATE_HISTORY_LIMIT (default 50), pruning oldest first, and a capture identical to the previous one is dropped rather than taking a slot in the history. Hydration now pushes only the newest state: every history entry collapses to the same container-side name, so pushing more would just overwrite in place. A resume pick already sent at claim time suppresses the push entirely, since it would land before the broker's deferred load fires. With the library holding the history, slots stop being a user-facing concept and become the register the emulator writes through. The player pins to the autosave slot, which is also what hydration targets and what save-and-exit already used, so in-game quick-load lands on the same file the picker last sent down. That made the slot picker and the separate load-autosave button redundant, and required save-state to accept the autosave slot: the two coarse slot bounds collapse into one, and _assert_valid_slot no longer takes an allow_autosave flag. Frontend: the state strip gains grid and list layouts for histories the horizontal row buries, persisted per user. The pre-game screen is one layout instead of two, with the resume panel always present so a first run and a long history read as the same screen, and the memory card picker split into its own section. Dolphin inherits all of this. xemu does not: it has no slot convention to place a stamp against, so its captures still update in place.
PCSX2 embeds a PNG inside every .p2s savestate, so a pulled state gets its resume-picker thumbnail for free. Dolphin savestates carry no frame, so its broker captures one at save time and serves it from GET /state-screenshot. Move the sourcing decision out of _store_state_asset and into _pull_state_to_library, so extract-or-fetch is resolved in one place and the image is passed down. A 404 from the broker is the normal "this one captures no frames" answer, so it is not logged. Also drop the PCSX2-only wording from the memory-card comments, since Dolphin now shares those paths.
Neither screenshot source validated what it handed over. The zip path trusted an entry that only claims to be a PNG by name, and the broker path trusted whatever came back over HTTP, so an error page from a proxy in front of a broker would have been written into the user's screenshots directory and bound to a state as its thumbnail. Check the magic bytes in _store_state_screenshot rather than at each source, since both feed that one function.
The seven hand-rolled urllib blocks for state files, save archives and memory cards become three primitives: a raising binary GET, a best-effort GET that swallows the routine 404, and a best-effort PUT. The nosec annotation and the secret header now live in one place each.
Contributor
Author
|
migrations might need a look through, some of this work is older and we've moved past the current numbers, something we may have to adjust at point of merge |
LoneAngelFayt
marked this pull request as ready for review
July 24, 2026 05:26
Contributor
Author
|
@gantoine This should be ready I updated the migrations so they should they should be clean this would be great to get into 5.1.0 to make the streaming feel more feature complete instead of a tech demo lol |
gantoine
removed
the
on-hold
gantoine
self-requested a review
July 24, 2026 11:45
Member
Contributor
Greptile SummaryThis PR substantially extends native streaming persistence and lifecycle management.
Confidence Score: 2/5The PR is not safe to merge until release errors relinquish session claims, interrupted adoption can recover, and public cards can be selected for streaming. Detached teardown leaves Redis claims intact after intermediate failures, separately committed adoption state can trap retries in a permanent 502 response, and the claim picker plus backend ownership check make public cards unreachable. backend/endpoints/streaming.py, frontend/src/v2/components/Player/MemoryCardPicker.vue, backend/endpoints/memory_cards.py Important Files Changed
Prompt To Fix All With AIFix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
backend/endpoints/streaming.py:2577-2578
**Failed teardown retains session claim**
When broker shutdown, card evacuation, wiping, or notification raises during detached release, this handler only logs the exception and skips the Redis deletion at line 2564. The API has already reported a successful release, but subsequent players continue to see the container as occupied until stale takeover or the six-hour TTL clears it.
### Issue 2 of 3
frontend/src/v2/components/Player/MemoryCardPicker.vue:53-54
**Public cards remain unselectable**
When another user publishes a card for this emulator, the claim picker calls the owner-only listing instead of `getSharedMemoryCards`, so the public card never appears. Supplying its ID directly also fails the backend ownership check, leaving the new sharing endpoint and visibility control unusable for streaming.
### Issue 3 of 3
backend/endpoints/streaming.py:2179-2186
**Adoption retry rejects stored card**
When the card version was committed but recording the separate adoption outcome failed, the next claim reads the same container card and deduplication finds the existing version. This branch treats that idempotent result as an import failure, deletes the claim, and returns 502 on every retry instead of recording the outstanding adoption decision.
Reviews (1): Last reviewed commit: "Merge remote-tracking branch 'upstream/m..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+2577
to
+2578
| except Exception: | ||
| log.exception("session teardown failed, platform=%s", platform) |
Contributor
There was a problem hiding this comment.
When broker shutdown, card evacuation, wiping, or notification raises during detached release, this handler only logs the exception and skips the Redis deletion at line 2564. The API has already reported a successful release, but subsequent players continue to see the container as occupied until stale takeover or the six-hour TTL clears it.
Knowledge Base Used: Streaming and Emulation
Prompt To Fix With AI
This is a comment left during a code review.
Path: backend/endpoints/streaming.py
Line: 2577-2578
Comment:
**Failed teardown retains session claim**
When broker shutdown, card evacuation, wiping, or notification raises during detached release, this handler only logs the exception and skips the Redis deletion at line 2564. The API has already reported a successful release, but subsequent players continue to see the container as occupied until stale takeover or the six-hour TTL clears it.
**Knowledge Base Used:** [Streaming and Emulation](https://app.greptile.com/romm/-/custom-context/knowledge-base/rommapp/romm/-/docs/streaming-and-emulation.md)
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+53
to
+54
| const { data } = await memoryCardApi.getMemoryCards({ emulator }); | ||
| cards.value = data; |
Contributor
There was a problem hiding this comment.
When another user publishes a card for this emulator, the claim picker calls the owner-only listing instead of getSharedMemoryCards, so the public card never appears. Supplying its ID directly also fails the backend ownership check, leaving the new sharing endpoint and visibility control unusable for streaming.
Knowledge Base Used:
Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/src/v2/components/Player/MemoryCardPicker.vue
Line: 53-54
Comment:
**Public cards remain unselectable**
When another user publishes a card for this emulator, the claim picker calls the owner-only listing instead of `getSharedMemoryCards`, so the public card never appears. Supplying its ID directly also fails the backend ownership check, leaving the new sharing endpoint and visibility control unusable for streaming.
**Knowledge Base Used:**
- [Streaming and Emulation](https://app.greptile.com/romm/-/custom-context/knowledge-base/rommapp/romm/-/docs/streaming-and-emulation.md)
- [Frontend App Structure (v1)](https://app.greptile.com/romm/-/custom-context/knowledge-base/rommapp/romm/-/docs/frontend-app.md)
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+2179
to
+2186
| log.error( | ||
| "adopted memory card matched an existing version of card %d", | ||
| memory_card.id, | ||
| ) | ||
| await async_cache.delete(_session_redis_key(session_key)) | ||
| if created_blank_card_id is not None: | ||
| db_memory_card_handler.delete_card(created_blank_card_id) | ||
| raise HTTPException(status_code=502, detail=_CARD_IMPORT_FAILED_DETAIL) |
Contributor
There was a problem hiding this comment.
When the card version was committed but recording the separate adoption outcome failed, the next claim reads the same container card and deduplication finds the existing version. This branch treats that idempotent result as an import failure, deletes the claim, and returns 502 on every retry instead of recording the outstanding adoption decision.
Knowledge Base Used:
Prompt To Fix With AI
This is a comment left during a code review.
Path: backend/endpoints/streaming.py
Line: 2179-2186
Comment:
**Adoption retry rejects stored card**
When the card version was committed but recording the separate adoption outcome failed, the next claim reads the same container card and deduplication finds the existing version. This branch treats that idempotent result as an import failure, deletes the claim, and returns 502 on every retry instead of recording the outstanding adoption decision.
**Knowledge Base Used:**
- [Streaming and Emulation](https://app.greptile.com/romm/-/custom-context/knowledge-base/rommapp/romm/-/docs/streaming-and-emulation.md)
- [Database Models and Migrations](https://app.greptile.com/romm/-/custom-context/knowledge-base/rommapp/romm/-/docs/database-models.md)
How can I resolve this? If you propose a fix, please make it concise.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Follow-up to #3211.
Closes #3968: the streaming container is now gated by a broker-minted, session-bound stream token. The broker mints the token at launch, RomM carries it in the claim host URL, and the container's nginx vhost enforces it on every request (document, assets, and the WebSocket upgrade) via
auth_request. The RomM-side half of that gate (carrying the token through to the iframe host URL) lands in this PR.Library sync, memory cards, session hardening
Save states and save files now sync between the container and the user's library, so a session can be resumed from any machine. State thumbnails come from whichever source the emulator offers: PCSX2 embeds a frame in the
.p2sarchive, while Dolphin's broker captures one alongside the save and serves it from/state-screenshot. Adds per-user memory cards with version history (opt-in per platform, PCSX2 and GameCube), hydrated at claim and evacuated at release. Sessions get heartbeats, so a stale one can be taken over instead of wedging the container.Importing a card that's already there
Turning on
memory_card_syncused to wipe whatever the container was already holding on the first claim. The first claim now stops and asks: adopt that card as version one, or start clean. The answer is recorded per user and container, so it's asked once. The flag is also ignored, with a warning, on platforms that have no memory card (Wii, Wii U, Xbox), where whole-card sync would have quietly replaced the per-file save sync those platforms actually rely on.Force-release notifications
Ending someone's session from the admin panel used to leave their tab on a dead stream with no explanation. They now get a dialog naming the admin and the reason, then go back to the game's details page.
Save state history
Pulled states used to overwrite the slot they came from, so saving often meant only the newest capture per slot survived. Every capture is now its own library asset. Retention is capped by
STREAMING_STATE_HISTORY_LIMIT(default 50 per ROM/emulator/user) and identical back-to-back captures are dropped.Since the library holds the history, slots stop being a user-facing concept and become just the register the emulator writes through, so the slot picker and load-autosave button are gone. Older states resolve unchanged, no migration needed.
Dolphin and xemu both get this. xemu's snapshots do carry a slot in the filename, so timestamps hang off it the same way.
Known follow-up: the state list grows to ~50 per user per emulator, so the picker wants pagination. Not in this PR.
AI assistance
Written with substantial AI assistance (Claude Code). Design, review, and end-to-end testing against real PCSX2 and Dolphin containers were mine; the implementation was largely AI-authored under that direction and read through before committing.
Checklist