forked from unom/punktfunk
main
3359
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 enricobuehler
|
d92266a402 |
Merge pull request 'The console UI overhaul: motion, look, and a library you can actually navigate' (#269) from worktree-console-ui-overhaul into main
Reviewed-on: unom/punktfunk#269 |
||
|
enricobuehler
|
451f95079e |
fix(console): the focus recede ran backwards on all six pale palettes
Caught on the screenshot pass, which is the only place it could have been caught: every unit test and every dark-palette render agreed the recede was fine. `recede_matrix` ported Apple's `.brightness(-0.24·d)` literally, and that is dark-mode arithmetic — on a dark field, "down" is "away". This crate ships thirteen palettes and six of them are PALE, where darkening a card increases its contrast against a light ground. On `holo` the effect was not subtle: the UNFOCUSED host tile rendered as the heaviest object on screen, a grey slab beside the focused card it was supposed to be receding behind. The recede now moves a card toward its GROUND rather than always toward black, taking the direction from the scrim — which already encodes which way the field leans, because it tends to black on a dark palette and white on a pale one. The saturation drain is unchanged in both directions; colour has no handedness. The new test asserts the two poles disagree in SIGN — a receded card sinks on `violet` and lifts on `mint` — because that is the property, and any test written against one palette would have passed before this fix. |
||
|
enricobuehler
|
d927fcb2b0 |
feat(console): Collections — group by platform, walk them, open one
The user's ask, verbatim: group by console, navigate the platforms, pick PS3, see its games. Y on a library opens a carousel of collection tiles; A on one pushes the same `LibraryScreen` with a FILTER set, titled "Host · PS3"; B walks back out. The whole flow is asserted end to end rather than in pieces, because it is the flow that was asked for. It borrows the home carousel's tile language on purpose. A collection is a place you go, exactly as a host is, and the console should have one idea of what that looks like — same sprung cursor, same entrance, same recede, same halo. Each tile fans up to three real covers, taken from the posters the library screen has ALREADY decoded rather than fetched again; a group with no art yet shows its monogram, which is also the permanent look of a platform full of art-less ROM entries. Filtering is index-level. The drill-in sets a `GroupKey` and the shelf collates against it, so the shared model, the art pump and the fetch flow never learn that collections exist — the filtered shelf is the same screen, not a copy of the library with things removed. The gate is a real one, and it is a pair. `worth_browsing` is false for a library with a single group, and BOTH the Y binding and the Y hint read it, so the legend can never advertise a press that only thuds. Launchers alone never count toward it: every library has them, and a collections screen offering only "Launchers" is precisely the one-tile screen the rule exists to prevent. Y is also refused from inside a collection — a drill-in from a drill-in would collate a set that is already one group. Sort lives here, on the shoulders, as four pills. The shelf re-reads `library_sort` every frame, so stepping it re-orders the collection tiles AND the shelf waiting behind this screen at once — which is the argument for putting it on this screen rather than in a dialog. `LibraryScreen` stopped unpacking its host into five scalars and keeps the `HostRow` whole, because Collections has to hand one back to `LibraryScreen::new` and two partial copies of the same host is the state that goes stale first. One thing worth recording: the flow test failed on its first run, asserting the filtered shelf held 0 titles instead of 2 — a screen adopts the shared model on its first sync, and the test asked before the app would have rendered. The sequencing was the bug, not the filter, and the test now draws a frame the way the console does. It is the kind of thing that would otherwise have been "fixed" by weakening the assertion. Verified in the pf-gtkflow container: fmt, clippy --all-targets -D warnings, plain build, 121 console-ui + 213 client-core tests green. |
||
|
enricobuehler
|
c354d7cd09 |
feat(console): a grid arrangement for the library, plus art eviction and a skeleton shelf
The coverflow is a browsing surface: one cover at a time, big, artwork doing the talking. It is the wrong shape for "I know what I want, show me everything", which on a 200-title shelf means holding Right and watching. The grid is that other surface — about eighteen covers at once — and it is a VIEW rather than a second screen: same cursor, same collated order, same art cache, same detail band underneath. Only the arrangement differs. `grid_step` is the pure half, and it carries the one real subtlety: the two boundary rules are different on purpose. A row's ENDS refuse, exactly as the shelf's do, because wrapping would make a held Right scan the whole library and the shoulders already do that. But Down into a SHORT last row CLAMPS to the last title rather than refusing — a ragged tail is a layout accident, not a boundary the user chose to hit. Both, plus page jumps and degenerate grids, are pinned by tests. Where the toggle lives is a deviation from the plan, and deliberate. The plan put View on X and Collections on Y, but X is already Copy link in the library and the grid needs Up/Down for rows, which leaves exactly one free face button for two features. Rather than evict a shipped binding, `library_view` becomes a settings row in Interface beside the palette and reduce-motion — which is what that tab is for, and this is a preference you set once, not something you toggle mid-browse. Y stays free for Collections. Art eviction is not cosmetic. `LibraryScreen::art` had no eviction at all, so every cover ever scrolled past stayed decoded for the life of the screen — the shelf touched a dozen and hid it; the grid touches all of them. Posters are now frame-stamped as they are DRAWN and the coldest trimmed past a budget of 160. Stamping after the draw rather than on arrival is the load-bearing detail, and the test says so: an LRU keyed on arrival would cheerfully evict the six covers the cursor is sitting among. Only decodes are trimmed — the encoded bytes are not kept here either way, so the fetch pipeline is untouched and a re-entry costs one frame of grey. Loading becomes a skeleton in the arrangement the real cards will arrive in, with one travelling sheen — the wait is for a LIST whose shape is known before its contents are, so showing that shape is more honest than a blank field that abruptly becomes a coverflow. It costs one extra paint and freezes under reduced motion. Two things fell out of the refactor and are worth naming: the art-less card face is now one function both arrangements call (a launcher without a poster still must not read as a game whose cover failed to load), and the detail band is shared rather than copied — it also gained the platform, which is the whole point of plumbing it, because "Shadow of the Colossus" means something rather different with "PS2" under it. Verified in the pf-gtkflow container: fmt, clippy --all-targets -D warnings, plain build, 118 console-ui + 213 client-core tests green. One run aborted inside rav1d's CDEF filter; that is environmental (emulated amd64), confirmed by A/B — the immediately preceding run carried the identical pf-client-core sources and passed 213/213, and the re-run passes. |
||
|
enricobuehler
|
c804a0dc9f |
feat(console): plumb platform to the shelf and give it a collation module
`GameEntry.platform` ("PC", "PS2", …) has crossed the wire since the model
landed and was thrown away at the console's boundary: both `LibraryGame` mapping
sites in the session binary built the struct without it. It is what the
rom-manager plugin populates, and without it "group by console" has nothing to
group by. Two lines, and the fake-library loader inherits it for free because it
maps the same entry type.
The interesting half is `collate` — a pure module, no Skia, because grouping
rules are exactly the kind of thing that reads obviously correct and is quietly
wrong, and because it is the half that has to be identical on every client. This
file is the portable spec the Apple and Android ports implement.
Two rules earn their tests:
- A platform-less game does NOT go to "Unknown". A Steam library has no
platforms at all, so an "Unknown" bucket would swallow it whole and be a worse
view than no grouping; a store-front game buckets under its STORE ("Steam"),
which is both true and useful, and only an entry with neither lands in
"Other".
- A-Z folds the leading article. "The Witcher 3" belongs under W; left alone
every "The …" piles up under T and the sort is useless exactly where a long
library needs it. English articles only, with a guard so a title that IS an
article doesn't sort as an empty string and float to the front.
Launchers lead by construction rather than by every caller remembering to put
them there, so design D4 survives grouping for free. Everything returns INDICES:
the art cache, the fetch pump and the cursor all key off the shared model's
ordering, and handing back cloned games would fork the identity of every title.
The shelf now routes its display order through `collate::filtered` even
ungrouped and unsorted — which is why this lands green on its own, and more
importantly is the right shape: shelf, sort and the coming drill-in are then one
screen with one cursor arithmetic. `SortKey::HostOrder` is the default and is
asserted byte-identical to the order the host sent, so a user who never touches
a sort pill sees no change whatever. `LibraryScreen` gains a `view: Vec<usize>`
the cursor indexes instead of `games`, and a `filter` the drill-in will set.
`library_sort` joins `trust::Settings` beside `ui_palette` and `reduce_motion` —
presentation only, never in a profile, parsed leniently so a newer client's key
reads as today's shelf. Its pills arrive with the Collections screen; the sort
is honoured now.
`clients/session/fixtures/mixed-platform-library.json` is the standing dev asset
the rest of Part C is built against: launchers, five platforms, several stores,
platform-less entries, and two deliberately awkward titles ("The Witcher 3",
"Émigré") so a broken fold shows up on glass and not only in a unit test.
Verified in the pf-gtkflow container: fmt, clippy --all-targets -D warnings,
plain build, 111 tests green.
|
||
|
enricobuehler
|
be8183caab |
feat(console): host tiles get their OS mark, the chip gets a battery, the strip gets Rescan
`HostRow.os` has been plumbed since the model landed, with a comment saying the drawing was a follow-up because "the Skia glyph set doesn't exist yet". It does exist: assets/os-icons ships thirteen licensed masters, and `pf_client_core::os::os_icon_tokens` already resolves a chain to them - walking most-specific-first and applying the brand aliases (`macos` -> `apple`, `steamos` -> `steam`). Every other front-end walks that same list. So the console takes the shared resolver rather than inventing one, and gets its table GENERATED from the masters (`scripts/gen_os_mark_table.py`, hooked into the existing `gen-os-icons.sh`) rather than hand-transcribed. Thirteen paths of up to 3.5 kB where one mangled character is a silently wrong logo is not work for a human, which is precisely the reasoning the launcher-icon tables already carry. A new master now reaches the console for free; the script's closing note says so. Two corrections to the plan this implements, both found in the code: - The chain is SLASH-separated and resolves most-specific-FIRST, not "the first known token of a `;`-chain". A `linux/fedora/bazzite` host draws Bazzite, and falls back through Fedora to Tux - so the console is right about thirteen distros rather than the four the plan scoped. - The hint bar was already a glass pill, not "ink on the field". What it was missing is that it mixed its OWN glass (a flat wash and a hand-rolled stroke), making it the one floating surface that ignored the palette; it now goes through `theme::panel` like the chip and the toast, and picks up the lit edge. `draw_monogram` becomes `draw_badge`: the OS mark when the chain resolves, the initial when it doesn't. A substitution, not an addition - a badge showing both a Tux and an "L" says the same thing twice - and an older host that advertises no `os` keeps its monogram pixel for pixel. The controller chip gains a pad silhouette and a battery pip. `PadInfo` gets an additive `battery: Option<PadBattery>`; nothing crosses the wire, this is local SDL state. The plan expected to poll "on the existing pad-refresh cadence" - there isn't one, `publish()` is entirely event-driven (hotplug, pin change). And `pad_info` is deliberately open-free because an open GRABS the hardware, while SDL only reports power for an OPEN device. So the level is read from the ONE pad the service already holds open - `menu_open`, the nav pad, which is open exactly while a console is on screen and is the only pad any UI asks about - on a 15 s poll inside the loop that already wakes every 10 ms. Every other pad publishes `None`, which is the honest answer. `None` renders as no battery at all, never 0 %: a wired pad, a Steam virtual pad and SDL's `-1` "powered, level unknown" are all the same non-answer, and 0 % is the one reading that sends someone hunting for a charger. Charging outranks the low-charge red, because a pad at 4 % on the cable is not the problem a pad at 4 % off it is. Finally, Rescan: a second sentinel tile trailing Add Host, sending the `ConsoleCmd::Probe` that has existed unsent by any screen since it was written. A controller surface has no pull-to-refresh, so the affordance has to be a tile. The two trailing tiles are actions rather than hosts, so `hosts.get(i)` answering both "which host" and "which action" with one `None` became a `Slot` enum - with a second action tile that ambiguity is a bug waiting, and the test that matters is that an accidental A on the end of the strip can never start a session. Verified in the pf-gtkflow container: fmt, clippy --all-targets -D warnings, plain build, 104 tests green. |
||
|
enricobuehler
|
a289f7b9af |
feat(console): recede by draining colour, not by painting black over it
An unfocused card lost scale and gained a flat black veil. A veil only darkens, so a receded card stayed exactly as colourful as the focused one and the eye went on reading it as a competing subject - which is why the veil had to be heavy to work at all, and why the shelf read as "dimmed" rather than "deep". `theme::recede_matrix` adds the two channels the Apple gamepad UI uses and this crate did not: saturation down 42 % and brightness down 24 % at full recede, as one 4x5 colour matrix on the layer each card composites through. The veil then drops to HALF its old strength on both surfaces (home 0.24 -> 0.12, the coverflow's RECEDE_DIM 0.30 -> 0.15), left doing the one job a flat darkening is good at: separating cards that overlap. Alpha is deliberately untouched by the matrix, and there is a test saying so. The coverflow's side cards overlap, so a recede that reached alpha would let them show through each other - which is the reason the veil exists in the first place. A second test pins the focused card as EXACTLY identity, because a matrix that tinted the focus by half a percent would be invisible in review and wrong in every screenshot. The focused tile also gains an accent-tinted halo under its shadow - the palette-aware mark that says "this one" from a sofa, where a 12 % scale difference says nothing. On the coverflow it is drawn in screen space before the card's own transform, because it is light spilling AROUND the card and cannot live inside the rounded rect the card clips itself to; it fades with the sprung proximity rather than snapping on the integer cursor, so it travels with the strip. Glass gains a lit top edge (`theme::panel_highlight`): a 1 px inner stroke fading out over the top 40 %, which is what makes a material read as material rather than as a tinted rectangle. Deliberately a separate call rather than a flag on `panel`, so the cost discipline is visible - tiles and the one focused row get it, the dozens of resting rows a settings screen paints every frame do not. Cost, stated plainly: the coverflow previously had no per-card layer, so the recede adds one per VISIBLE card there (bounded to the card by raising it after the clip, and skipped entirely for the focused, settled one). That is the plan's named O(visible cards) price and the thing to watch on a Deck frame graph. The home carousel already composited each tile through a layer, so it pays nothing extra. The typography sweep found less than expected, which is worth recording: the crate is already almost entirely on the 0.55 / 0.7 / 0.85 ladder. Two genuine outliers, both in the library - a group heading a shade louder than the identical role in `MenuList` (0.5 -> 0.45) and a detail subtitle off the ladder (0.5 -> 0.55). The `fg(0.92)` runs are NOT outliers: they are letters on a button badge and text on a toast pill, sitting on their own ground rather than on the field. `ROW_MAX_W` is already Apple's 620, and the 30 dp title against 16 dp row labels is within a hair of Apple's 34/17 rhythm, so neither moved. Verified in the pf-gtkflow container: fmt, clippy --all-targets -D warnings, plain build, 99 tests green. |
||
|
enricobuehler
|
50ef253e43 |
feat(console): the screen transition becomes an interruptible spring (vectors v2)
Pressing B while a screen was still arriving did nothing at all. The transition was a 0.26 s ease-out-cubic and every event that landed inside it was thrown away, so holding B to back out of a deep stack stuttered at every level: press, wait, press again. That wall is the one place the console felt less alive than the Apple shell it is modelled on. `Motion` becomes one sprung scalar. The paint recipes read `spring.pos` exactly where they read `ease_out_cubic(raw)` - same 36 dp slide, same 0.985/0.96 scales, same 0.4 reveal alpha, so the choreography is untouched and only the time-course changed. Those four numbers stop being literals buried in the paint sites and become named constants, because the vectors file claims to pin them for all three clients and a literal is a thing no test can reach. What the spring buys is the retarget. Back mid-push flips THAT SAME spring from 1.0 to 0.0: velocity carries, so the entering screen decelerates, turns, and goes back down as one continuous motion, and `finish_nav` takes it off the stack when the spring lands. A tween cannot do that - its progress is a function of elapsed time, so reversing means either a snap or a second animation played behind the first. The test asserts the continuity directly (no sample jumps by more than a frame of the travel already underway), because "cancel and play a pop" would pass a weaker test and still look wrong. Back is now always heard; everything else stays queued-dropped until the spring passes 0.85, so the double-tap protection survives. The threshold is on POSITION, not elapsed time - with a spring those are different questions, and the one that matters for input is whether the screen under the cursor is the one being aimed at. Mid-pop, A is still refused (activating a half-dismissed screen is a mis-tap, not intent) while Back starts the next pop immediately, which is the stutter this removes. At the root the transition declines the press, because there is no parent to fall back to and B there means quit. Parity: `console-vectors.json` gains a `motion_spring` block and goes to version 2. Springs are integrator-dependent, so v2 pins PARAMETERS where v1 pinned sampled positions - two runtimes that both honour response/damping agree to the eye and disagree in the third decimal, and sampling would pin the disagreement instead of the feel. The v1 `motion` block stays in place, marked `$deprecated` and still correct, because the Android client's ConsoleVectorsTest reads it; this crate's test switches to the new block and asserts the note is still there, so deleting v1 here would fail loudly rather than redden Android silently. (Note for the follow-ups: the Apple client mirrors the v1 constants in GamepadShell but does NOT test them - its ConsoleVectorsTests covers the palette table only. The plan assumed otherwise.) `Progress` is deleted. The transition was its last caller, and this crate lints dead code as an error - keeping a timer nothing times behind an allow would be the wrong kind of tidy. Verified in the pf-gtkflow container: fmt, clippy --all-targets -D warnings, plain build, 97 tests green. |
||
|
enricobuehler
|
2314637c0d |
feat(console): screens arrive instead of appearing - the staggered card entrance
Until now a console screen popped into existence fully formed; the only thing that moved was the shell's own push slide. This ports the Apple gamepad UI's `CardEntrance` semantics as `anim::Entrance`: a pure function of the shell clock and an item's distance from an anchor, so a screen holds ONE of them and asks it per item. No per-card state to keep in step with a host list that churns under discovery. Per item: a delay of 0.07 s per step from the anchor (capped at 0.42 s, which is what stops a 400-title shelf still arriving a minute later), then a 0.6 s window of ease-out-BACK travel - it crosses 1.0 and settles back, which is the difference between a card thrown into place and one slid there - with the fade finished in the first third, so what you read is the motion and not a dissolve. The anchor is the CURSOR, so a restored selection assembles around the eye. Three surfaces, each applying the envelope to its own geometry: - The home carousel: tiles arrive 26 % small, 34 dp low, fading, folded into the transform and layer alpha each tile already applies. Hit boxes follow the drawn geometry, because a press during that second must land on the card the eye sees. - The library coverflow: the same, plus a real Y-turn on the card's OWN rotation matrix. Apple had to fake its turn with a cos-squeeze because SwiftUI cannot snapshot a rotated layer to glass; Skia has no such constraint, so the strip genuinely fans open like a book. Cards turn away in the direction they sit from the anchor. - Menu lists: 12 dp rise, 0.03 s stagger, capped at eight rows. Same language, deliberately quieter - a settings list that fans open like a shelf of box art is a settings list showing off. NOT replayed on a tab switch: re-fanning the rows on every L1/R1 would turn a skim through the sections into a flicker. The library waits for content before it plays. Art streams in per title AFTER the list lands, so without a gate the entrance would reliably show off a rank of grey placeholder faces; it arms once the cards around the cursor have posters, or 400 ms have gone by and they clearly are not coming - so an art-less library of custom entries still gets its entrance, just later. On cost: the coverflow's resting frame is byte-for-byte as cheap as it was. It deliberately has no per-card layer - side cards overlap, so it dims them with an opaque veil rather than whole-card alpha - and the entrance raises one only while a card is still translucent, after the clip so `None` bounds mean the CARD and not the screen. A full-screen layer per arriving card is the one way this could have cost real time on a Deck. Under reduce motion the whole thing degrades to a staggerless crossfade, decided once when the entrance is armed so one entrance plays one way. Verified in the pf-gtkflow container: fmt, clippy --all-targets -D warnings, plain build, 94 tests green. |
||
|
enricobuehler
|
46d648cf0f |
feat(console): name the motion, spring the micro-motion, and let it all be turned off
The console's feel was scattered across a dozen hand-picked time constants, so "how a focused row arrives" was a tau someone chose in a hurry rather than a decision anyone could find. `anim.rs` grows a `SpringSpec` table (FOCUS, INDICATOR, PRESS) with a const `kc()` that performs SwiftUI's response/damping -> k/c conversion; a test pins it against TRAY_K/TRAY_C, which were hand-derived from exactly that arithmetic, and a second states the damping choices as behaviour (FOCUS must overshoot, INDICATOR must not). Three widgets stop easing and start springing, each for a reason: - Row focus splits into two channels. Scale is sprung, because the whisker of overshoot is the pop that makes a row read as picked up; tint and alpha keep `approach`, because an overshooting colour lands on a tint that is not in the palette. - The tab pill's x and width are sprung, so a fast skim through the sections accumulates into one accelerating travel instead of a series of eases that each restart at zero speed. - A stepped value slips 14 dp and springs back, crossfading the old value out. The list detects the change ITSELF by comparing what it drew against what the screen hands back, so no screen has to report whether its edit landed - and a refused adjust correctly produces no motion at all. Toasts gain a kind. "Paired with X", "Session ended" and "Couldn't connect" were the same grey pill, so the only way to tell them apart was to read them; each now carries a mark and a hairline. Error is the one tint that is fixed rather than palette-derived: moss's accent is a green, and reporting a failure in the colour the UI uses for "this is fine" is the bug. And a Reduce motion setting, under Interface beside the palette. It freezes the backdrop shader's clock (gated in `draw_aurora`, so the connect takeover inherits it), flattens push/pop to a 0.2 s crossfade with no slide or scale, and drops the recoil and slip travel - while keeping every haptic, because the pulse is what replaces the motion, not another thing to take away. A console setting rather than an OS mirror: there is no system reduce-motion SDL can read portably. It doubles as the OLED-friendly mode. Verified in the pf-gtkflow container (this crate is invisible to a Mac checkout - `cargo check -p pf-console-ui` there compiles nothing at all): fmt, clippy --all-targets -D warnings, a plain build, and 90 tests green. |
||
|
enricobuehler
|
adf895ced1 |
Merge pull request 'main is red: the PyroWave worker proxy never wrote distrust_references' (#268) from worktree-pyrowave-distrust-refs into main
Reviewed-on: unom/punktfunk#268 |
||
|
enricobuehler
|
18fdc4cc6b |
fix(encode): the PyroWave worker proxy never wrote distrust_references, so its own guard test failed
Pre-existing on main, inherited by merging it. `distrust_references` arrived with the RFI anchor work in `93c1ed07`; `RemotePyroWave` did not gain it, and the trait-coverage test beside the impl caught exactly that. The answer is a no-op, for the same reason `invalidate_ref_frames` returns false and `request_keyframe` does nothing: PyroWave is intra-only, so every AU is already a keyframe and there is no RFI anchor trust to withdraw. The in-process encoder reaches the same answer by inheriting the trait default, and that is fine there. It is not fine HERE, which is the point the guard test is making. This type is a proxy: an inherited default means the worker never hears the call, so a method that does something is silently dead on every worker-backed session while the in-process path keeps working. The test therefore requires the method to be WRITTEN, so that "nothing to do" is a visible decision rather than an omission. Written, with the reason. Verified with CI's exact command — `cargo test -p pf-encode --features nvenc,vulkan-encode,pyrowave` — 122 passed, and clippy clean under the same features. |
||
|
enricobuehler
|
7ba344cf4e |
Merge pull request 'Lossless audio: the 0xD3 PCM plane, 44.1–176.4 kHz, surround, verified on glass (ABI 24)' (#263) from worktree-hires-audio-pcm into main
Reviewed-on: unom/punktfunk#263 |
||
|
enricobuehler
|
e6476eddcf |
fix(core): a doc line starting with > read as a markdown blockquote and failed clippy
`--all-targets` clippy treats `/// >24 kHz` as the start of a quote block and then rejects the unmarked lines that continue it. I added that test and gated it on tests and fmt but not clippy, so it would have reached CI red. Reworded rather than escaped — the sentence reads better without the symbol anyway. |
||
|
enricobuehler
|
47c9162433 |
Merge remote-tracking branch 'origin/main' into worktree-hires-audio-pcm
# Conflicts: # crates/pf-client-core/src/pad_audio.rs |
||
|
enricobuehler
|
a9ffd3becd |
feat(host): a Hello that names a format but not the capability was ignored in total silence
Found on glass, and it cost the first run of the §13.2 check. The host resolved `plane="0xC9 Opus"` while every condition a reader could see was satisfiable — the operator policy was on, the session was stereo, the rate was supported, and the video bitrate left room. The reason was gate condition 1, which is deliberately NOT logged because "the client did not set the capability" is every ordinary session with every shipping client and would drown the log. That reasoning holds for the ordinary case and not for this one. A `Hello` carrying a rate or a depth but no `CLIENT_CAP_AUDIO_HIRES` is contradictory: something asked, and is being ignored. The two halves come from different places in a client — the capability from a settings toggle, the format from whatever that toggle resolved to — so they can drift apart, and an embedder that gets it wrong currently sees nothing at all. One warn, only in the contradictory case, naming both halves and saying they must be set together. |
||
|
enricobuehler
|
362be21ebb |
fix(probe): the format was on the wire but the capability bit was not, so the host was right to ignore it
Correcting my own previous commit, which claimed the probe had been "asking for the lossless plane by accident". It had not, and the comment I deleted for being wrong was right. This `Hello` is built BY HAND and never passes through `advertised_client_caps`, the helper that derives CLIENT_CAP_AUDIO_HIRES from the requested format for the shipping clients. So the probe's explicit 48000/16 was inert: the host's gate tests the capability first, and without the bit a format on the wire is a request it correctly ignores. No session was ever mislabelled, and the old comment's "never sets CLIENT_CAP_AUDIO_HIRES" was an accurate statement about this file. What was actually broken is what I added: `--audio-format` set the rate and depth and not the bit, so it asked for nothing. Found on glass — the host resolved `plane="0xC9 Opus"` while every gate condition looked satisfiable, and nothing was logged, because condition 1 is deliberately unlogged (it is every session with every shipping client). Both are set together now, and the comment says why they must be. With that fixed the plane resolves end to end against a real Linux host: 0xD3, 96 kHz, 24-bit, 192 samples per channel — the 2 ms frame the ladder predicts for 96/24 at the default MTU. |
||
|
enricobuehler
|
bca85db160 |
fix(probe): it asked for the lossless plane by accident and then discarded every frame of it
The probe sent an explicit 48 000/16 as its "legacy" audio request. Under the capability
rule that is a genuine hi-res request — the bit is set when either field is non-zero,
because it keys on "the caller specified a format" — so against a host with the operator
policy on, the probe advertised the capability, was handed the `0xD3` plane, and counted
nothing, because its decode arm only ever matched `0xC9`.
Its own comment asserted the opposite ("never sets CLIENT_CAP_AUDIO_HIRES"), which is
what made this invisible to a reader. Same shape as the Android bug, same root: a doc
that stopped being true when the rule changed. It sends the `0`/`0` sentinel now.
Then it learns the plane it was accidentally asking for. `--audio-format` takes
`opus` (the default, unspecified) or a rung, validated against core's own supported-rate
predicate rather than a restated list; `--audio-out FILE` writes the DECODED audio as
raw interleaved f32 for offline analysis.
That last one is the point. Design §13.2 — "play a >24 kHz tone on the host and confirm
it arrives" — has been the one check no test suite can replace, and it was written as a
listening session. It is now a command: play the tone, run the probe with
`--audio-format lossless96 --audio-out`, and look for energy above 24 kHz. A brick wall
there indicts the host's capture specifically, because core's own test already proves
the transport carries it.
The decode arm reads the RESOLVED format off the Welcome, never the request. The host
may decline hi-res for any of the five reasons in the gate and answer Opus, and a probe
that trusted its own ask would mis-parse every datagram it was actually sent.
|
||
|
enricobuehler
|
b347682eb2 |
feat(console-ui): Gaming Mode was the one client surface with no audio-format row
Every other client got the picker; this one load-modify-saves the whole settings struct, so a format chosen elsewhere round-tripped through it untouched and simply could not be set from the console. The row renders from `pf_client_core::session::AUDIO_FORMATS` rather than restating the values, so it picks up rungs the other clients add without being edited. An unrecognised stored value — a newer client's rung arriving through the shared profile catalog — renders as Opus rather than a dash, because that is what the wire mapping resolves it to; the row and the request agree. The stereo gate stays, but for a different reason than the one that was written down. The host's `channels != 2` decline is gone and the arithmetic supports that: at 48 kHz both 5.1 and 7.1 fit a 1 ms frame. But `pf-client-core` still filters a surround request out before it reaches the wire, so a live row under 5.1 would be a control that changes nothing — precisely the lie this screen dims rows to avoid, and it would disagree with the GTK dialog reading the same settings file on the same machine. The comment says the old reason is dead, names the real one, and says to delete the arm when the client-side filter learns the ladder rather than when the host rule changed. No override marker or profile commit: this screen has neither for any row, by design — the console pins profiles rather than editing them. |
||
|
enricobuehler
|
f206feb588 |
test(core): nothing proved that the plane actually carries what it exists to carry
The whole reason for a second audio plane is that Opus is 48 kHz by construction, so content above 24 kHz is gone before its encoder sees it. That claim had no test. Every existing one checks bit-exactness code by code, which says nothing about frequency — a pipeline that silently band-limited would pass all of them. A 30 kHz tone now goes through the real `0xD3` payload path at 96 kHz and 176.4 kHz and must come out with its energy intact and every sample inside one 24-bit code. The detector's discrimination is asserted too, in the same test. A single-bin DFT that reads high everywhere would "prove" survival through a pipeline that deleted the tone, so the test also measures a frequency that is NOT in the signal and requires it to read as silence. A spectral assertion without that guard is decoration. This is the SOFTWARE half of design §13.2. The other half — that the host's capture did not resample on the way in — is WASAPI autoconvert and PipeWire's resampler, and needs a host and an interface. What this settles is the inference: a brick wall at 24 kHz in an on-glass spectrum now indicts the capture path specifically, because the transport is known to carry it. Goertzel rather than an FFT, so this costs no dependency in a crate that ships into five clients. |
||
|
enricobuehler
|
cd8e19a7d5 |
Merge pull request 'A pipewire type in a cfg(any(linux, test)) signature has been holding the Windows client leg red' (#267) from worktree-padaudio-win-cfg into main
Reviewed-on: unom/punktfunk#267 |
||
|
enricobuehler
|
47d8301aeb |
feat(host): the presentation clock advanced by a frame's label rather than its length
Admits the 44.1 kHz family and hi-res surround, and fixes the drift that admitting them turns from latent into live. `next_pts_ns += frame_us * 1000` advanced the clock by the frame's NOMINAL duration. That was harmless while every rate divided every rung, and 44 100 Hz divides none of them: a "5 ms" frame carries 220 samples per channel, which is 4 988 662 ns. The clock therefore invented 2 267 574 ns every second — 2 272 ppm, 8.2 seconds an hour — and the re-anchor beside it is `max`, so it only ever corrected FORWARD. A fast clock was never pulled back, and the A/V sync loop would have chased it forever. The clock is now a running sample total rather than a sum of per-frame durations, which accumulates exactly zero where summing floored frames accumulates about a nanosecond a frame. It lives in a small `PtsClock` because the drift sat inside `audio_thread`, which no test can reach; as a struct it is provable, and the planted-error run pins every ladder rung rather than just 5 ms. `advance` folds whole seconds out of the sample count into the base — exact, since `rate x channels` samples is precisely one second — because `usize` is 32-bit on some targets and 176.4 kHz 7.1 would wrap it in under an hour. The frame pacer was changed to agree even though it is cosmetic: it is gated on real data being available, so a nominal value produced a slot the pacer waited out rather than time it invented. Two clocks describing the same frame and disagreeing by 0.23 % is the kind of thing a later reader reconciles in the wrong direction. The rate gate reads `pcm::rate_is_supported` instead of restating the set. The Windows 48 kHz floor is untouched and never needed touching — the `max` sits inside the "requested above engine" arm, so a 44 100 request never tripped it. The comment now separates the two rules that read as one line: refusing to ask for more than the engine has is the honesty rule for either plane; the 48 kHz floor is for Opus alone, because libopus accepts 8/12/16/24/48 kHz only. Surround's `channels != 2` decline is DELETED rather than relaxed. The design called it a one-line restriction; it was worse than that, because the check ran BEFORE the frame ladder and overrode the one piece of code that knows the answer. The ladder is channel-aware, so it decides and `None` declines — and the design's blanket "surround is out at the default MTU" turns out to be wrong in both directions: 48 kHz 5.1 fits at 1.5 ms and 7.1 at 1 ms, while 16-bit 5.1 fits even at 88.2 and 96 kHz. What is actually true is that nothing surround fits above 48 kHz in 24-bit, and no 7.1 fits above 48 kHz at all. Two stale figures corrected while adjacent: the budget doc claimed a 20 Mbps session affords the whole stereo ladder (176.4/24 stereo is 8.5 Mbps and wants 33.9), and the operator-facing decline log quoted 1.5–4.6 Mbps for a plane that now spans 1.4–8.5 in stereo and 33.9 in 7.1. |
||
|
enricobuehler
|
6bbdafc289 |
feat(apple): the Swift port of the de-jitter policy had the same divide-first defect as core
This client hand-ports `JitterPolicy`, `AvSync` and `DroughtConceal` into Swift rather than draining core's decisions, so every fix core just made had a second, silent copy here. `perMS = (rateHz / 1000) * channels` divided before it multiplied, exactly as core did, so 44 100 Hz became 44 samples per millisecond and every depth, target and reported figure was 2.3 % low. All sixteen conversion sites now multiply first. `frameSamples` delegates to the same floor-per-channel rule core uses instead of deriving from `perMS` — at 44 100 Hz a 5 ms FRAME is 440 interleaved samples where 5 ms of AUDIO is 441, and the shed and the near-miss margin both mean "exactly one packet", so a self-derived answer described a packet that does not exist. Swift traps on overflow rather than wrapping, and the samples-to-ms direction takes a caller-supplied count that `setSyncTarget` can drive to `Int.max / 2`, so that one saturates explicitly — an untrapped multiply there would abort inside the render callback. `DroughtConceal` counts frames now and derives both millisecond figures from the resolved frame, and its two thresholds are two real frames rather than a fixed 10 ms. The old shape charged 5 ms per concealed frame whatever the frame was: on the 1 ms frame that 48 kHz/24-bit 7.1 lands on, it bought a fifth of the concealment it promised and reported 24 ms of synthesis as 120. The frame count stayed right, which is why nothing played wrong and nothing caught it — only the relationship between the count and the frame length exposes it, and that is what the new test asserts. The stereo gate is gone from all three settings surfaces and from the connect path, and the format picker offers the 44.1 family. The resolved rate is rendered with its fractional kHz — integer division would have printed a 44 100 Hz session as "44 kHz", and `String(format:)` would have localised the separator. |
||
|
enricobuehler
|
0196c75769 |
fix(android): every Opus session was advertising the lossless capability, and the ABI doc told it to
Two things: a live opt-in bug, and the 44.1 kHz family. Android returned `(48_000, BITS_16)` for the "Standard (Opus)" setting and handed it to `connect_with_audio_format`, whose capability rule sets the bit when EITHER field is non-zero — it keys on "the caller specified a format", not "the format differs from the default", because otherwise 48/16 would be the one rung on the ladder nobody could ask for. The host's gate accepts 48/16 as a supported format. So on any host with the operator policy enabled, a user who chose Opus silently got the lossless plane at 1.5 Mbps. `punktfunk_connect_ex11`'s doc caused it: it said passing 48000/16 was "exactly `ex10`, byte-for-byte on the wire". That stopped being true when the capability rule changed and nobody updated the sentence. It now says the opposite explicitly, including that a hardcoded 48 000/16 as a stand-in for "default" opts every ordinary session in. Both ends of the Android path send the `(0, 0)` unspecified sentinel now, and two tests pin the asymmetry from either side of the JNI boundary. The 44.1 kHz half mirrors core: the client's own `per_ms` had the same divide-first defect, so its conversions are exact now, and `frame_samples` delegates to core rather than re-deriving — at 44 100 Hz a 5 ms FRAME is 440 interleaved samples where 5 ms of AUDIO is 441, and the shed and near-miss margin both mean "exactly one packet". The openable-rate probe descends the requested rate's own family before falling to the 48 kHz floor, because AAudio grants a rate or fails and never substitutes. The floor is accepted unprobed — universally granted, and the answer a failed probe falls back to — so a default session still opens no stream and pays nothing. The probe uses the requested channel count because the resolved one does not exist until the Welcome, which errs toward Opus. Surround is no longer hidden. The gates were removed rather than relaxed: core's ladder is channel-aware and the host decides, so the client asks and reports what it was given. |
||
|
enricobuehler
|
f30f7e88b0 |
feat(client): the desktop had no way to ask for the lossless plane except an environment variable
Apple and Android both shipped a real audio-format picker; the desktop had a stopgap env lever, because `SessionParams` had no field for it and the one struct literal that fills it lives in another crate. It has one now, and the four clients share a vocabulary. The stored values are byte-identical to Android's and Apple's — `opus`, `lossless48`, `lossless96` under the key `audio_format` — because a profile that round-trips between a phone and a TV but not to the desktop is the exact bug worth avoiding. They are read from those clients rather than invented here, and a value this build does not recognise resolves to Opus rather than refusing the connect. `PUNKTFUNK_AUDIO_HIRES` still overrides the setting, in BOTH directions, which is how this crate already treats every other `PUNKTFUNK_*` lever. A lever that loses to a stale profile is useless for the thing operators reach for it for, and one that can only switch a feature ON is half a lever. One behaviour deliberately changed: an unparseable value used to mean "off". It now warns and is ignored, so the user's setting still decides — garbage silently defeating a switch somebody set in a UI is worse than the pre-UI behaviour it replaced. Unset, off and garbage are three distinct outcomes now, and a test pins all three. The precedence itself is a pure function of (env, setting) so it can be tested without mutating the process environment, which is the idiom the neighbouring parse already used. Stats carry the RESOLVED format off the Welcome, never the requested one, and the OSD prints it — a desktop that says "lossless" while the host declined is the same class of lie as claiming a sample rate you did not get. |
||
|
enricobuehler
|
ec500b0943 |
feat(host): monitor mode declined hi-res because nobody had written the lookup it was waiting on
The design said hi-res is offered in stream-sink mode and declined in monitor mode "unless the monitored node's own rate can be read from the registry", and that lookup was never written — so `PUNKTFUNK_STREAM_SINK=0` could never have the feature at all, by omission rather than by decision. It exists now. Reading our own capture stream's rate would be worthless: in monitor mode we tap someone else's sink THROUGH PipeWire's resampler, which reports a clean rate whatever is upstream — the same blindness as WASAPI's autoconvert, which is the trap this whole feature is built around. So the lookup binds the graph's elected default sink and reads that NODE's format: one bounded round-trip with a 3 s timer, so a sick graph costs a fallback rather than a stalled handshake. It reads `default.audio.sink`, the elected default, NOT the neighbouring `default.configured.audio.sink` that this host's own stream-sink code writes. That one is a preference: unset on a box nobody configured, and able to name a node that no longer exists. Unknown declines. No graph default, no format, no answer inside the timer — each resolves to Opus rather than guessing, because the whole point is that a session must never claim a rate its content does not have. That asymmetry is stated at the lookup, at the call site and in the module doc, since "unknown means fine" is the opposite convention to the one the wiring plan uses a few files away. One nuance the design does not address: a monitor tap emits the GRAPH-side rate, while `Format` on an adapter node forwards to the device side. On an ordinary box those are the same number, because PipeWire opens the device at the graph rate whenever it can. They diverge only for a device that cannot run the graph's rate, where reading the device side declines something that would have worked — the safe direction. The exactly right source is the sink's monitor PORT's own format, one further registry hop, named at the call site as a follow-up rather than left implied. Also documents the one variable name both ends read: the host treats `PUNKTFUNK_AUDIO_HIRES` as a boolean and the desktop client accepts a rate or a rate/depth pair, so a box that is both sees two grammars. `1` is the spelling that means "on" to both. |
||
|
enricobuehler
|
63312e0a19 |
feat(core): the rate divided before it multiplied, which is the whole reason 44.1 kHz was deferred
Lifts the design's §4.1 deferral and admits 44 100 / 88 200 / 176 400 Hz. `JitterPolicy` and `AvSync` computed `per_ms = rate_hz / 1000 * channels` and then expressed every figure as `ms * per_ms`. The division came FIRST, so 44 100 became 44 samples per millisecond and every depth, target, shed threshold, hard cap, de-prime fuse and reported `buffer_ms` was 2.3 % low. 48 000 and 96 000 were exact only because they happen to divide. The design prescribed "denominate the policy in samples with a rational `per_ms`". No rational type was needed: the defect was operator ORDER. Multiplying first and dividing last is exact at every rate for one integer division per conversion, and 48/96 kHz stay bit-identical by construction — `per_sec == 1000 × per_ms` exactly there, so both conversions reduce to the old expression. Every existing policy test passes untouched. `frame_samples()` now delegates to `pcm::samples_per_frame` rather than re-deriving the count. At 44 100 Hz those differ: 5 ms of audio is 441 interleaved samples but a 5 ms FRAME is 440, because 220.5 samples per channel do not exist. The near-miss margin and the shed both mean "exactly one packet", so a self-derived answer would have described a packet that does not exist. `frame_duration_ns` is the new inverse, and it exists because a frame's label and its duration have come apart. 44 100 divides none of the seven ladder rungs, 88 200 divides only 5 ms, 176 400 only 5 ms and 2.5 ms — so a rung is a nominal length for the wire and the ring, never a duration. Anything advancing a timestamp must use the real sample count or the clock runs 0.23 % fast forever, which the A/V sync loop would fight and never win. `MAX_CONCEAL_PACKETS` was a frame count documented as "50 ms at the protocol's 5 ms frames" — 20 ms at a 2 ms lossless frame. It is now `MAX_CONCEAL_MS` with the count derived, and `AudioGapTracker` carries the frame the same way `JitterPolicy` and `DroughtConceal` already do. The ABI conceal buffer sizes its run from the derived cap while keeping the frame size at the longest rung — mismatched deliberately, and only in the direction that cannot overrun. Overflow is handled where the multiply-first order creates it: 480 000 ms at 176 400 Hz by 8 channels is 6.8e11, which wraps a 32-bit usize before the divide brings it back, so the intermediates are u64/u128 and saturate rather than wrap — a wrapped window is a tiny one, i.e. a fuse that blows instantly. |
||
|
enricobuehler
|
75eaf9ad91 |
Merge pull request 'The game library was unreachable in Android's touch UI, and no library view could copy a title's link' (#266) from worktree-library-access-and-copy-link into main
Reviewed-on: unom/punktfunk#266 |
||
|
enricobuehler
|
d5838f381a |
fix(client-core): a pipewire type in a cfg(any(linux, test)) signature broke the Windows client leg
Main has been red on both Windows client jobs since the pad-audio work landed:
error[E0433]: cannot find module or crate `pipewire`
--> crates\pf-client-core\src\pad_audio.rs:368:39
error: could not compile `pf-client-core` (lib test) due to 1 previous error
`pad_audio.rs` gates its pure-logic helpers `cfg(any(target_os = "linux", test))` so the
sink-picking decisions stay unit-testable on every platform. `sink_from_props` was given
the same gate, but it is not pure logic: it takes a `&pipewire::spa::utils::dict::DictRef`,
and `pipewire` is declared under `[target.'cfg(target_os = "linux")'.dependencies]`.
So on Windows the `test` arm pulls the item into the `lib test` target, where the crate
does not exist. Nothing but `--all-targets` compiles that target, which is why no Linux
gate could see it — including the container gate used to validate the original change,
which lints pf-client-core for the host target only.
The function's sole caller already lives inside a plain `cfg(target_os = "linux")` block
and no test refers to it, so the `test` arm was only ever dead weight. Dropped it.
Swept the rest of the crate for the same shape: of the 13 `cfg(any(target_os = "linux",
test))` items in pad_audio.rs, this is the only one that names a Linux-only crate, and no
other file in pf-client-core uses that gate. All 13 other pipewire references sit under a
plain `cfg(target_os = "linux")`.
Linux codegen is unchanged: `target_os = "linux"` was already true on that arm, so the
item compiles exactly as before there.
|
||
|
enricobuehler
|
c7fe326b99 |
Merge pull request 'Steam renamed its cached art, so most covers were never found — and the banner that stood in blew the iOS tile open' (#265) from worktree-steam-cover-capsule into main
Reviewed-on: unom/punktfunk#265plugin-kit-v0.4.2 |
||
|
enricobuehler
|
65b4af9e51 |
Merge pull request 'The library grid never got the FlowBox recursion guard, so every game click overflowed the stack' (#264) from worktree-gtk-library-card-activation-overflow into main
Reviewed-on: unom/punktfunk#264 |
||
|
enricobuehler
|
14bdfeaeeb |
fix(client-windows): the library tile's overflow sat inside its own launch backstop
The poster's new "…" button was a child of the border carrying on_tapped, so opening the menu was also a click on the tile. host_tile on the hosts page already splits these — the tapped region is the inner summary and the menu button is its SIBLING in the grid — and the library tile now does the same. Relying on Button marking Tapped handled is a guess about event routing where the shell already has an answer that does not need one. |
||
|
enricobuehler
|
ae6d872bed |
test(client-android): shoot the touch library, and pass the shelf's callbacks as lambdas
Two follow-ups on the library work: * LibraryScreen handed ConsoleLibrary/TouchLibrary `::launch` and `::copyLink`. Two callable references to the same local function compare EQUAL however different the frame they captured, so a skipped recomposition would have left the child calling a closure over stale settings — the trap SettingsScreen documents at scopeProfile(), having already been bitten by it. Lambdas instead, which is what every other call site here passes. * The touch grid was a brand-new screen with no visual coverage. TouchGrid goes internal (the reason Coverflow already is) and the harness grows a portrait scene over the same mock shelf the coverflow scene uses, so the column count, the group headings and the launcher brand-mark tile are all captured. |
||
|
enricobuehler
|
b949bf2062 |
feat(clients): the game library was unreachable in Android's touch UI, and no library view could copy a title's link
Two gaps, both about the library: * On Android, the shelf was reachable only with Y from a console tile — a finger has no Y, so the touch home had no route to it at all. LibraryScreen grows a second presentation (the poster GRID the Apple, GTK and Windows shells already draw) and a card's overflow grows "Browse library…", behind the same experimental toggle as everywhere else. * No library view offered "Copy link", although design/client-deep-links.md §5 names the library game context menu as an attach point beside the host card and the pinned card. Every shell now hands out a title's own URL — the same self-emitted form a host card gives, plus that game's launch= id, so pasting it into Playnite or a Stream Deck macro boots straight into the title. The gesture follows each surface: a context menu on the touch/mouse grids (Apple, Android, GTK; Windows uses a tile overflow button, since the reactor hangs menu_flyout off buttons only), and the X face button plus a legend entry on the three gamepad coverflows, where there is no right-click. |
||
|
enricobuehler
|
d8f7984477 |
fix(core): the drought fuse was spent in 5 ms units on a plane whose frames are 2 ms
`DroughtConceal` charges one frame per concealed frame but bounds itself in WALL-CLOCK milliseconds, and the two disagreed about how long a frame is. It assumed 5 ms, so on a 2 ms lossless frame the `max_ms` budget ran out after two fifths of the time the tuning intends, and the `plc_ms` line over-reported concealment by the same factor. The frame COUNT was always right — it charged 5 and divided by 5 — which is exactly why this went unnoticed: the load-bearing number was correct and only the two human-facing ones were wrong. It now counts frames directly and derives both millisecond figures from the negotiated frame, so `packet()` needs no division at all. The two thresholds move with it. Both were `2 * FRAME_MS`, expressing "two frames", so they are now two real frames rather than a fixed 10 ms — on a 2 ms plane the old value waited five frames before conceding there was a stall. Both clients pass their resolved frame in, and the desktop client drops the local frame tally it had to keep while core could not be told. |
||
|
enricobuehler
|
5c746fbea5 |
chore(plugin-kit): 0.4.2 — publish the art scan fix, which no plugin can pick up otherwise
The Steam scanner is `@punktfunk/plugin-steam`, in its own repo, and it builds with `--external '@punktfunk/*'` — so `findLocalArtFile` is resolved from the registry at install time, not bundled. Until the kit cuts a version, every host keeps resolving 0.4.1 and keeps missing the covers. Same shape as the `icon` field at 0.4.1: kit change, out-of-tree producers, and nothing in this repo says so. Nothing but the version moves here. |
||
|
enricobuehler
|
59fc9294ea |
fix(client/apple): a fallback banner sized the poster tile it was supposed to be cropped into
Reported as the library "breaking" on iOS whenever a title showed a banner instead of a cover: the tile expanded out of its bounds. `PosterImage` already meant to prevent exactly this — its own comment said a banner "would otherwise report a much wider intrinsic size than the card and overflow into neighboring cards" — and the guard it used, a flexible frame plus `.clipped()`, does not do it. `scaledToFill` answers a proposal with a size that COVERS it, i.e. larger; `.frame(maxWidth: .infinity, maxHeight: .infinity)` then clamps that answer to infinity, which is no clamp at all, so the image's size propagates straight up through `aspectRatio(2/3, .fit)` and decides the tile. Measured offscreen with ImageRenderer, one LazyVGrid column at 170pt: 460x215 header, before poster 1750.9 x 818.4 tile 545.6 x 255.0 460x215 header, after poster 170.0 x 255.0 tile 170.0 x 255.0 300x450 cover, either poster 170.0 x 255.0 tile 170.0 x 255.0 A 545pt tile in a 170pt column is what the report is describing, and the cover case coming out right either way is why this only ever showed on the titles whose cover was missing. The fix gives the sizing role to something that has no opinion about it: `Color.clear` takes the proposal, and the art rides as its overlay, where it can be drawn but never measured. `LibraryCoverflowView` reuses `PosterImage` directly, so it is fixed by the same change. |
||
|
enricobuehler
|
f5f11f84f0 |
fix(plugin-kit): the Steam art scan knew one filename and one layout, so most covers were never found
Forza Horizon 6 shows a banner where its cover should be. The cover is on
disk the whole time: `librarycache/2483190/711e39.../library_capsule.jpg`,
300x450, the exact asset Steam itself draws. The scan never looked for that
name, fell back to the flat CDN URL for `library_600x900.jpg`, which 404s for
this appid, and the client then walked its candidate list down to the header —
a 460x215 banner in a 2:3 poster slot.
Measured against a real 779-app `appcache/librarycache` (the .41 box), the scan
was missing far more than one title. Three findings, each independent:
* `library_capsule.jpg` is the newer name for the 2:3 cover. 46 appids carry
only that name; none carry both it and `library_600x900.jpg`.
* `header.jpg` is the newer name for the header. 594 appids carry it, 122
carry `library_header.jpg`, and again no appid carries both — so the one
name we knew covered 16% of them.
* The `<appid>/<name>` layout, with no hash dir in between, is the MAJORITY:
623 of 779 appids. `findLocalArtFile` walked only `<appid>/<hash>/<name>`
and the oldest `<appid>_<name>` form, so it saw none of them.
The renamed files are the same assets — every `library_capsule.jpg` in that
cache measures 300x450, the same as every `library_600x900.jpg`, and both
header spellings measure 460x215 — so this is purely about knowing to look.
Simulating the resolver over that cache, per-appid art found locally:
portrait 25 -> 328 hero 75 -> 323
header 86 -> 716 logo 66 -> 295
None of this was visible before because a title that resolves no local file
still gets a CDN URL, and for anything Valve has not re-hashed that URL works.
It is the newer titles — the ones whose flat CDN URL 404s — that lose their art
outright, which is why this reads as "some games" rather than "the library".
|
||
|
enricobuehler
|
fb9f605120 |
Merge pull request 'The grey native-vulkan stream: a host re-anchor claim the client could not check' (#262) from worktree-grey-frame-reanchor-fixes into main
Reviewed-on: unom/punktfunk#262 |
||
|
enricobuehler
|
9e71cda298 |
fix(client-linux): the library grid never got the FlowBox recursion guard, so every game click overflowed the stack
Clicking any game in the host library aborted punktfunk-client outright:
thread 'main' has overflowed its stack
fatal runtime error: stack overflow, aborting
A click on a FlowBoxChild emits `child-activated` on the *FlowBox*, never the
child's own `activate`, so a grid whose per-card handler hangs off
`child.connect_activate()` has to bridge the two. The naive bridge is a cycle:
FlowBoxChild's default `activate` handler re-emits `child-activated` on its
parent, which re-enters the bridge, which activates the child again.
|
||
|
enricobuehler
|
bee924162a |
feat(client): the desktop client decoded one plane, at one rate, and read the engine's opinion of neither
Work package H5, client half — Linux and Windows. `AudioDec` grows a PCM arm behind the same `decode`/`conceal` pair, so the pull loop is plane-agnostic and `AudioGapTracker` still decides how many frames are missing. The lossless plane conceals with `PcmConceal` rather than libopus, because a raw frame carries no decoder model to extrapolate from. A codec the client cannot decode is now refused out loud and the session runs video-only, instead of a `0xD3` payload being fed to libopus. The conceal scratch was `5760 * channels` — 120 ms at 48 kHz, and an overrun at 96 kHz. It is now 120 ms at the resolved rate for Opus, or exactly one negotiated frame for PCM. On Windows the engine trap is closed on both sides of the connect. `can_render_at` reads the render endpoint's own mix format before the handshake and withholds the capability bit when the engine cannot carry the rate — the client-side twin of the host rule, and the only point at which declining is still free. `render_thread` reads it again before initialising and warns if the endpoint moved underneath us. `autoconvert` would otherwise have silently downsampled a 96 kHz stream on arrival and wasted the entire exercise while every log line agreed the session was hi-res. Both graphs stay 32-bit float, deliberately and with the reasoning at the site: core hands both planes over as f32, so an S24 graph would re-quantise the very samples the plane exists to deliver unrounded. §9 asks a client whose device refuses the rate to "say so and fall back". That is not implementable where the doc puts it — by the time the device opens, the wire format is fixed by the Welcome and the plane cannot switch mid-session, so falling back would mean killing audio. The decision moved to where it is still free: gate before connect, report loudly at open. PipeWire's `NODE_LATENCY` was the string literal "240/48000"; it is built at runtime now so the graph quantum stays one protocol frame at any rate, and a new `param_changed` handler logs what the graph actually granted. Verified on Linux in a container: check, clippy -D warnings, build and 210 tests, all green. `audio_wasapi.rs` compiles nowhere available here — macOS cannot build the crate at all and the MSVC cross target dies in openh264's C build — so Windows CI is its first compiler. |
||
|
enricobuehler
|
45c37f1a3b |
feat(apple): the ring measured milliseconds in a constant nobody had told about the negotiated rate
Work package H6, Apple half. The Swift client is the one that PORTS the de-jitter policy rather than draining core's decoded frames, so every hardcoded 48 in `AudioRing` was a second, silent copy of an assumption core had already stopped making. `perMS`, the one-second ring capacity, and `AvSync`'s weight all follow the resolved rate now, and the frame length follows the resolved `audio_frame_us` through the accessor core grew for exactly this. That frame length matters in four places, all of which were quietly wrong on a lossless session: the target floor is a device quantum plus one frame, the smooth shed drops one frame, the seam crossfade is capped at half of one, and the near-miss margin means "less than one packet in hand". Frozen at 5 ms against a 2 ms frame the last of those means two and a half packets, which grows the target on a ring that was never close to starving. Core has been changed to agree. Frame geometry is computed in microseconds. 2 500 µs is the one rung on the ladder where routing through integer milliseconds silently loses a fifth of the frame, and every other rung is ms-exact — which is precisely what would have let that bug hide. `setPreferredSampleRate` now runs on every branch (the `.playback` ones, which are mic-off iOS and all of tvOS, asked for nothing at all before) and runs before `setActive`, since that is when the hardware is configured. What the route actually granted is read back afterwards and warned about when it differs. The graph is deliberately built at the RESOLVED rate rather than the granted one. The source node's format describes the samples, which arrive at the wire rate; declaring the device's 48 kHz while writing 96 kHz samples plays the stream at half speed. And a literal fallback is impossible anyway — the plane never switches mid-session — so the honest behaviour is to let the engine resample and SAY SO, which is report-not-refuse rather than the silent resample the design forbids. Settings gain an audio-format choice defaulting off, stereo-gated, with the same raw values the Android leg uses so a profile round-trips between the two clients. The HUD shows the RESOLVED format, because a declined lossless session that still says "lossless" is the same class of lie as claiming a rate you did not get. |
||
|
enricobuehler
|
ec1e7be7dd |
feat(android): AAudio grants a rate or fails, so the fallback had to happen before the Hello
Work package H6, Android half — plus the near-miss margin in core, which the Apple leg independently proved was measuring the wrong thing. AAudio never substitutes a rate: an explicit request is granted or the open fails. So a 48 kHz rung sitting below a 96 kHz session on the open ladder could only ever produce playback at 2x speed, or a resample the design forbids in as many words. The fallback therefore runs BEFORE the handshake: a probe opens a stream at the requested rate, reads back what was granted and closes it without ever starting it, so nothing routes and no audio focus is taken. The ladder is 96 -> 48 keeping the depth -> the legacy pair, so a device that will not grant 96 kHz still gets a 48/24 lossless session rather than silence. Only 96 kHz is probed; a default session opens nothing and pays nothing. The open ladder still gains a rate dimension, because a probe is one measurement at one instant and the supervisor reopens across route changes. Rate is the outermost dimension, and the last rung is AAUDIO_UNSPECIFIED for the HAL that refuses an explicit request while already running at exactly the rate we wanted — `arm` still holds it to the session's rate, so that rung can rescue a session but never mislabel one. The conceal scratch is now sized per plane: Opus keeps its 120 ms legal maximum, PCM uses the longest ladder rung. Sizing both from the Opus figure was 24x too large at 96 kHz, and sizing both from the PCM one would have been far too small for Opus. The poll tick is one frame of the running plane rather than a fixed 5 ms, so on a 2 ms session the drought-concealment arm keeps pace with playout instead of losing 3 ms in 5. In core, the near-miss margin stops being a constant. Its own doc always said "less than one protocol frame was left in hand", but it was frozen at 5 ms — which against a 2 ms lossless frame means two and a half frames, so it grew the target on a ring that was never close to starving, inverting exactly what a near-miss exists to detect. Identical on every Opus session. The Android CI lane's test filter is an allowlist and eleven classes sat outside it, including the audio HUD ones, which had never run in CI at all. All eleven pass, so nothing was hiding — but nothing was gating either. They are listed now, explicitly rather than by glob, because the unfiltered task also drags in the screenshot scenes. |
||
|
enricobuehler
|
b4255631ac |
feat(host): the gate could not tell whether capture would really deliver the rate it was promising
Closes the one place the implementation and the design did not meet, plus two constants that still measured time in the Opus frame. §8.4 says hi-res resolves only if "the capture path can genuinely deliver the rate", but the capturer is opened by the audio thread, well after `negotiate` builds the Welcome. The gate could only check that the format was well-formed; the truth arrived later and the audio thread ended the plane — a silence outcome the design calls unacceptable. The rate is now answered before the Welcome, by a probe that opens no stream and changes nothing about the box. It is three-valued on purpose. Windows reads the endpoint's own mix format via a read-only rerun of the wiring plan — deliberately NOT the real wiring pass, which parks defaults, mints endpoints and logs, none of which may happen mid-handshake for a session that is about to resolve to Opus anyway. Linux needs no query at all: in stream-sink mode the host declares the format itself, so it is honest by construction, and monitor mode cannot know, so it declines. Anything else, or any probe failure, is Unknown and declines too — unlike the wiring plan, where an unknown format means "assume it's fine", here it means "we cannot prove content matches label", which is the whole point of the feature. The audio thread's per-iteration check stays, with its role reduced to the probe-to-open race — an operator changing the endpoint format, a hotplug re-plan, a graph renegotiation. Its action is unchanged because there is still only one correct one: the plane cannot switch to Opus mid-session, and mis-clocked samples under the promised label is precisely what this feature exists to prevent. `INFILL_AFTER` and `LATE_DEPARTURE` were both denominated in the 5 ms Opus frame while documenting themselves as "one protocol frame". On a 2 ms lossless frame the first spent its 500 ms infill budget in 200 ms of real time and the second let a two-frame slip read as on-time. Both now derive from the session's resolved frame. `INFILL_MAX` deliberately stays wall-clock and says why. `SendStats` lost its `Default` in the process, deliberately: a zero frame makes every departure compare as late, and since the window is rebuilt on each 30-second flush that would not even have been a once-per-session mistake. `PUNKTFUNK_AUDIO_HIRES` is now documented for operators, with the cost stated plainly and without overselling — the design's own §12 says hi-res is likely inaudible for game content and that the real win is bit-exactness. |
||
|
enricobuehler
|
a81c6ac789 |
feat(core): a ported de-jitter policy had no way to learn the real frame length
Two independent workstreams hit the same wall: `PUNKTFUNK_AUDIO_FRAME_MS` is the Opus plane's 5 ms, the lossless plane runs 4 ms at 48/24 and 2 ms at 96/24, and nothing in the C surface carried the resolved value. An embedder that PORTS the de-jitter policy rather than draining `next_audio_pcm` — the Apple client does exactly this — was stuck compiling against 5 ms, which sheds 2.5 frames at a time on a 96 kHz session and puts the target floor a frame too deep. The earlier judgement that no C embedder needs this rested on `next_audio_pcm` reporting each frame's real length in `frame_count`. That is not true in the case that matters: concealed frames are PREPENDED into the same buffer, so the count answers "how many samples did I get", not "how long is one frame". The two questions coincide only when nothing was lost. Microseconds, not milliseconds — the ladder has sub-millisecond rungs and 2 500 µs must not truncate to 2 ms. `0` means the host stated nothing, in which case the old constant is right. Added, not widened, like the rate and depth accessors before it. |
||
|
enricobuehler
|
88d29d3615 |
chore(core): regenerate the C header for the FRAME_MS note
cbindgen picked the doc comment up on the next build; the checked-in header must not disagree with the source it is generated from. |
||
|
enricobuehler
|
099904967d |
docs(core): PUNKTFUNK_AUDIO_FRAME_MS is the Opus frame, and a ring sized from it is wrong on the lossless plane
`PUNKTFUNK_AUDIO_SAMPLE_RATE_HZ` got a paragraph explaining that it is the default rate and that hi-res sessions must use the accessor. Its sibling `PUNKTFUNK_AUDIO_FRAME_MS` got nothing, and it has the identical hazard: the lossless plane negotiates 4 ms at 48/24 and 2 ms at 96/24, so an embedder sizing a playout ring as frames × PUNKTFUNK_AUDIO_FRAME_MS is wrong by up to 2.5x with nothing in the C surface saying so. Both constants are KEPT unchanged — embedders size rings from them and deleting either is a silent C break. What was missing is the note. No accessor is added because `next_audio_pcm` already reports each frame's real length in `frame_count`, which is the figure to size from; the comment now points there. |
||
|
enricobuehler
|
d09b4668c6 |
fix(core): the cheapest lossless rung was the one format nobody could ask for
Three follow-ups from the lossless plane. The hi-res capability bit was derived from "the caller asked for something other than 48 kHz/16-bit". That rule is wrong at exactly one point, and it is a point that matters: 48/16 is both the default AND the cheapest lossless rung (1.5 Mbps against Opus's 256 kbps), so it was the one format on the ladder no caller could request. The bit now means "the caller SPECIFIED a format", with 0 as the unspecified value. No wire byte and no ABI change — the wire already encodes an explicit 48000/16 identically to absent, and the capability bit is what carries the intent. That inverts a default, so every legacy entry point had to move with it: `connect` and the four pre-v24 `punktfunk_connect_ex*` variants passed an explicit 48000/16 and would otherwise have started advertising hi-res on behalf of embedders that never asked. They now pass 0/0, and the comment at each site says why the distinction is load-bearing. `JitterPolicy` had two decisions denominated in FRAMES rather than milliseconds — the floor under the effective target, and the smooth shed's one-frame drop — both written when 5 ms was the only frame this protocol had. The lossless plane negotiates 4 ms at 48/24 and 2 ms at 96/24, where the old constants shed 2.5 frames at a time and faded across an entire frame. `set_frame_us` fixes both and caps the seam crossfade at half a frame, because a fade as long as the material it is fading is not a crossfade. It is a setter rather than a constructor argument for two reasons: the default keeps every Opus session and all seventeen policy tests bit-identical, and `audio_frame_us` is resolved by the host and only known after the Welcome — later than the ring is built. Frame length is computed in microseconds so 2 500 µs does not truncate through integer milliseconds. `audio_kbps()` reports what the plane costs, for the stats line §4.6 asks for. It answers only for PCM, and the answer is exact rather than measured: PCM is constant-bitrate by construction, so a byte counter would add sampling noise to a number already known precisely. Opus is VBR with a host-side ladder position, so the client has no honest figure and says None instead of inventing one. |
||
|
enricobuehler
|
cef4b849e7 |
fix(client): a Linux-only helper was gated on test, so the Windows client's clippy lane could not build
`sink_from_props` carries `cfg(any(target_os = "linux", test))` like its neighbours in this module, but unlike them it takes a `pipewire::` type in its signature — and `pipewire` is a Linux-only dependency. The bare `test` arm therefore compiles it in test configuration on EVERY platform, where the crate does not resolve. It builds green in release, because nothing reaches the function off Linux. It only fails under `clippy --all-targets`, which builds the lib test target — so the Windows client's release build passes and its clippy lane goes red. That is how it reached main. Its only caller, `walk_graph`, is already `cfg(target_os = "linux")`, and no test calls it, so the gate simply becomes Linux-only. The neighbouring helpers keep their `test` arm: their signatures are plain data, which is the whole point of that pattern. Pre-existing on main and unrelated to the audio work in this branch; fixed here because it blocks this PR's Windows lane. |
||
|
enricobuehler
|
761895ec46 |
feat(host): capture was pinned to 48 kHz and one oversized frame could silence audio for the whole session
The capture facade, both backends and the encode loop take a rate; the encode loop gains a PCM branch; and the session resolves which plane it runs at handshake. `AudioCapturer` grows `sample_rate()` alongside `channels()`, and both backends report what they ACTUALLY opened rather than what was asked for. That distinction is the feature: on Windows `AUDCLNT_STREAMFLAGS_AUTOCONVERTPCM` means asking a 48 kHz engine for 96 kHz succeeds, returns no error, and hands back interpolation — a session that would log 96 kHz, spend the bandwidth, and carry nothing above 24 kHz. The endpoint's own mix format was already being read and logged one line before the request; it is now read BEFORE the format is chosen and the request is capped to it. The design said to decline outright when the engine is slower. Taken literally that regresses the shipped Opus path: a 44.1 kHz endpoint is an ordinary Windows configuration, and declining down to 44,100 would hand libopus a rate it does not accept. The decline is floored at 48 kHz, so only a hi-res request can lose. The resolve gate is the five conditions from the design, each declining to Opus with a logged reason — a fallback is not a failure, but an unexplained one is. Its cost check compares the plane against the video bitrate because audio rides datagrams outside the ABR loop: whatever it takes is off the top and ABR can neither see it nor reclaim it. A request that does not fit declines rather than being quietly handed a cheaper rung. One condition cannot be answered where the design puts it. "The capture path can genuinely deliver the rate" is unknowable at handshake, because the capturer is opened by the audio thread well afterwards. The gate checks format validity; the audio thread checks the truth every iteration and, on the PCM plane only, ends the plane rather than shipping mislabelled samples. Ending it is a silence outcome the design calls unacceptable, and it is confined to a 96 kHz opt-in whose endpoint turns out to run at 48 kHz — the real fix is probing capture before the Welcome, which this pass does not do. Separately, a pre-existing bug this feature makes reachable: the audio and pad-audio send paths treated ANY `send_datagram` error as "connection gone" and tore the plane down for the rest of the session. Three of quinn's four error variants are not that. `TooLarge` now counts and drops one frame, `UnsupportedByPeer`/`Disabled` end the plane once with a reason, and only `ConnectionLost` breaks the session. The counts join the existing 30-second egress line. Note that the send BUFFER's drops remain invisible: quinn evicts oldest-first and returns Ok, so "buffer full" never reaches a caller. The counter here is TooLarge only. `PUNKTFUNK_AUDIO_HIRES` defaults OFF, unlike `PUNKTFUNK_AUDIO_REDUNDANCY` — this spends bandwidth the user did not previously agree to, so it is asked for on both ends. GameStream capture stays 48 kHz permanently and says why at the call site: Moonlight's protocol is Opus 48 kHz and is not ours to renegotiate. |