Codec default, settings picker fix, catalog perf, and concurrency cleanup - #89
Codec default, settings picker fix, catalog perf, and concurrency cleanup#89MonsieurNours wants to merge 6 commits into
Conversation
|
Thanks @MonsieurNours for putting these improvements together. I reviewed the PR against the latest Before merging, could you please address the points below? The main goal is to keep the useful changes from this PR without losing the newer performance and controller-navigation improvements from 1. Fix the Release build errorsThe current PR passes SwiftFormat and SwiftLint, but the unsigned Release tvOS build fails in two places inside
Both
Please update them like this: Task { @MainActor [self] in
self.applyDecodedVideoFormat(format)
}Task { @MainActor [self] in
self.handleSignalingEvent(event)
}This keeps the intended strong capture during the main-actor task and allows the Release build to compile. 2. Remove the Xcode project file changes from this PRCould you please remove all changes to The PR currently:
The target-level This makes the signing setup contributor-specific and difficult to reproduce in CI or on another developer’s machine. Signing configuration can be handled separately in another PR with a complete local and CI setup. For this PR, please revert the complete 3. Remove the duplicate hero-art prefetchThe existing .prefetchHeroArtOnFocus(game.heroImageUrl ?? game.heroBannerUrl)The PR adds another modifier to the same card path around line 251: .prefetchHeroArtOnFocus(game.heroBannerUrl ?? game.boxArtUrl)Because these modifiers use different URL fallback orders, focusing one game can start two separate image requests and decoding operations.
Please remove the additional modifier added around the grid card and keep hero prefetching centralized in 4. Rebase onto the latest
|
97440cf to
23c1ca3
Compare
H265 (HEVC) is hardware-decoded on Apple TV 4K and is the only codec that carries 10-bit/HDR10 through the app's custom decoder. The previous default H264 is 8-bit only; AV1 falls back to a software SDR path. Users can still switch manually.
- GFNStreamController: bind self to a named local (`guard let controller = self`) and let the main-actor Tasks bridged from the VideoSurfaceView and SignalingClient callbacks use it. This fixes the Release-configuration "Implicit use of 'self' in closure" errors while keeping the intended strong capture during the task, and stays SwiftFormat-stable (no redundant `self.`). - InputSender: mark RemoteInputMode.remoteActsAsMouse nonisolated so it can be read from nonisolated contexts. - CloudNowApp: register the token-refresh background task via the .backgroundTask scene modifier instead of reading the @State AuthManager in App.init, fixing the runtime "accessing State without being installed on a View" warning.
…heets On tvOS the Picker pushed-detail presentation rendered a blank screen for the stream-quality and controller settings and trapped controller focus with no way back. Replace those pickers with a reusable SettingSelectionRow that opens a padded modal list (SettingSelectionSheet), leaving the owenselles#92 server-location UI, toggles and steppers untouched. The sheet: - focuses the currently selected value on open via .defaultFocus (important for the long game-language list), - keeps the resolution TV-standards / Other grouping and SF Symbol labels, - renders the color-mode description in orange when AV1 is selected (software decode warning), - blocks global tab shoulder-button navigation while open via .blocksGlobalControllerNavigation().
- Seed expandedGame in GameCarouselView.init so a tapped game opens directly in its expanded card; focus lands on Play via the expanded detail's existing .defaultFocus, removing one controller action. Built on main's positionedGames layout and its handlesCarouselControllerNavigation(isEnabled:) gating. - Reduce the metadata reveal delay from 360 ms to 180 ms (reduceMotion bypass preserved) for a snappier open.
23c1ca3 to
49d497b
Compare
|
Thanks for the thorough review! I rebased the branch onto the latest 1. Release build errors — Fixed. I kept the strong capture during the main-actor task, but instead of view.onDecodedVideoFormatChanged = { [weak self] format in
guard let controller = self else { return }
Task { @MainActor in controller.applyDecodedVideoFormat(format) }
}Reason: the repo's SwiftFormat ( 2. Xcode project file — Reverted. 3. Duplicate hero-art prefetch — Removed. 4. Rebase onto 5. Settings sheets — Kept, with the requested adjustments:
Kept: Validation: SwiftFormat ( |
The modal selection sheets wrapped their List in a NavigationStack with .navigationTitle. On tvOS that floating title lets scrolled rows render through it (visible when defaultFocus scrolls to the selection) and the navigation bar's deferred layout pass flashes the list at the wrong offset on first presentation. Replace it with a fixed header above the List, matching the server-location screens. Also replace the SF Symbol Label in option rows with an explicit fixed-width icon column so resolution icons no longer collide with their labels.
The previous commit dropped the NavigationStack entirely, but the sheet
sizes to its content's ideal height and a bare List collapses without
it, leaving only the floating title. Keep the NavigationStack for
sizing, suppress its floating title with .navigationTitle(""), and
render the fixed header above the List — the same pattern
ServerPickerScreen already uses.
|
Thanks for testing! Points 1 and 2 are fixed in 8f71a11: the selection sheets no longer use a floating navigation title — the list was scrolling under it (the ghosted "Resolution" text), and the navigation bar's deferred layout pass was also causing the first-render flicker. The sheets now use the same fixed-header pattern as the server-location screens. The resolution icons also got proper spacing instead of colliding with their labels. Point 3 (rumble): I couldn't reproduce it — just retested on this branch and rumble works as expected on my setup. Nothing in this PR touches the input/rumble path ( |
|
@MonsieurNours Thanks for the changes. No Findings
|
| Review point | Status |
|---|---|
Release explicit-self errors |
✅ Fixed; unsigned Release build passes |
| Xcode project/signing changes | ✅ Removed; no project.pbxproj diff |
| Duplicate hero-art prefetch | ✅ Removed; LibraryView.swift is unchanged |
| PR #91 controller/catalog optimizations | ✅ Preserved |
positionedGames carousel optimization |
✅ Preserved |
Exact focus restoration and .defaultFocus |
✅ Preserved |
| Reduce Motion handling | ✅ Preserved |
| Carousel accessibility | ✅ Preserved |
| Modal controller-navigation blocking | ✅ Preserved |
| Current selection focused in sheets | ✅ Implemented |
| AV1 orange warning | ✅ Preserved |
| Resolution grouping, badges, and icons | ✅ Preserved |
| H.265 default with H.264 compatibility | ✅ Implemented |
| Existing stored codec selections | ✅ Preserved |
| Carousel opens directly in expanded state | ✅ Implemented |
| 180 ms metadata reveal | ✅ Implemented |
| UI overlap and first-render flicker | |
| Reported rumble regression |
UI overlap and flicker
Commits 96a4086 and 8f71a11 structurally address the reported UI problems:
- The floating navigation title was replaced with a fixed header.
- The empty navigation title prevents rows from rendering beneath the old title.
NavigationStackremains in place to preserve the sheet’s correct sizing.- Resolution icons now use a fixed-width column and spacing.
The implementation matches the reported cause, but first-presentation flicker is runtime behavior and cannot be conclusively verified through compilation or static review. Please repeat the original simulator or Apple TV interaction before closing this point.
Rumble regression
I did not find a code change in this PR that explains a rumble-only regression:
- Rumble enablement and intensity are unchanged.
ControllerHapticsand haptic packet handling are unchanged.- The only
InputSenderchange is thenonisolatedannotation on a pure computed property, which has no runtime behavior. - H.264 and H.265 continue using the same input protocol.
- Codec filtering affects video negotiation, not the input/haptics data channel.
Please A/B test using:
- The same Apple TV, controller, and game on current
main. - The same setup on PR head
8f71a11. - PR head with H.264 selected explicitly, to exclude the new H.265 default. (I will do the test once again myself)
Final verdict
There are no known BLOCKER or MAJOR code defects.
Before approval, I recommend:
- Rebasing the branch on current
main. (Auto rebase should work) - Restoring selected-state accessibility in the custom setting sheets.
- Making background refresh cancellation propagate correctly.
- Repeating the original sheet overlap/flicker scenario.
- Completing the rumble A/B test against current
main.
Once these points are addressed or conclusively verified, the PR should be ready for approval and merge



Summary
Groups several fixes and improvements made while getting the app building and browsing smoothly on Apple TV 4K.
Changes
Settings — modal selection sheets (fixes blank/trapped picker screens)
On tvOS, the stream-quality and controller
Pickers pushed a blank detail screen and trapped navigation (no way back — the app had to be force-quit). Replaced every picker with a reusableSettingSelectionRowthat opens a padded modal list, the same pattern as the already-working zone picker: compact rows plus a readable sheet with a checkmark on the selected value.Video codec default → H265
Changed the default codec from H264 to H265 (HEVC): it's hardware-decoded on Apple TV 4K and is the only codec that carries 10-bit/HDR10 through the app's custom decoder. H264 is 8-bit only; AV1 falls back to a software SDR path. Still user-switchable.
Catalog performance
GameGridnow prefetches the hero image on card focus (the Store only prefetched box art before), removing the cold-decode lag when opening a game.GameCarouselViewopens straight to the tapped game's expanded card so focus lands on Play, and the metadata reveal delay is cut from 360 ms to 180 ms.Concurrency / warning cleanup
GFNStreamController:guard let self+ explicit[self]capture in the main-actor Tasks bridged from the video/signaling callbacks — clears the Swift 6 "captured var self in concurrently-executing code" and "weak ownership differs" warnings.InputSender:RemoteInputMode.remoteActsAsMousemarkednonisolated.CloudNowApp: token-refresh background task registered via the.backgroundTaskscene modifier instead of reading the@StateAuthManager inApp.init(fixes the runtime "accessing State without being installed on a View" warning).Build — local code signing
Wired
Local.xcconfigas the base configuration for the project's Debug/Release configs and removed the hardcodedDEVELOPMENT_TEAM, so contributors set their own Team ID via the gitignoredLocal.xcconfig(perLocal.xcconfig.example).Testing