Skip to content

Fix three issues found in #666 - #683

Open
alansley wants to merge 2 commits into
e-dream-ai:fix-resume-busyloop-streaming-fallbackfrom
alansley:review-fixes-666
Open

Fix three issues found in #666#683
alansley wants to merge 2 commits into
e-dream-ai:fix-resume-busyloop-streaming-fallbackfrom
alansley:review-fixes-666

Conversation

@alansley

@alansley alansley commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Three follow-ups on fix-resume-busyloop-streaming-fallback, found reviewing #666. Based directly on 47f25e9, so this should merge into that branch cleanly and fold into #666.

1. Player.cpp — the resume clamp lost its unconditional upper bound. The new check only clamps when the dream's metadata frame count is known, but Dream::frames is an uninitialised int that CacheManager leaves at 0 whenever the server payload omits "frames". So a wrapped last_played_frame (the uint32 wrap from the old VAAPI PTS bug) on a dream with no length metadata goes through unclamped and hangs the decoder at startup — the exact failure the deleted 24h@60fps bound was there to prevent. Restored as an unconditional first check, with the metadata bound as the tighter second one. Both now reset to frame 0 rather than frames - 1: metadata can under-report the real length, and resuming to a single frame that immediately skips on is worse than starting the dream over.

2. JSONStorage.cpp — the new try/catch also catches writes. GetOrSetValue is shared by Get and Set, and the Set callbacks assign through as_bool() / as_uint64(), which throw on the same kind mismatch. A hand-edited value of the wrong type makes Set a silent no-op that still returns true and marks the store dirty — and since Settings::Get repairs a failed read by calling Set + Commit, the repair silently fails too. The bad value becomes unfixable from inside the app, and every read of that key warns and rewrites the whole settings file. Writes now replace the mismatched node with one of the right kind and let the callback write into it.

3. PlaylistManager.cpp — the streaming fallback needs a leash. The dream it returns is by construction not on disk and, never having been preflighted for streaming, has no URL either — so the caller resolves one over the network, synchronously, while holding the update mutex. Offline (the usual reason the cache is empty) that fails, the decision is cleared, and the next update repeats it: the busyloop traded for a blocking one, a timeout per cycle. Now skipped entirely in offline mode and rate-limited to one attempt per 10s otherwise, which also bounds the repeated refetch on a single-entry playlist where the "next" dream is necessarily the one that just played.

Builds clean on Linux (CMake/Vulkan). Not runtime-tested against the offline/empty-cache path — that one's from reading.

🤖 Generated with Claude Code

amillionbouncyballs and others added 2 commits July 31, 2026 13:44
* ⚠️  WARNING — DEFAULT CACHE LIMIT IS 10 GB ⚠️
  settings.content.cache_size defaults to 10 GB (CacheManager.cpp).
  On first run with a large existing cache the eviction logic will
  PERMANENTLY DELETE cached dream videos until the total is under
  10 GB. If you have a large cache, set this key BEFORE running:
    ~/.config/infinidream/settings.json:
      "settings.content.cache_size": <desired GB as integer>
  This is pre-existing behaviour, not introduced by this commit, but
  newly relevant as Linux users may be running the client for the
  first time after a long beta period with accumulated content.

