From bd41d3698b2e91c61eb1ac7e569eea1630121b73 Mon Sep 17 00:00:00 2001 From: 4gray Date: Wed, 29 Jul 2026 20:35:21 +0200 Subject: [PATCH] fix(portals): let the height veto a width-derived quality, and refresh route facts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both of these are gaps I saw and chose not to close last round; a reviewer was right that neither survives its own reasoning. The shape check only ran below 1200, so the HD ranges kept publishing wrong-but-confident labels: 1440x1080 is anamorphic 1080 and 1600x900 is 900p, and both were "720p" with `api` provenance — the provenance that means the provider said so. Ranges are fine up there, the standard widths really are far apart, but only once a known height can veto the answer. Same rule the matched formats already used: a shorter frame is a letterboxed master, a taller one is a different shape and gets no tag. And the route row picked up provider facts only when discovery reran. On a sparse panel `get_vod_info` can answer with no year and no TMDB id, so the movie key is unchanged, nothing reruns, and the row keeps stating nothing — leaving `audioDiffersFactually` one-sided and the dub warning unreachable on exactly the switch it exists for. It now takes those facts on without rediscovering, merged onto the existing row so a probe result already sitting there survives. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 2 +- docs/architecture/vod-multi-source.md | 22 ++++++---- .../vod-source-metadata.util.spec.ts | 25 +++++++++++ .../multi-source/vod-source-metadata.util.ts | 39 ++++++++++++----- .../vod-multi-source-host-session.spec.ts | 41 ++++++++++++++++++ .../vod-multi-source-host.service.ts | 43 +++++++++++++++++++ 6 files changed, 152 insertions(+), 20 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index fbc6694ba..27fd7831d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -896,7 +896,7 @@ engine` (restart required) or - Finds the same movie in the user's other imported playlists and adds a "Sources N" chip to the Xtream VOD action row (only when ≥1 alternative exists), plus a `.source-caption` line reporting where playback is coming from. The chip opens a 460px anchored CDK-overlay popover (`libs/ui/components/src/lib/vod-sources/`; not `MatMenu`, which caps its width at 280px), reused unchanged in the inline player's now-playing bar and on the playback-error screen. Both chips are handed the same `matchKind` and `vodAutoFailover` and both write the setting back. The chip counts alternative **streams**; the caption ("also found in N other playlists") counts distinct **playlists** via `alternativePlaylistCount`, because the popover groups one portal's copies under that portal. - Scope v1 is **Xtream ↔ Xtream, movies only, Electron only**. Stalker never reaches the `content` table and M3U is a JSON blob whose search forces `content_type:'live'`; both are additive later since `VodSourceCandidate.portalType` already carries all three. In the PWA every entry point is gated off by a bridge `typeof` check and the chip renders nothing. -- **Metadata provenance is the core contract.** Every field is `{value, provenance}` where `api`/`probe` are facts (plain tag), `parsed` is a title-regex guess (tag prefixed `~`, warn colour), and absent renders **no tag at all** plus a `check` chip. `factualOnly()` in `vod-source-metadata.util.ts` is the only accessor allowed for ranking/failover, so guesses are structurally unable to influence a decision. `VodSourceProbeStatus` separates `fail` (contacted and refused) from `unknown` (timed out / blocked / no capability) — an unchecked source is never shown as offline. Quality is derived from pixel **width** because letterboxing crops height. +- **Metadata provenance is the core contract.** Every field is `{value, provenance}` where `api`/`probe` are facts (plain tag), `parsed` is a title-regex guess (tag prefixed `~`, warn colour), and absent renders **no tag at all** plus a `check` chip. `factualOnly()` in `vod-source-metadata.util.ts` is the only accessor allowed for ranking/failover, so guesses are structurally unable to influence a decision. `VodSourceProbeStatus` separates `fail` (contacted and refused) from `unknown` (timed out / blocked / no capability) — an unchecked source is never shown as offline. Quality is derived from pixel **width** because letterboxing crops height — but a known height vetoes the answer on every tier, since cropping only removes lines: a taller frame is a different shape (1440×1080 anamorphic or 1600×900 are not 720p, 960×540 is not 576p) and gets no tag rather than a wrong one carrying `api` provenance. The route's OWN row is never resolved, so it takes its facts from the `get_vod_info` the page already loaded (`providerVodMetadataOf`, shared with the resolver) and picks them up via `refreshRouteFacts()` even when they arrive without changing the movie identity — otherwise `audioDiffersFactually` has nothing on one side and the dub warning cannot fire on a route-to-alternative switch. - Discovery (`DB_FIND_TITLE_SOURCES`, trigram FTS over `content_title_fts`) is lazy and returns only what the `content` table can prove; titles whose tokens are all shorter than three characters ("Up", "It") fall back to a scan, since the trigram tokenizer cannot index them at all. A source that is never read looks exactly like one that does not exist, so: the current playlist is excluded **in SQL** and duplicates collapse there too (`GROUP BY cat.playlist_id, c.xtream_id` before the limit — one playlist's dozens of identically ranked category rows would otherwise crowd out every alternative), and the scan matches an ASCII token as a whole word (`' ' || LOWER(title) || ' ' GLOB '*[^a-z0-9]it[^a-z0-9]*'`) ordered by title length **with no row limit** — FTS keeps its 60-row window because it ranks by relevance, while a scan cannot rank, and the GLOB reads every row regardless so a limit would only truncate the answer. The year gate covers BOTH match tiers: `normalizeTitleKeys` strips bracketed segments, so "Dune (1984)" normalizes identically to "Dune" and would otherwise be an *exact* match for the 2021 film; a bracketed year is read out of the raw title and a stated disagreement rejects the row — but the two tiers read different forms: the base tier accepts bracketed or trailing (it just stripped a trailing year, the only thing separating "Dune 1984" from "Dune 2021"), while the exact tier reads bracketed ONLY, since reaching it means both titles are the same string and a trailing number is then part of the NAME ("Blade Runner 2049" against a metadata year of 2017 would otherwise vanish once enrichment lands). A non-ASCII token cannot be folded by `LOWER()` (ASCII-only) but CAN be by a GLOB character class (UTF-8 code points), so `caseInsensitiveGlobPattern` folds the case in JS and emits one `[lowerUpper]` class per character — returning `null`, leaving the two substring tests alone, for a GLOB metacharacter or a length-changing case map (`ß`→`SS`). The movie's own year comes from `releaseTagYear` (bracketed or trailing only), never `extractYear`: a year inside the NAME ("2001: A Space Odyssey") would fail every genuine 1968 copy at the year gate and move the pin key once enrichment lands. One row inside the excluded playlist is kept when the caller names it (`keepContentId`), because a pin can point at another copy in the playlist being viewed — the host reads the pin before discovery for exactly this. Resolution is deferred to click/pin/check because `content` stores no `container_extension` and `constructVodUrl` returns `''` without one — each alternative costs a live `get_vod_info` against the foreign playlist's credentials. - Switching = one `inlinePlayback.set({...next, startTime})`, never null-then-set, so the player and engine survive and re-seek. The carried position is read *before* the 15s persistence throttle, and `VodDetailsPlaybackService` uses a one-shot `resumeSettled` latch so a resuming engine's `timeupdate` at ~0 cannot overwrite the resume point. `handleInlineTimeUpdate` returns that verdict and the route feeds multi-source the requested `startTime` until the engine reaches it — one latch for both, or a switch during the initial seek would restart the film. Before anything plays there is no live position at all, so the controller is seeded from the persisted one (`seedResumeSeconds`, one-way: a live value always wins). Portal failures in the multi-source path log through the redacting `createLogger`/`redactSensitiveData` — an Xtream error message carries the stream URL, and that URL is built out of the username and password. - Pins are keyed portal-agnostically (`tmdb:{id}` else `title:{base}:{year}` else the yearless `title:{base}:`, `vod_source_pins` table); enrichment supplies the id and the year late, so a pin may sit under any poorer form — three key sets (`pinKeysFor`): `lookup` passes every alias most-trusted-first, `write` holds only keys naming exactly one film, and `loaded` records where the pin on screen was found — the yearless alias is readable but never written or deleted on spec, since it is shared by every remake, with the single exception of the row this session actually read. A write stores the decision under **every** key in `write` (`setVodSourcePin(db, pin, retireKeys, aliasKeys)`: one upsert per key plus the leftover retirement, in a single transaction), because a movie's identity grows — recorded only under the enriched `tmdb:` key, a pin is invisible to the next reopen, which starts out with just a title and a year, and stays invisible for good if enrichment is off or never answers. A pin is not decoration: the primary Play action starts from the pinned source (except when that button reads Stop — an active external session wins, or the control would launch a second player), and it outranks everything else in failover ranking. The row changes only after the write lands, so a refused pin is never shown as saved. Starting a pinned source loads THAT source's own playback position — progress is keyed by (playlist, stream), so the row the page loaded belongs to the route's copy. The primary button says nothing at all until that row is in, and "is it in" is answered by comparing the loaded pin **id** rather than mere presence, or re-pinning would leave the button wearing the previous copy's timecode. An external player launched for an alternative carries the OTHER playlist's ids, so `VodDetailsPlaybackBindings.activeSource` feeds one `ownsContent()` predicate used by BOTH the session matcher and the playback-position bridge — if they disagree, the page shows Stop for a session whose progress it throws away and a later switch rewinds hours. Two identity keys: `vodMultiSourceMovieKey` (title, year, tmdbId) makes TMDB enrichment re-trigger discovery and rebuild the pin keys, while `vodMultiSourceSessionKey` (`playlistId:contentId`) decides whether that rerun is a refresh or a new session — a refresh keeps the active source, its resolved facts, the tried set, the live position and any switch in flight; only a different film resets them. diff --git a/docs/architecture/vod-multi-source.md b/docs/architecture/vod-multi-source.md index 517b92411..a50b60718 100644 --- a/docs/architecture/vod-multi-source.md +++ b/docs/architecture/vod-multi-source.md @@ -61,14 +61,20 @@ Three rules follow, and each is enforced in code rather than by convention: letterboxed masters are cropped vertically — a 2.39:1 1080p film is 1920×800, and 800 alone is indistinguishable from a 1280×800 encode. With no width, a height is trusted only within 5% of a standard frame height; otherwise no tag - is emitted. Below the HD widths the ranges stop working: 800×600 - and 800×450 are neither 480p nor each other, and 720 wide is NTSC 480p or - PAL 576p depending only on the height. Those formats are therefore matched - rather than bucketed, and an unrecognised shape returns nothing rather than - a label that would be published as an `api` fact. A known height still has - to be consistent with the match: cropping only ever *removes* lines, so a - shorter frame is a letterboxed master of that format, while a taller one - (640×480 against 640×360) is a different shape and gets no tag. + is emitted. Below the HD widths the ranges stop working: 960×540 and 1024×576 + are two formats inside one 900–1199 range, 800×600 and 800×450 are neither + 480p nor each other, and 720 wide is NTSC 480p or PAL 576p depending only on + the height. Those formats are therefore matched rather than bucketed, and an + unrecognised shape returns nothing rather than a label that would be + published as an `api` fact. + + A known height has to be consistent with the width **on every tier**, + ranges included. Cropping only ever *removes* lines, so a shorter frame is a + letterboxed master of that format, while a taller one is a different shape: + 640×480 against 640×360 below, and 1440×1080 (anamorphic 1080) or 1600×900 + against the 720p range above. All of those get no tag. Bucketing HD widths is + otherwise sound — the standard widths really are far apart — but only once + the height is allowed to veto the answer. Provenance is per-field and changes over time: at discovery a row has only `parsed` tags, because the `content` table stores no container, codec or audio. diff --git a/libs/portal/shared/data-access/src/lib/multi-source/vod-source-metadata.util.spec.ts b/libs/portal/shared/data-access/src/lib/multi-source/vod-source-metadata.util.spec.ts index 5ada5ee13..e1ee80f1c 100644 --- a/libs/portal/shared/data-access/src/lib/multi-source/vod-source-metadata.util.spec.ts +++ b/libs/portal/shared/data-access/src/lib/multi-source/vod-source-metadata.util.spec.ts @@ -164,6 +164,31 @@ describe('applyApiMetadata', () => { ).toEqual({ value: '576p', provenance: 'api' }); }); + it('refuses a wide frame taller than the format its width names', () => { + // 1440x1080 is anamorphic 1080 and 1600x900 is 900p. Both fall in the + // 1200-1699 band, so both were published as "720p" — with `api` + // provenance, which is the one that means the provider said so. + expect( + applyApiMetadata(candidate(), { width: 1440, height: 1080 }).quality + ).toBeUndefined(); + expect( + applyApiMetadata(candidate(), { width: 1600, height: 900 }).quality + ).toBeUndefined(); + }); + + it('still names a letterboxed wide master by its width', () => { + // Cropping only removes lines, so a short frame is this format. + expect( + applyApiMetadata(candidate(), { width: 1920, height: 800 }).quality + ).toEqual({ value: '1080p', provenance: 'api' }); + expect( + applyApiMetadata(candidate(), { width: 1920, height: 1080 }).quality + ).toEqual({ value: '1080p', provenance: 'api' }); + expect( + applyApiMetadata(candidate(), { width: 3840, height: 2160 }).quality + ).toEqual({ value: '2160p', provenance: 'api' }); + }); + it('gives no quality for a width between the known formats', () => { // 1100 wide is no standard shape. The old range answered "576p" for // it; an absent tag and a check chip is the honest reply. diff --git a/libs/portal/shared/data-access/src/lib/multi-source/vod-source-metadata.util.ts b/libs/portal/shared/data-access/src/lib/multi-source/vod-source-metadata.util.ts index 667aef313..b85fc5ce5 100644 --- a/libs/portal/shared/data-access/src/lib/multi-source/vod-source-metadata.util.ts +++ b/libs/portal/shared/data-access/src/lib/multi-source/vod-source-metadata.util.ts @@ -251,6 +251,20 @@ function isPositiveNumber(value: number | null | undefined): value is number { * Anything that is not one of these shapes gets no tag at all, because a guess * here would be read as a measurement. */ +/** + * HD and up, as `[minimum width, frame height, label]`. + * + * Ranges are honest here — the standard widths really are far apart — but the + * height still has to agree, for the same reason it does below: a frame TALLER + * than the format is a different shape, not a crop of it. + */ +const WIDE_FORMATS: ReadonlyArray<[number, number, string]> = [ + [3400, 2160, '2160p'], + [2400, 1440, '1440p'], + [1700, 1080, '1080p'], + [1200, 720, '720p'], +]; + const KNOWN_FORMATS: ReadonlyArray<[number, number, string]> = [ [1024, 576, '576p'], [960, 540, '540p'], @@ -303,18 +317,21 @@ function qualityFromDimensions( height: number | null | undefined ): string | null { if (isPositiveNumber(width)) { - if (width >= 3400) { - return '2160p'; - } - if (width >= 2400) { - return '1440p'; - } - if (width >= 1700) { - return '1080p'; - } - if (width >= 1200) { - return '720p'; + const wide = WIDE_FORMATS.find(([minWidth]) => width >= minWidth); + if (wide) { + const [, formatHeight, label] = wide; + // The same rule the matched formats use. Cropping only ever + // REMOVES lines, so a shorter frame is a letterboxed master of this + // format and the width still names it — but a TALLER one is a + // different shape: 1440x1080 is anamorphic 1080 and 1600x900 is + // 900p, and calling either "720p" states a measurement its own + // pixels contradict, under the provenance that means the provider + // said so. + return isPositiveNumber(height) && height > formatHeight * 1.05 + ? null + : label; } + return knownFormatQuality(width, height); } diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host-session.spec.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host-session.spec.ts index f867a294e..7f989f718 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host-session.spec.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host-session.spec.ts @@ -232,4 +232,45 @@ describe('VodMultiSourceHostService — session lifecycle', () => { ALT_THREE.id, ]); }); + + it('takes on provider facts that arrive without changing the identity', async () => { + movie.set(MOVIE_A); + await flushEffects(); + expect(rowFor(CURRENT_A_ID)?.audio).toBeUndefined(); + + // A sparse panel answers `get_vod_info` with no year and no TMDB id, + // so the movie key is unchanged and nothing reruns discovery. The + // route row would keep stating nothing, and every comparison against + // it stays one-sided — the dub warning could never fire. + movie.set({ + ...MOVIE_A, + metadata: { audioCodec: 'ac3', containerExtension: 'mkv' }, + }); + await flushEffects(); + + expect(discovery.discover).toHaveBeenCalledTimes(1); + expect(rowFor(CURRENT_A_ID)?.audio).toEqual({ + value: 'ac3', + provenance: 'api', + }); + expect(rowFor(CURRENT_A_ID)?.container).toEqual({ + value: 'mkv', + provenance: 'api', + }); + }); + + it('keeps what the row already knows when facts arrive', async () => { + movie.set(MOVIE_A); + await flushEffects(); + await service.check(CURRENT_A_ID); + expect(rowFor(CURRENT_A_ID)?.probe?.status).toBe('ok'); + + movie.set({ ...MOVIE_A, metadata: { audioCodec: 'ac3' } }); + await flushEffects(); + + // Rebuilding the row from the movie would be the obvious way to do + // this, and it would silently throw away a probe the user asked for. + expect(rowFor(CURRENT_A_ID)?.probe?.status).toBe('ok'); + expect(rowFor(CURRENT_A_ID)?.audio?.value).toBe('ac3'); + }); }); diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host.service.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host.service.ts index cfcda59d1..46de0a8fc 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host.service.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host.service.ts @@ -7,6 +7,7 @@ import { type Signal, } from '@angular/core'; import { + applyApiMetadata, VodMultiSourceController, VodSourceDiscoveryService, VodSourceResolverService, @@ -93,6 +94,8 @@ export class VodMultiSourceHostService { private movieIdentity: string | null = null; /** The row standing for the playlist the route is on. */ private routeSourceId: string | null = null; + /** The provider facts already overlaid on that row, to overlay each once. */ + private routeFactsKey: string | null = null; /** Resolves once the discovery on the way has published its sources. */ private loadInFlight: Promise | null = null; @@ -154,14 +157,54 @@ export class VodMultiSourceHostService { const key = vodMultiSourceMovieKey(movie); if (this.lastMovieKey === key) { + // Same film, described the same way — but `get_vod_info` can + // land without touching title, year or TMDB id while adding + // the provider's facts about the stream. Rediscovery would be + // wasted work; only the route's own row is missing anything. + this.refreshRouteFacts(movie); return; } this.lastMovieKey = key; + this.routeFactsKey = null; void this.load(movie); }); } + /** + * Overlay the provider's facts onto the route's own row, in place. + * + * The row is built when discovery runs, which on a sparse panel happens + * before `get_vod_info` answers — and if that answer adds no year and no + * TMDB id, the movie key does not change, so nothing rebuilds the row and + * it keeps stating nothing. Every comparison against it is then one-sided: + * the dub warning in particular cannot fire at all. + * + * Merged onto the existing row rather than rebuilt from the movie, so a + * probe result already sitting on it survives. + */ + private refreshRouteFacts(movie: VodMultiSourceMovie): void { + const facts = movie.metadata; + const routeSourceId = this.routeSourceId; + if (!facts || !routeSourceId) { + return; + } + + const factsKey = JSON.stringify(facts); + if (this.routeFactsKey === factsKey) { + return; + } + + const existing = this.controller.findSource(routeSourceId); + if (!existing) { + return; + } + + this.routeFactsKey = factsKey; + this.controller.updateSource(applyApiMetadata(existing, facts)); + this.publish(); + } + /** * (Re)discover the sources for a movie. *