Skip to content

feat(bridge): share play frame ring, guest pad and host override - #29

Open
AlxcBTW wants to merge 13 commits into
mainfrom
feat/share-play-bridge
Open

AlxcBTW wants to merge 13 commits into
mainfrom
feat/share-play-bridge

Conversation

@AlxcBTW

@AlxcBTW AlxcBTW commented Oct 27, 2026 •

Copy link
Copy Markdown
Member

What

Engine side of share play's bridge group 0x05xx (spec), for all games, off unless the net service asks for it over the bridge connection.

  • libSceNetCtl: kinds 0x0501 to 0x0506. share open maps the ring a share worker created (name anyplay-..., session-local on Windows, shm_open on POSIX), validates header, size and ring_bytes and answers 0x8501 (0 mapped, 1 cannot open or validate, 2 already running). share close, share mode and a lost net service connection neutralise and disconnect the guest pad in the same step. share pad is accepted only in hand_over (slot 0) and together (slot 1) and drops states with bits 15 to 31 set. override and a once-a-second share status go to the service. ShareRing.hpp is the producer (slot sequence protocol, overwrite the oldest slot, CPU letterbox, audio ring).
  • libScePad: the guest is pad 1 in hand_over and a connected pad 2 in together (buttons, sticks, triggers only), read under the existing pad lock and falling back to the host's input when no share runs.
  • libSceVideoOut: select (View or touchpad button) plus Options held for one second, and the new StopShare key (F8), call NetBridgeShareOverride: the guest pad is dropped at once and override 1 (took control back) or 2 (stop) is sent.
  • libSceAudioOut: AudioOut2's stereo 48 kHz mix goes to the ring's audio part while a share runs.
  • libSceAgcDriver: while a share runs, a presented frame is read back like a frame dump and copied into the ring when its slot's fence signals (VulkanDevice.cpp, a dumpShare flag on the existing readback path; nothing runs without a share).

Tested

Not tested locally (C++ is built by CI only). Windows: gh workflow run build.yml -f pr=29 -f linux=false (the linux job runs on the self-hosted runner). New tests: net_bridge_share (fake net service: reply codes, mode and slot gating, bit validation, neutral on mode change, close and connection loss, ring slots and audio ring through a real OS mapping, status, override), bridge_protocol (wire vectors), pad_share (guest pad in the Pad library), pad_share_chord (timing).