Linux: add GUI sign-in wizard (closes e-dream-ai#622)

Implements a 3-step ImGui sign-in wizard for the Linux client,
replacing the previous console prompt. Matches the look and flow of
the Mac/Windows wizard (ported from FirstTimeSetupWin32.cpp).

New files:
  client_generic/Client/FirstTimeSetupVulkan.h
  client_generic/Client/FirstTimeSetupVulkan.cpp

    3-step wizard: email entry → OTP verification → tips/completion.
    Light-sheet ImGui style matching Mac/Windows appearance.
    System font loading with fallbacks: NotoSans → DejaVu →
      Liberation → ImGui default.
    Keyboard input via xkb_state_key_get_utf8 for full UTF-8/
      international layout support.
    Clipboard paste via Ctrl+V and Shift+Insert, backed by wl-paste
      with xclip/xsel fallbacks.
    Async OTP send/verify with atomic completion flags.
    Playback paused while wizard is visible, restored on close.

Modified:
  EDreamClient.cpp        — ESHasFirstTimeSetupCallback() added so the
                            console login path can be skipped when the
                            GUI wizard is registered.
  client.h                — Linux pre-window auth block now skips the
                            console prompt entirely when the GUI wizard
                            is registered.
  client_linux.h          — Registers the wizard callback before
                            CElectricSheep::Startup() runs; suppresses
                            event loop while wizard is visible.
  RendererVulkan.cpp      — Calls FirstTimeSetupVulkan_DrawIfNeeded()
                            inside the existing ImGui frame.
  DisplayVulkan.cpp       — Feeds keyboard and mouse Wayland events
                            into the wizard. Renamed mouse-moved
                            callback (s_mouseCallback →
                            s_onMouseMovedCallback) for clarity.
  PlatformUtils_Internal.h / PlatformUtils_Linux.cpp
                          — Renamed PlatformUtils_GetMouseCallback()
                            to PlatformUtils_GetOnMouseMovedCallback().
  CMakeLists.txt          — Added FirstTimeSetupVulkan.cpp to sources.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* Linux: settings dialog, playlist status in Disk tab, downloader warning, ops runbook

Three changes born from a debugging session where the client appeared to be
stuck downloading nothing despite healthy quota and disk space. Root cause was
the account being assigned a 4-dream developer test playlist server-side; the
downloader was correctly idle because everything in that playlist was already
cached. None of this was visible without reading logs carefully.

1. Linux settings dialog (SettingsDialogVulkan)
   Full ImGui-based settings dialog for Linux, mirroring the Windows/macOS
   equivalents. Tabs: Account, Controls, Disk, Display, Advanced.

2. Playlist status in Disk tab
   The Disk tab now shows a "Playlist" row with the current playlist name,
   cached/total dream count (e.g. "4 / 164 downloaded"), and the live
   downloader status string. A misconfigured account — stuck on a test
   playlist with everything already cached — is now immediately visible
   when opening Settings, without needing to grep log files.

3. Downloader warning when all playlist dreams are cached
   DreamDownloader::FindDreamsThread now logs a one-shot warning when
   getNextUncachedDream() returns null while quota is still healthy:
     "All N dreams in playlist \"...\" are already cached — nothing to
      download. If unexpected, check your server-side playlist assignment
      (remaining quota: X.XX GB)."
   Also sets the download status string to "All playlist dreams cached"
   so it surfaces in the settings dialog immediately.

4. FORCE_PLAYLIST_CHANGE_RUNBOOK.md
   Documents the full procedure for switching a user's active playlist.
   There is no REST endpoint for this — it requires a Socket.IO event on
   the /remote-control namespace (not the default / namespace, which
   silently accepts connections but ignores all events). Includes the
   Python script, how to find production playlist UUIDs, confirmation
   steps, and the key facts that make this non-obvious.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: Linux .gitignore cleanup and Mac/Windows auth regression

LinuxBuild/.gitignore was missing rules for in-source CMake artifacts
(CMakeCache.txt, CMakeFiles/, Makefile, cmake_install.cmake), runtime
assets copied to the build root by CMake (config.xrc, shaders/, *.png,
*.ttf, *.xpm), generated Wayland protocol glue (xdg-shell-client-protocol.h/c),
and runtime caches (deps-cache/, model-cache/, build-rife/). These all
accumulate when someone runs cmake . in-place instead of cmake -B build.
Stale leftovers from a previous in-source build have been deleted rather
than ignored where appropriate.

Fix a regression introduced in the previous commit where
EDreamClient::Authenticate() guarded LoginWithMagicLinkCode() with
!ESHasFirstTimeSetupCallback() without a LINUX_GNU guard. On Mac,
ESWindow.mm registers the first-time setup callback unconditionally at
window creation; Windows does the same in FirstTimeSetupWin32.cpp. This
caused ESHasFirstTimeSetupCallback() to always return true on both
platforms, silently skipping LoginWithMagicLinkCode() — a regression from
prior behaviour. The guard is now wrapped in #ifdef LINUX_GNU so only the
Linux code path skips the console auth flow when a GUI wizard is registered.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* Remove FORCE_PLAYLIST_CHANGE_RUNBOOK.md

* Linux: fix SettingsDialogVulkan dead code, X11 overlay input, HiDPI scaling, and wizard button sizing

Four issues identified in PR e-dream-ai#647 code review, plus a wizard trigger regression
and button sizing follow-up, all fixed in this commit.

1. SettingsDialogVulkan was compiled but never wired up
   - Added SettingsDialogVulkan.cpp to CMakeLists.txt sources
   - SettingsDialogVulkan_Register() called in CElectricSheep_Linux::Startup()
   - SettingsDialogVulkan_DrawIfNeeded() called in RendererVulkan::EndFrame()
     immediately after the wizard draw, before ImGui::Render()
   - IsVisible() guard added to HandleOneEvent() so the settings dialog blocks
     game key bindings while open (mirrors the existing wizard guard)
   - Ctrl+, mapped to ESShowPreferences() via new KEY_Comma case (XK_comma on
     X11, hardcoded evdev 51 on Wayland)

2. X11 sessions got a dead, inescapable overlay
   FeedKey / FeedMousePos / FeedMouseButton were only wired to Wayland callbacks.
   On X11 the wizard (and now settings dialog) rendered but accepted no input.
   - X11 KeyPress/KeyRelease: keysym saved before XFree(); FeedKey called for
     both overlays with evdevKey = keycode - 8 and nullptr xkb_state; event is
     consumed (continue) if either overlay accepts it
   - X11 MotionNotify: FeedMousePos forwarded to both overlays
   - X11 ButtonPress/ButtonRelease: X11 button numbers mapped to BTN_LEFT/
     RIGHT/MIDDLE (272/273/274) and forwarded via FeedMouseButton

3. Fonts hardcoded at 1.0× scale (no HiDPI)
   - New PlatformUtils_InitUIScale / GetUIScale / SetUIScale in
     PlatformUtils_Linux.cpp; scale stored as g_platformUIScale (clamped 1–4×)
   - InitUIScale() seeds from GDK_SCALE / QT_SCALE_FACTOR env vars so GNOME /
     KDE fractional scaling sessions get a reasonable default immediately
   - Wayland: wl_output listener bound in onRegistryGlobal(); geometry and mode
     events collect physWidthMm and pixelWidth; done event computes
     DPI = pixelWidth * 25.4 / physWidthMm and derives scale = DPI / 96.
     Integer wl_output.scale used as fallback when physical dimensions are absent.
     Multi-monitor: SetUIScale only called when the new value exceeds the current
     one, so a lower-DPI secondary monitor cannot overwrite the primary HiDPI scale.
   - X11: DisplayWidthMM() queried after XOpenDisplay; DPI computed the same way
     and passed to SetUIScale()
   - FirstTimeSetupVulkan and SettingsDialogVulkan both refresh g_uiScale from
     PlatformUtils_GetUIScale() at show time, before LoadWizardFonts /
     LoadDialogFonts, so fonts are always sized for the actual display DPI

4. Wrong issue reference in PR title — already fixed before this commit

5. Wizard not appearing after auth token cleared
   EDreamClient::Authenticate() guarded ESShowFirstTimeSetup() behind
   settings.app.firsttimesetup == false. Users who had previously completed
   setup (flag = true) never saw the wizard again after clearing their token.
   The guard is removed; the existing shownSettingsOnce per-session bool is
   sufficient to prevent repeated shows within one run. (Affects all platforms
   that share EDreamClient.cpp — macOS and Windows handled the flag separately
   in FirstTimeSetupManager.mm and were unaffected.)

6. Wizard button and input field sizing
   All interactive controls on the email and code steps were rendered in the
   15px body font, which looked too small relative to the 28px title text and
   the overall dialog proportions.
   - New g_fontButton loaded at S(20.f) (20 logical px at 96 DPI, scaled by
     PlatformUtils_GetUIScale(); e.g. ~23px on a 109 DPI display)
   - Applied via PushFont/PopFont to: Skip, Send code, Need an account?, Verify
     code, Try again. Send code uses a manual AddText call so it receives the
     explicit font pointer and size via CalcTextSizeA / AddText(font, size, ...)
   - Email input field also uses g_fontButton; y frame-padding reduced from
     S(10) to S(8) so field height = S(20) + 2×S(8) = S(36), flush with buttons
   - Button widths widened to fit the larger label text: kEmailSendBtnW 106→120,
     kEmailCreateAccountW 235→300, kCodeVerifyW 110→150, kCodeTryAgainW 88→120

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* Linux wizard: fix OTP step layout and bump all interactive text to button font

Three issues on the code-entry (OTP) step:

1. Error message was tiny and had no room
   kCodeErrorOffsetY=144 placed the error immediately at the bottom of the OTP
   input field (which ends at ~y=146), and kCodeVerifyOffsetY=158 left only 14px
   for the error text before the Verify button — far too little for even a single
   line of body text, let alone a wrapped multi-line message.

   Replaced the fixed-Y layout for the error+buttons section with flow-based
   positioning: cursor is set to kCodeAfterOtpY=152 (just below the OTP field),
   the error message (if present) renders naturally and pushes buttons down via
   Dummy() spacing, and Verify/Try again follow in flow order. kCodeStepPanelH
   expanded from 250 to 330 to give the flow room (worst case: 3 lines of error
   + spacing + two buttons ≈ 280px from kCodeAfterOtpY).

   Removed now-unused constants kCodeErrorOffsetY, kCodeVerifyOffsetY,
   kCodeTryAgainOffsetY.

2. Error message and "Verifying..." were in body font (S(15.f))
   Both now use g_fontButton (S(20.f)), consistent with buttons and the email
   input field. "Verifying..." renders via SameLine after the Verify button
   instead of an absolute SetCursorPos.

3. Email step validation error was also body font
   "Please enter your email address" / "Please enter a valid email address" now
   also rendered in g_fontButton.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* Linux: fix X11 ButtonReleaseMask, stale Escape in settings, Wayland Ctrl+,, and cursor visibility

- X11: add ButtonReleaseMask to XSelectInput so ImGui sees mouse-up events;
  without it the first click left the button "held" and blocked all subsequent clicks
- SettingsDialogVulkan: call ClearInputKeys() on show to discard stale Escape
  state carried over from closing the wizard
- Wayland: add XKB_KEY_comma to keysym switch so Ctrl+, reaches HandleOneEvent
- X11: add applyDefaultCursor() and toggle cursor visibility per-frame while
  any ImGui overlay is open

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* Linux: settings dialog and wizard UI polish — fonts, layout, and overlays

Settings dialog font fix (root cause):
ImGui 1.92 introduced FontSizeBase, which is initialised to 13 px (ProggyClean)
on the very first rendered frame before any custom fonts are loaded.  Setting
io.FontDefault alone does not change FontSizeBase, so all settings dialog text
was silently rendered at 13 px regardless of the font loaded at S(20).
Fix: wrap DrawSettingsDialog() in PushFont(g_regularUiFont, S(20.f))/PopFont()
so the font and its size are both active for the entire dialog, matching the
pattern the wizard already uses for every explicit PushFont call.  Also store
the loaded font in g_regularUiFont and re-apply io.FontDefault on every open so
a prior wizard run cannot leave a stale S(15) body-font pointer as the default.

Settings dialog layout changes:
- Font size bumped from S(17) to S(20) across all tabs (NotoSans loaded from
  system fonts; falls back to embedded ProggyClean if none found).  Diagnostic
  log lines record which font file was loaded and at what pixel size.
- Dialog height increased from S(390) to S(450) to accommodate taller text.
- Footer: Close button grown to S(90)×S(32), "?" button to S(24); the "?" is
  drawn via InvisibleButton + AddText on the draw list for pixel-perfect glyph
  centering rather than relying on ImGui button frame padding.
- Version string simplified to GetAppVersion() only (no git hash / build date);
  the real version will come from -DAPP_VERSION=X.Y.Z at release tag time.
- Controls tab: removed SetWindowFontScale(14/17) and all companion calls;
  section height S(160)→S(240), footer row height S(34)→S(40); button label
  "View Help Page" corrected to "View help page".
- Account tab: "Signed in as" text and "Sign out" button placed on the same
  row (previously on separate rows); dotRadius scaled via S(); action button
  height S(30)→S(36), width S(100)→S(110); formHeight for logged-in S(92)→S(100).
- Disk tab: fixed playlist panel text overwriting itself — the old code used
  GetItemRectMax().x (screen-space) as a window-local cursor position, pushing
  text off-screen.  Rewritten to use GetContentRegionAvail().x inside the child
  window for right-aligning the count, with SameLine for the playlist name and
  a clamped wrap position to prevent overlap.  playlistGroupH S(62)→S(72).
- Display tab: "Preserve Aspect Ratio" checkbox centred in the available area
  instead of being offset by fixed S(140)/S(80) constants.
- Advanced/Proxy tab: removed SetWindowFontScale(14/17); proxyContentInsetY
  S(22)→S(38) so "Use Proxy" clears the bold "Proxy" header; proxyGroupH
  S(160)→S(200) so the Password row is no longer clipped.

Wizard changes:
- Card width narrowed from 879→700 to match the settings dialog footprint.
- Card height S(580)→S(620) to absorb the taller S(20) tips text.
- "All set!" tips panel row heights grown (kThanksRowTopH 110→215,
  kThanksRowBotH 92→132) and copy/button font sizes unified at kUiTextSize=20.
- "Open web remote" and "Open playlist browser" button widths widened
  (118→186, 136→224) so the labels are not clipped at S(20) font.

Lato font regression fix (HUD overlay):
FontVulkan.cpp previously always used the embedded ProggyClean/ProggyForever
vector font.  Fixed to first attempt loading Lato-Regular.ttf from the same
directory as the running executable (where the build system already copies it),
falling back to the embedded font only when the file is absent.

CMake / build data:
- Added configure_file step that generates BuildData.json in the binary
  directory at cmake-configure time.  APP_VERSION defaults to "0.0.0" and is
  overridden via -DAPP_VERSION=X.Y.Z when cutting a release.  Also captures
  git short SHA and build date for internal reference.
- New BuildData.json.in template added alongside CMakeLists.txt.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* Linux: fix playlist playback freezes and Vulkan swapchain loop

Playback stability:
- UpdateTransition: reorder snap/freeze so HasFinished fires first —
  previous order let nextClipBuffering's early return block the snap,
  leaving a stalled outgoing clip frozen even when the incoming clip
  is also buffering
- Player::Update: don't consume m_nextClip frames until the crossfade
  starts; extends the existing seamless guard to all preloaded clips
  so the decoder fills the queue to its backpressure cap and playback
  begins near frame 0 instead of mid-stream (~3 s of content reclaimed
  per clip, giving preloading proportionally more lead time)
- IsPreloadComplete: lower threshold 10 → 5 frames to match the
  Rebuffering exit point and avoid discarding a live preloaded decoder
  for a fresh one
- Decoder: reset m_SeekTargetFrame = -1 after seek to stop re-seek
  loop that caused 21-second stalls; add av_bsf_flush after seek;
  diagnostic warnings for slow av_read_frame and EAGAIN spin

Vulkan swapchain:
- Respect driver-enforced currentExtent (required by spec); call
  SyncSize() after recreation so BeginFrame's mismatch check doesn't
  immediately trigger another recreation loop
- Skip frames when extent is zero (window minimised on some WMs)
- Add CDisplayOutput::SyncSize() for post-recreation size sync

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* Fix Linux playlist switching and Vulkan lifecycle

Serialize remote playlist work onto the player update thread, harden transition readiness, and correct X11/Vulkan synchronization and shutdown resource ordering.

* Prevent playlist switches from blocking on remote media

Prepare playlist metadata and streaming URLs on the network worker before applying a queued playlist change. Reuse the prepared URL when the player parses the playlist so server requests cannot stall the render thread.

Open remote FFmpeg decoders on their frame-reading worker and add interrupt callbacks for stop and shutdown requests. Keep local decoder opens synchronous so missing or corrupt cached files still fail immediately.

Propagate asynchronous open failures to clips and cancel incomplete transitions cleanly instead of waiting forever for frames. Also add a short backoff after download-link failures to prevent tight retry loops, especially while shutting down.

Validated with a Linux build and a 100-second X11 playlist-switch stress run covering seven switches and four successful remote decoder opens, including one that took over five seconds without blocking playlist processing.

* Package build metadata in Linux AppImage

Copy the CMake-generated BuildData.json beside the packaged executable so PlatformUtils can resolve the version, Git revision, and build date exactly as it does for an unpackaged Linux build.

Verified by rebuilding and launching the AppImage; the runtime banner reports revision e06dbff and build date 2026-07-22.

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
The 1-second retry sleep in FindDreamsThread used
boost::this_thread::sleep_for(boost::chrono::seconds(1)), which pulls
in the compiled boost::chrono library (steady_clock::now()). The
x86_64 slice of that library isn't linked, so universal builds failed:

  Undefined symbols for architecture x86_64:
    boost::chrono::steady_clock::now()
    referenced from boost::condition_variable::wait_for<...>
    in DreamDownloader.o

Use boost::this_thread::sleep(boost::get_system_time() +
boost::posix_time::seconds(1)) instead, which is header-only and
needs no boost::chrono link dependency.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@alansley alansley changed the title Review follow-ups for #666 (resume clamp, settings writes, streaming fallback) Fix three issues found in #666 Sep 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants