Statistics HUD: fix video zoom, two-column layout, drop redundant diagnostics#76
Statistics HUD: fix video zoom, two-column layout, drop redundant diagnostics#76oliversluke wants to merge 4 commits into
Conversation
|
Text "Session 68672b52 • 80-250-97-37. cloudma... |
The in-stream player laid the video and the HUD/pause-menu/time-warning out as siblings of a ZStack. The video is a UIViewControllerRepresentable, which — unlike a plain view — can expand to the union of its ZStack siblings' content. A tall Standard statistics HUD (especially with Diagnostics enabled) grew the ZStack past the screen, and the video's resizeAspectFill gravity then zoomed/cropped the picture to fill the taller bounds. Move the HUD/menu/warning into an .overlay of the video. An overlay is sized to the view it decorates and can never grow it, so the video's bounds stay fixed to the screen regardless of how tall the HUD gets. Positioning and transitions are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
703a722 to
7abf152
Compare
The Standard statistics HUD fits the screen comfortably on its own, but enabling Diagnostics appends the whole Debug section (~15-25 extra lines, several of them wrapping sentences), roughly doubling the panel height so the lower rows run off the bottom of the screen. When Diagnostics is on, lay the Standard HUD out in two columns — core stats (Network/Video/Audio/Session) on the left, Debug on the right — which roughly halves the height so everything stays visible. Compact and Standard-without-Diagnostics are unchanged (single column). Splits the panel/column layout helpers so the fixed 380pt width now lives on each column rather than the whole panel. Complements owenselles#76 (which stops the tall HUD from zooming the video); this keeps the extra Diagnostics rows on screen. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Standard statistics HUD fits the screen comfortably on its own, but enabling Diagnostics appends the whole Debug section (~15-25 extra lines, several of them wrapping sentences), roughly doubling the panel height so the lower rows run off the bottom of the screen. When Diagnostics is on, lay the Standard HUD out in two columns — core stats (Network/Video/Audio/Session) on the left, Debug on the right — which roughly halves the height so everything stays visible. Compact and Standard-without-Diagnostics are unchanged (single column). Splits the panel/column layout helpers so the fixed 380pt width now lives on each column rather than the whole panel. Complements owenselles#76 (which stops the tall HUD from zooming the video); this keeps the extra Diagnostics rows on screen. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The bottom of the Debug section carried six free-form sentence lines left over from before the structured Network/Video/Audio/Session/Debug rows existed. They largely repeat data now shown in those rows — e.g. the session-summary line duplicates Resolution/FPS/Codec, the color and decoded-video lines duplicate the Video Format row, and the app-queue / display-layer "dropped" counts overlap Video Drops/freezes. The German "App N in Warteschlange" reading of the ever-growing enqueued counter was also misleading. Remove the session-summary, app-queue, sample/convert, display-layer, color-diagnostic, and decoded-video lines, keeping the conditional fallback warning and RTC-log status. Also removes the now-orphaned code: GFNStreamController.diagnosticSessionSummary, the StatsHUDView detectedColorLabel helper, six AppLocalization formatters (diagnosticSessionSummary, displayLayerMetrics, colorDiagnosticStatus, decodedVideoStatus, hdrSupportLabel, decoderPathLabel), and their ten now-dead L10n keys across all 34 locale files. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@aarikmudgal Updated this PR to cover everything we discussed, and marked it ready for review:
Device-tested on Apple TV 4K. Ready for another look. |
|
@oliversluke I think the debug menu shouldn't be visible in "Release" run. Maybe only when the run is in debug mode.
And maybe the processing delay in the session metrics. rest lines only for debug run, what do you think? |
Developer diagnostics (the Debug HUD section, RTC event log) are now a DEBUG-build-only feature: - SettingsView hides the whole Diagnostics section under #if DEBUG. - normalizedForClient forces diagnosticsEnabled off under #if !DEBUG, so a persisted flag (set on a debug build or carried over) is never honored in Release — it runs at launch and before every session, so the Debug section, stats collection, and RTC event log all stay off. Since the Debug section disappears in Release, move the one metric worth keeping for end users — Processing delay (WebRTC totalProcessingDelay) — into the Standard HUD's Video section next to Decode time. Compact stays minimal (unchanged). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@aarikmudgal Good calls — both done in the latest commit: Diagnostics is now DEBUG-build-only. The Settings "Diagnostics" section is hidden under Processing delay stays visible. Since the whole Debug section disappears in Release, I moved that one metric (WebRTC So this PR now covers: video-zoom fix, two-column Standard HUD, redundant-diagnostics removal, DEBUG-only diagnostics gating, and the processing-delay move. Ready for another look. |
|
@oliversluke I think there was a misunderstanding, we need the diagnostics menu in release as well only this below section to be in Debug app mode. Also the processing delay can be moved to session on left column bottom.
So in release mode the standard diagnostic view will be one column (addition of Processing delay at the bottom) and when I debug mode we should have second column and in future extend more debug specific metrics in there. |
|
@aarikmudgal To make sure I build exactly what you have in mind (I think I over-gated it — killed the whole diagnostics feature in Release rather than just the low-level section), here's our current behavior per decision point. Could you fill in the right-hand column (or just correct my italic guesses)? Then I'll realign.
Once I have your answers I'll adjust the PR accordingly. |
|
@oliversluke Updated |
|
@aarikmudgal Thanks for the answers — before I realign, one important thing to flag, because it changes what "keep it in Release" actually costs. The "Diagnostics" toggle isn't the 2nd column — the column is just its visible readout. Flipping it on turns on developer instrumentation (the Settings subtitle even says "adds receiver timing, renderer metrics, frame counters and instruments signposts"). Concretely, ON does:
So the implication of "keep the toggle in Release, only build-gate the column": a Release user who turns Diagnostics on pays for all that instrumentation (+ optional 15 MB disk capture) and sees nothing change on screen, because the column they'd be enabling is build-gated away. It's not "the toggle does nothing" — it's "the toggle silently adds overhead for no visible benefit." This is really a developer-instrumentation feature, not a user display option. So I'd frame the choice as:
If you want something user-facing in Release, the RTC event log is the piece worth surfacing; the rest is developer-only by nature. Happy to go either way — I just want the tradeoff to be deliberate. To unblock what we already agree on: the video-zoom fix ( |
|
@oliversluke Indeed great points, lets go this way, by default the "Diagnostics" and "RTC Logging" are "Off" in the app but they are available. Even in Release bundle. |
|
Thanks for all the back-and-forth on this! To keep things moving, I've split the video-zoom fix into its own PR — it's independent and you've already confirmed it works: #78. That one's ready to merge on its own whenever you like. I'm marking this PR as draft for now so we can take our time getting the diagnostics side right. While implementing I noticed the boundary between the user-facing stats and the developer instrumentation is a little tangled in the current code (the "Debug" column is actually driven by the same lightweight once-a-second stats as the rest of the HUD, and some of the heavier per-frame capture isn't wired to anything anymore). I'd like to untangle that and write up a clearer proposal before we lock in the Release/Debug behaviour, then re-open for review. Appreciate your patience! |
|
No worries @oliversluke We work for the best of community :) So all good. Thank you for your investments... |
The Standard statistics HUD fits the screen comfortably on its own, but enabling Diagnostics appends the whole Debug section (~15-25 extra lines, several of them wrapping sentences), roughly doubling the panel height so the lower rows run off the bottom of the screen. When Diagnostics is on, lay the Standard HUD out in two columns — core stats (Network/Video/Audio/Session) on the left, Debug on the right — which roughly halves the height so everything stays visible. Compact and Standard-without-Diagnostics are unchanged (single column). Splits the panel/column layout helpers so the fixed 380pt width now lives on each column rather than the whole panel. Complements owenselles#76 (which stops the tall HUD from zooming the video); this keeps the extra Diagnostics rows on screen. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>




Three related fixes to the in-stream Statistics HUD, all around the tall Standard + Diagnostics case. (Folds in the two-column work that was previously #77.)
1. Stop the HUD zooming the video
The video and the HUD/pause-menu/time-warning were laid out as siblings of a
ZStack. The video is aUIViewControllerRepresentable, which — unlike a plain view — can expand to the union of its ZStack siblings' content; a tall Standard+Diagnostics HUD grew the layout andvideoGravity = .resizeAspectFillthen zoomed/cropped the picture. Moving the HUD into an.overlayof the video fixes it: an overlay is sized to the view it decorates and can never grow it.(Note: the earlier "padding inside the flexible frame" fix from #62 wasn't reverted — it just only stopped the padding from growing the layout, not a tall HUD's content height.)
2. Two-column Standard HUD when Diagnostics is on
Standard alone fits the screen, but enabling Diagnostics appends the whole Debug section and pushes the panel off the bottom. With Diagnostics on, the Standard HUD now lays out in two columns — core stats (Network/Video/Audio/Session) on the left, Debug on the right — so everything stays on screen. Compact and Standard-without-Diagnostics are unchanged (single column).
3. Drop redundant free-form diagnostics
The bottom of the Debug section carried six free-form sentence lines left over from before the structured rows existed, largely repeating data now shown structurally (session summary → Resolution/FPS/Codec; color & decoded-video → Video Format; app-queue/display-layer "dropped" → Video Drops/freezes). The German "App N in Warteschlange" also mis-read an ever-growing enqueued counter as a queue backlog. Removed those six lines plus the now-orphaned code: a
GFNStreamControllerproperty, a StatsHUD helper, sixAppLocalizationformatters, and their ten dead L10n keys across all 34 locale files. The conditional color-fallback warning and RTC-log indicator stay.Testing
Device-tested on Apple TV 4K (Standard + Diagnostics): no zoom, two columns fit on screen, lean Debug section. Simulator build + SwiftFormat + SwiftLint (strict) pass.
🤖 Generated with Claude Code