Not done and listed in the debt files: the older AudioOut ports are not in the sound (only AudioOut2's mix is), a second game-visible user for together, HDR10 output is not shared, and nothing measured on a GPU.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file (gaps are under Functional in the debt files)
  • No notes, investigation .md files or images added to the repository
  • New third-party code is a submodule built from source (none)
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Fixes: none
  • Unblocks: none (anyplaynet board task T38)
  • GPUs tested: none; CI has no GPU, so the readback hook in VulkanDevice.cpp needs an engine owner's GPU run
  • AI-assisted: yes

Adds the bridge kinds 0x0501 to 0x0506 of the share play spec: share open maps
the ring a share worker created and answers 0x8501, share close and mode
neutralise the guest pad in the same step, share pad is gated by mode, slot and
button bits, override and status go the other way. A frame sink on the existing
presentation readback fills the ring, libScePad takes the guest as pad 1
(hand_over) or pad 2 (together), and select plus start held for a second or the
StopShare key (F8) take control back at once.
@AlxcBTW
AlxcBTW requested review from TideOPL and parl0v October 27, 2026 00:24

@parl0v parl0v left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I read the whole diff against the share play spec. The ring and bridge side is in good shape. Header validation can't overflow, and the geometry is copied once at attach, so a producer changing the header later can't move the writes. The unmap is serialised with the copies, and close, mode change, override and connection loss all neutralise the guest pad in one step. With no share running, the cost is one atomic load per present and per pad read. I hand-checked the share pad and share open vectors, and both are byte-exact.

Changes I'd like before merge:

  1. The gating tests don't test the gating. In tests/NetBridgeShare.cpp:196-200 (and :212-214 for together), the wrong-slot and bits-15-31 states are followed by a valid state, and the wait only checks the last one. If I delete the slot check or the button mask, the test still passes. The watch check at :185-190 passes whatever Pad() stores, because NetBridgeSharePad returns 0 in watch anyway. Every accepted state bumps serial, so one way to fix it: record it, send the invalid states plus one valid state, and require exactly +1.
  2. One mutex for the frame copy and the pad state. NetBridgeShareSubmitFrame holds state.mutex through the whole CPU letterbox, and NetBridgeSharePad takes the same mutex from inside libScePad's stateMutex. So in hand_over and together, a game's pad read waits for a full-frame copy, and Pad() and the override queue behind it too. There's no deadlock, since the order is always pad then share, but I'd split it into a ring mutex and a pad mutex.
  3. Scaling cost on the presenter thread. ShareRing.hpp:145 does a 64-bit divide per destination pixel for sourceColumn, under presentMutex, and the readback is at the display buffer's size. A 4K title copies about 33 MB a frame before scaling. Precomputing the column offsets once per size pair removes the divides. If my run shows it's still over the 1.0 ms budget, the next step would be a GPU blit to the ring size (RecordBlitInto, as the direct path already does).
  4. The ring layout isn't pinned to the spec. Every layout check reads through the same ShareRing offsets the producer writes with, so a wrong offset passes. One test with literal numbers would pin it. For the 16x8 test ring (stride 64, 64 audio frames) I get 2208 bytes, slots at 64/608/1152, audio at 1696, audio position at 40, slot fields at +0/+8/+16/+20, and magic 41 4E 59 53 48 52 30 30 at 0.

Smaller things, fine as follow-ups:

  • present_us is the retire time and frame counts shared frames. slot.submittedAt already exists for the first, and the flip count would match the spec for the second.
  • dropped also counts frames skipped by the rate cap, so a 144 fps game at a 60 cap reports about 84 a second. I'd count only ring-busy and refused frames.
  • POSIX: after fstat, check st_uid == geteuid() and (st_mode & 077) == 0, since the spec says only the game's user can read the mapping. On Windows I'd write the Local\ prefix explicitly.
  • A short share open payload answers 1, while the bridge protocol says unsupported. F8 always sends stop, while the spec's stop-key row says "as the chord". Code or spec should change in both cases.
  • ShareOverrideChord.hpp:13 uses the literals 0x100000 and 0x0008; PadButton::TouchPad and PadButton::Options would be clearer. pad_share_chord tests the timer, not the wiring in PadInput.cpp:313.
  • The new entries in libSceAgcDriver.md and libScePad.md start second ### Functional headings. They belong under the existing ones.
  • BridgeProtocol.cpp still prints "19 frame kinds". Fixed vectors for 0x8501, 0x0505 and 0x0506 would cover the bridge-to-service direction too.

The GPU part isn't verified yet. The VideoOut chord and the AGC readback need a real run, and I still owe it. It will come from my box as soon as it's free, with frame-time numbers against the 1.0 ms budget. Until then, the conventions failure on the GPU checkbox is expected.

@parl0v

parl0v commented Oct 27, 2026

Copy link
Copy Markdown
Member

One more, from the dispatched Windows build (run 40426204600, refs/pull/29/head): it's red because of this PR. 2421 of 2422 tests pass, the four new ones included, but loadexec_arguments fails with GetLastError: 126 - The specified module could not be found on guest_loadexec_host.prx. libSceAgcDriver now links libSceNetCtl (libSceAgcDriver/CMakeLists.txt), and that test copies only the libraries listed in libSceSystemService/CMakeLists.txt:25 next to the relinked title, so libSceNetCtl.dll is missing there. The quick fix is to add $<TARGET_FILE:libSceNetCtl> to that list. I'd rather the graphics driver didn't link the network library at all, though, for example by having NetCtl register a frame sink that AGC calls, which would also keep the layering clean. The same question applies to libScePad and libSceVideoOut. Linux was still queued, but I'd expect it to fail the same way.

libSceAgcDriver now imports libSceNetCtl, so the loadexec and guest memory
lifetime tests, which copy an explicit set of libraries, carry it too.
@AlxcBTW
AlxcBTW requested a review from parl0v October 27, 2026 01:14
macOS allows 31 characters for a shared memory name including its slash, so
every platform refuses longer ring names alike.
…re locks, precompute the scaling columns

- Gating tests record the pad serial and require exactly one accepted state after the invalid ones (wrong slot, bits above 14, short state), in hand_over, together and watch.
- The ring layout is pinned to the spec with literal offsets (2208 bytes for the 16x8 test ring).
- The frame copy, the audio copy and the guest pad have their own locks, so a pad read never waits for a frame copy and a frame copy never costs an audio block.
- The letterbox computes the source column offsets once per pair of sizes.
- present_us is the submission time and frame the presentation count; dropped counts only refused and busy frames.
- POSIX ring owner and mode check, explicit Local\ prefix on Windows, a short share open answers unsupported.
- Fixed vectors for 0x8501, 0x0505 and 0x0506, chord wiring test, debt headings.
@AlxcBTW

AlxcBTW commented Oct 27, 2026

Copy link
Copy Markdown
Member Author

Thanks, all four are done in b93ad39. Windows build (build.yml, dispatch for #29 on b93ad39 merged into main) is green; the Linux job was skipped (-f linux=false).

  1. Gating tests. The tests now read the pad serial before the invalid states (wrong slot, bits above 14, a short state, in hand_over and together) and require exactly +1 once the valid last state shows. Watch is checked through the two mode changes (a state accepted in watch would make it +3). I ran NetBridgeShare standalone with g++ on the fake service (no game): it passes, and with the button mask removed, with the slot check removed, and with the watch check removed, each fails on the intended assertion.
  2. Locks. Split, but into three: padMutex (guest pad, mode, sender), ringMutex (frame copy) and audioMutex. A frame copy under one mutex also made the audio try-lock fail, which dropped audio blocks, so audio got its own. A pad read takes only padMutex. Open and Close take ring then audio, nothing else nests.
  3. Scaling. The source column offsets are computed once per pair of sizes (ColumnTable); no divide per pixel. A test compares the output with the per-pixel formula for two size pairs in a row. CPU path kept; the GPU blit (RecordBlitInto) is the next step in the libSceAgcDriver debt entry, if your run misses the 1.0 ms budget.
  4. Layout test. Literal numbers from your list, checked against spec/share-play.md (offsets, header size, 3 slots of 544 bytes, audio at 1696, total 2208): header bytes, a header built by hand attaches at exactly 2208 (2207 refused), slots at 64/608/1152 with fields at +0/+8/+16/+20/+32, audio position at 40, audio data at 1696 including the wrap.

Follow-ups:

  • present_us is now slot.submittedAt; frame is the count of presented pictures. Network#118 stamps pictures with the worker's own clock, so present_us is informational only; spec updated in docs(share-play): the stop key stops in every mode network#122.
  • dropped counts only ring-busy and refused frames, not the rate cap.
  • POSIX: st_uid == geteuid() and (st_mode & 077) == 0 after fstat; Windows opens Local\ + name.
  • A short share open now answers unsupported (tested).
  • F8: I changed the spec, not the code. A key named stop should stop in every mode; the chord steps back to watch first. (network#122)
  • Chord uses a static_assert against PadButton::TouchPad / Options in PadInput.cpp, and the pad-loop decision moved into ShareOverrideChord::Advance(active, controller, buttons, now), which the test covers.
  • Debt entries moved under the existing ### Functional headings.
  • "19 frame kinds" text dropped; fixed byte vectors added for 0x8501, 0x0505 and 0x0506.

Still open on your side: the GPU run (VideoOut chord, AGC readback, frame-time numbers). Not merging; over to you.

@AlxcBTW

AlxcBTW commented Oct 27, 2026

Copy link
Copy Markdown
Member Author

Addendum: #30 adds a guest user (UserServiceSetGuestUser_nid_postfix / UserServiceClearGuestUser_nid_postfix; pad port N is user index N). #29 does not depend on it. There is a TODO at the together mode switch in NetBridgeShare.cpp and a sentence in the libScePad debt entry: entering together sets user 1, leaving it, closing and a lost connection clear it, and whichever PR merges second wires that. The guest's display name is not in the bridge messages yet. Proposed follow-up (not in #29): a guest_name field on share mode, the smallest spec change. The network macOS check on boykopovar#122 failed in an unrelated test (share-worker sound, peak level), docs-only PR; I will rerun it.

@AlxcBTW

AlxcBTW commented Oct 28, 2026

Copy link
Copy Markdown
Member Author

@parl0v, #29 is merged with current main and green, ready for your re-review. I have not merged it.

What changed in the merge (main merged in with a merge commit, no rebase or force push; head 4aedf7b):

Your four review points: I read them against the code at b93ad39 and what is now on the branch, and all four are in:

  1. Gating tests record the pad serial before the invalid states and require exactly +1 (tests/NetBridgeShare.cpp, around lines 320-360).
  2. The mutex is split into ringMutex, audioMutex and padMutex.
  3. ColumnTable in ShareRing.hpp precomputes the source columns per size pair; no divide per pixel.
  4. The layout test uses literal numbers: 2208 bytes, slots at 64/608/1152, audio at 1696, audio position at 40, the magic bytes.
    The follow-ups from the review (dropped count, present_us, POSIX owner check, short share open, chord constants, debt headings, vectors) are covered in AlxcBTW's reply.

Build: dispatch run https://github.com/anyplaynet/AnyPS5/actions/runs/40604896359 on this head: Windows, Linux and the relinker jobs (macOS arm64 and x86_64, asan) are all green.

Still open on your side: the GPU run (VideoOut chord, AGC readback, frame times against the 1.0 ms budget), and your question whether the graphics driver, libScePad and libSceVideoOut should link libSceNetCtl at all or go through a sink NetCtl registers. The DLL-list fixes only make the current linking work.

@AlxcBTW

AlxcBTW commented Oct 28, 2026

Copy link
Copy Markdown
Member Author

@parl0v, update on #29: main is merged in again (merge commit, no rebase or force push; head 0179d42), and the dispatched build is green on Windows and Linux (https://github.com/anyplaynet/AnyPS5/actions/runs/40619297219, with the macOS relinker jobs and asan).

What came in with main: the AGC fix for the first-present abort (#44, a 16-bit float export into a UINT target now decodes as a plain color export instead of throwing), the bridge-e2e pin to the protocol 1 net service (#45), and the HTTP/2 async fix (#41).

What I changed on this PR:

  • The input-mapper check was red because of feat(bridge): share play frame ring, guest pad and host override #29, not main: the PR adds the StopShare action (F8 by default), and the configurator in tools/input-mapper did not know it, so two of its tests failed. mapping.js and its test now list it as a keyboard-only action with the F8 default; all 24 input-mapper tests pass, and the check is green now.
  • bridge-e2e is green with the new pin.

Nothing else in the share play code changed since your last look. Open on your side as before: the GPU run (VideoOut chord, AGC readback, frame times against the 1.0 ms budget) and whether the graphics driver, libScePad and libSceVideoOut should link libSceNetCtl at all. Could you re-review when you have a moment? I have not merged it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants