mirror of
https://github.com/4gray/iptvnator.git
synced 2026-10-16 14:16:16 -08:00
fix(playback): keep the saved Embedded MPV player when the mpv check is inconclusive (#1803)
* fix(playback): keep the saved Embedded MPV player when the mpv check is inconclusive On Linux native-view the support check runs `mpv --version` by bare name and waits for the login shell PATH first. Since #1784 that lookup is asynchronous with a 10 s budget; when it ran out, the check ran on the inherited PATH and answered a plain `supported: false`. The settings store took that as a verdict and persisted the default player over a saved Embedded MPV selection. The main process probed again once the shell answered, but nothing restored the setting. `EmbeddedMpvSupport` now carries `inconclusive`. The native service sets it on a missing mpv while its probe has only seen the inherited PATH; the IPC handler declares that state before probing and registers the re-probe before the check, so a throwing check cannot leave it stuck. Every other answer stays final. Consumers no longer settle on an inconclusive answer: the settings store keeps the saved player, and the command palette and the settings search probe again on their next use instead of caching it for the session. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(playback): keep asking for Embedded MPV support while the answer is inconclusive Keeping the saved player on an inconclusive answer left a mounted player stuck on it: the session controller asked for support once, in its constructor, and the session effect never starts while unsupported, so the player did not recover after the login shell answered. The settings page held its one answer the same way. `watchEmbeddedMpvSupport()` asks again every 3 s until the answer is final or the surface is destroyed. The player controller and the settings page facade load support through it, so playback starts by itself and the Embedded MPV option appears without reopening the page. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(settings): follow an inconclusive Embedded MPV answer to a final decision The settings store checked a saved Embedded MPV selection once. After an inconclusive answer it kept the selection and never looked again, so when mpv turned out to be really missing the player stayed on Embedded MPV instead of falling back to the default one. The store now follows the answer with `watchEmbeddedMpvSupport()` until it is final and only then decides. It acts on an answer only while Embedded MPV is still the saved player, so a player picked meanwhile, also while the first answer was pending, is never overwritten. The watch backs off from 3 s to 30 s between rechecks, so a login shell that never answers does not keep the app polling at the first rate, and it no longer schedules a recheck after its answer handler stopped it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(settings): keep the settings search following an inconclusive Embedded MPV answer The settings page asks the search service for Embedded MPV support once, when its search facade is created. After an inconclusive answer the service only probed again on its next call, so with the page left open the Embedded MPV rows stayed unsearchable after the login shell answered, while the player option on the same page already updated. The service now follows the answer with `watchEmbeddedMpvSupport()` until it is final, which updates the open page and the command palette alike. A call made while the answer is still inconclusive restarts the watch, so it asks at once as before. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(settings): follow Embedded MPV support for search only while the settings page is open The settings search service is provided in the root injector, so the watch it started on its first use had no owner: with a login shell that never answers it kept asking every 30 s until the app quit, long after the settings page or the command palette that needed the answer was closed. A failed recheck also ended the watch as if it were a final answer, hiding the Embedded MPV rows for the rest of the session. The service now separates the two uses. `ensureEmbeddedMpvSupportLoaded()` is a single request again, for the command palette. The settings page calls `followEmbeddedMpvSupport()` and ends it when the page is destroyed. Only a final answer is kept: after an inconclusive one or a failed request the next use asks again, for the palette's player commands too. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
1 parent
040acbd976
commit
bc5a7fcbf9
24 files changed
+1361
-85
No files matched your search
@@ -185,6 +185,39 @@ support API from global workspace startup paths; use an explicit user action
|
||||
or idle preparation path when a renderer surface only needs to reveal optional
|
||||
Embedded MPV UI.
|
||||
|
||||
An unsupported answer can be `inconclusive`. The Linux native-view `mpv`
|
||||
executable check runs `mpv --version` by bare name, so the support and prepare
|
||||
handlers wait for the login shell PATH lookup (`startup/login-shell-path.ts`)
|
||||
first. When that lookup runs out of its budget, the check runs on the
|
||||
inherited PATH: `EmbeddedMpvNativeService` then reports a missing `mpv` as
|
||||
`supported: false` with `inconclusive: true`, keeps doing so while the cached
|
||||
result stands, and probes again once the shell answers. Every other answer,
|
||||
including a missing `mpv` after the shell answered, is final. An inconclusive
|
||||
answer is not a verdict on the machine: never persist a decision made from it.
|
||||
Whatever holds on to one answer follows it through `watchEmbeddedMpvSupport()`
|
||||
(`@iptvnator/shared/interfaces`), which asks again after
|
||||
`EMBEDDED_MPV_SUPPORT_RECHECK_MS`, backing off to
|
||||
`EMBEDDED_MPV_SUPPORT_RECHECK_MAX_MS`, until the answer is final:
|
||||
|
||||
- The settings store keeps a saved Embedded MPV selection while the answer is
|
||||
inconclusive, falls back to the default player only on a final unsupported
|
||||
answer, and never overwrites a player the user picked meanwhile.
|
||||
- The player (`EmbeddedMpvSessionController`) and the settings page stay
|
||||
mounted on one answer: a player mounted in that window starts playback by
|
||||
itself once `mpv` is found, and the option appears without reopening the
|
||||
page.
|
||||
- The settings page also follows the answer for its search
|
||||
(`SettingsSearchService.followEmbeddedMpvSupport()`) and ends that when it
|
||||
closes: the Embedded MPV rows become searchable while the page stays open,
|
||||
and nothing keeps asking once no surface shows them.
|
||||
|
||||
The command palette asks on demand, on every open, for its player commands and
|
||||
its settings rows (`ensureEmbeddedMpvSupportLoaded()`). Only a final answer is
|
||||
kept for the session; after an inconclusive one, or a failed request, the next
|
||||
open asks again. An open palette is a snapshot of that moment: it does not wait
|
||||
for a final answer, because a login shell that never answers would then keep
|
||||
it from opening.
|
||||
|
||||
When `embedded-mpv` is the saved player, the settings store schedules an idle `prepareEmbeddedMpv()` call. This intentionally moves the first native addon load away from the click-to-play path. It can still block the Electron main process briefly because Node native addon loading is synchronous, but doing it during idle is less visible than doing it when the user clicks a video. Actual MPV session creation still happens on playback because it needs the current Electron window handle and viewport bounds.
|
||||
|
||||
For the native-view engine, the MPV video surface is a platform view/window,
|
||||
@@ -967,7 +1000,7 @@ Defensive practice for this component:
|
||||
|
||||
Concrete bugs from the audit, recorded so they don't get reintroduced:
|
||||
|
||||
- **Infinite session-create loop.** `EmbeddedMpvSessionController.startSession` once wrote `this.support.set(prepared)` after the `prepareEmbeddedMpv` round-trip. The component's session-creation effect tracks `this.support()`, so the write fired the effect → cleanup disposed the session → new session was created → prepare ran again → support was set again. Symptom: endless "Loading stream…" spinner. Fix: do not write `support` inside `startSession`; the constructor's `loadSupport()` already populates it including capabilities.
|
||||
- **Infinite session-create loop.** `EmbeddedMpvSessionController.startSession` once wrote `this.support.set(prepared)` after the `prepareEmbeddedMpv` round-trip. The component's session-creation effect tracks `this.support()`, so the write fired the effect → cleanup disposed the session → new session was created → prepare ran again → support was set again. Symptom: endless "Loading stream…" spinner. Fix: do not write `support` inside `startSession`; the constructor's `watchSupport()` already populates it including capabilities.
|
||||
- **Stream restart on volume change.** The session-creation effect once read `this.volume()` directly to pass to `startSession`'s `initialVolume`. Each volume tick re-ran the effect, disposing and recreating the session — for VOD/series this restarted playback from the beginning. Fix: read it via `untracked(() => this.volume())`. Subsequent volume changes flow through `controller.applyVolume()`, never through the effect graph.
|
||||
- **Spurious `timeUpdate` re-emits and `volume.set` calls.** The session-fan-out effect calls `scheduleControlsHide()`, which reads `isPlaying`, `menus.anyOpen`, `statusLabel`, and `controlsVisible`. Those reads became tracked deps, so opening any popover, pausing, or hovering re-ran the body. No loop in isolation, but a parent that wires `timeUpdate` back into `playback.startTime` would have hit the volume-restart bug class. Fix: wrap the side-effect block in `untracked()` so the effect listens only to session changes.
|
||||
- **2 Hz no-op stalled-tracker re-runs.** Position polling updates `session` around 2 Hz. Tracking the full session would re-run stalled logic for snapshots with unchanged status, so the controller tracks only `sessionStatus` and invokes `EmbeddedMpvStalledTracker.track` inside `untracked()`, avoiding full-session reruns.
|
||||
|
||||
Reference in new issue
Block a user