diff --git a/docs/architecture/vod-multi-source.md b/docs/architecture/vod-multi-source.md index a87f6abec..125af10c7 100644 --- a/docs/architecture/vod-multi-source.md +++ b/docs/architecture/vod-multi-source.md @@ -24,6 +24,10 @@ blob whose search path forces `content_type: 'live'`. Both are additive later without changing the contracts — `VodSourceCandidate.portalType` already carries `'xtream' | 'stalker' | 'm3u'` and discovery sits behind a service interface. +A probe answer is cached per *request*, not per URL: two playlists can share a +stream URL and require different headers, and one of them answering 403 says +nothing about the other. + `ffprobe`/`ffmpeg` are not dependencies of this app and are not bundled, so `provenance: 'probe'` means reachability and latency only. There is deliberately no feature flag for codec probing: it would gate a code path with no binary @@ -57,7 +61,10 @@ 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. + is emitted. Below the HD widths the numbers stop separating cleanly — 720 + wide is NTSC 480p or PAL 576p depending on the height, 640 is 360p — so an + unrecognised shape returns nothing rather than a bucket that would be + published as an `api` fact. 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. @@ -360,6 +367,11 @@ a page that shows a Stop button for a session whose progress it discards keeps the resume point at wherever playback began, so a switch an hour later rewinds the whole session. +The caption itself appears only while a player is actually running — inline or +a matched external session. Discovery marks a source active as the page opens, +so gating on that alone would have the page claim "Playing from …" before Play +was pressed, and again after the player was closed. + Whichever source ends up playing, the "playing" badge follows it: starting the route's own stream (Play, Resume, Restart, or the fallback after a pin does not apply) hands the badge back to the route row, or the picker and caption go on 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 d59f35b15..76c775761 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 @@ -123,6 +123,37 @@ describe('applyApiMetadata', () => { ).toEqual({ value: '1080p', provenance: 'api' }); }); + it('does not call every small stream 480p', () => { + // The old rule labelled ANY width under 900 as 480p, published with + // `api` provenance — so a 640x360 stream stated 480p as a fact. + expect( + applyApiMetadata(candidate(), { width: 640, height: 360 }).quality + ).toEqual({ value: '360p', provenance: 'api' }); + expect( + applyApiMetadata(candidate(), { width: 854, height: 480 }).quality + ).toEqual({ value: '480p', provenance: 'api' }); + }); + + it('reads the height when one width covers two formats', () => { + // 720 wide is NTSC 480p or PAL 576p; only the height separates them, + // and without one there is no honest answer to give. + expect( + applyApiMetadata(candidate(), { width: 720, height: 576 }).quality + ).toEqual({ value: '576p', provenance: 'api' }); + expect( + applyApiMetadata(candidate(), { width: 720, height: 480 }).quality + ).toEqual({ value: '480p', provenance: 'api' }); + expect( + applyApiMetadata(candidate(), { width: 720 }).quality + ).toBeUndefined(); + }); + + it('emits nothing for a width below every known format', () => { + expect( + applyApiMetadata(candidate(), { width: 320, height: 240 }).quality + ).toBeUndefined(); + }); + it('emits no quality for an ambiguous cropped height', () => { // 800 lines alone could be a cropped 1080p or a 1280x800 encode. // Guessing here would publish a wrong value labelled as a fact. 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 cafe2d570..4f503aebc 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 @@ -238,6 +238,34 @@ function isPositiveNumber(value: number | null | undefined): value is number { * height. Anything else returns null, so the row shows no tag and offers a * check instead. Empty beats wrong. */ +/** + * Below the HD widths the numbers stop separating cleanly. + * + * 854 is 480p, but 720 is NTSC 480p or PAL 576p depending on the height, and + * 640 is 360p — which the old "anything under 900 is 480p" rule published as + * an `api` FACT for all of them. Empty beats wrong: an unrecognised shape + * returns nothing and the row simply carries no quality tag. + */ +function smallFormatQuality( + width: number, + height: number | null | undefined +): string | null { + if (width >= 800) { + return '480p'; + } + if (width >= 700) { + // The one width two formats share; only the height tells them apart. + if (!isPositiveNumber(height)) { + return null; + } + return height >= 520 ? '576p' : '480p'; + } + if (width >= 600) { + return '360p'; + } + return null; +} + function qualityFromDimensions( width: number | null | undefined, height: number | null | undefined @@ -258,7 +286,7 @@ function qualityFromDimensions( if (width >= 900) { return '576p'; } - return '480p'; + return smallFormatQuality(width, height); } if (!isPositiveNumber(height)) { diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route-playback.spec.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route-playback.spec.ts index 628ed5208..ab333653b 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route-playback.spec.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route-playback.spec.ts @@ -211,6 +211,47 @@ describe('VodDetailsRouteComponent — playback actions', () => { consoleWarnSpy?.mockRestore(); }); + it('claims to be playing only while something is', () => { + currentPlaylist.set({ id: 'playlist-1' }); + const component = fixture.componentInstance; + const playback = fixture.debugElement.injector.get( + VodDetailsPlaybackService + ); + Object.defineProperty(component.multiSource, 'sources', { + configurable: true, + value: () => [ + { + id: 'playlist-1:xtream:650020', + playlistId: 'playlist-1', + playlistName: 'Portal One', + portalType: 'xtream', + contentId: 650020, + rawTitle: 'Example', + matchConfidence: 'exact', + year: null, + isActive: true, + isPinned: false, + isTried: true, + probe: { status: 'idle' }, + }, + ], + }); + + // Discovery marks a source active as soon as the page opens, so the + // caption would otherwise say "Playing from ..." before Play is + // pressed — and again after the player is closed. + expect(component.activeSourceCaption()).toBeNull(); + + playback.inlinePlayback.set({ + streamUrl: 'http://example.com/movie.mkv', + title: 'Example', + }); + expect(component.activeSourceCaption()).not.toBeNull(); + + playback.inlinePlayback.set(null); + expect(component.activeSourceCaption()).toBeNull(); + }); + it('owns an external session for a copy in its own playlist', () => { currentPlaylist.set({ id: 'playlist-1' }); const component = fixture.componentInstance; diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route.component.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route.component.ts index 1e53a2f13..0939e27fc 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route.component.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route.component.ts @@ -549,7 +549,13 @@ export class VodDetailsRouteComponent implements OnInit, OnDestroy { /** The ".srcbar" caption under the action row: where this is playing from. */ readonly activeSourceCaption = computed(() => { const active = this.multiSource.sources().find((s) => s.isActive); - if (!active) { + // "Playing from" only while something actually is. Discovery marks a + // source active as soon as the page opens, so gating on that alone + // makes the line a claim about a player that has not started — or one + // the user has since closed. + const playing = + !!this.inlinePlayback() || !!this.matchedExternalPlayback(); + if (!active || !playing) { return null; } diff --git a/libs/services/src/lib/stream-probe.service.spec.ts b/libs/services/src/lib/stream-probe.service.spec.ts index c556dd2c7..0fa4810e8 100644 --- a/libs/services/src/lib/stream-probe.service.spec.ts +++ b/libs/services/src/lib/stream-probe.service.spec.ts @@ -52,6 +52,28 @@ describe('StreamProbeService', () => { ); }); + it('does not answer one playlist with another’s probe result', async () => { + const probe = jest + .fn() + .mockResolvedValueOnce({ status: 200, url: 'http://x/y.mkv' }) + .mockResolvedValueOnce({ status: 403, url: 'http://x/y.mkv' }); + const service = withBridge(probe as ProbeFn); + + const first = await service.probe('http://x/y.mkv', 'HEAD', { + userAgent: 'PlayerOne/1.0', + }); + const second = await service.probe('http://x/y.mkv', 'HEAD', { + userAgent: 'PlayerTwo/2.0', + }); + + // Two playlists can share a stream URL and require different headers. + // Reusing the first answer would state the second source is dead + // without ever having asked it. + expect(probe).toHaveBeenCalledTimes(2); + expect(first.status).toBe('ok'); + expect(second.status).toBe('fail'); + }); + it('carries the playlist headers into both attempts', async () => { const headers = { userAgent: 'MyPlayer/2.0', referer: 'http://x/' }; const probe = jest diff --git a/libs/services/src/lib/stream-probe.service.ts b/libs/services/src/lib/stream-probe.service.ts index 8c9b16ab3..af4980a4a 100644 --- a/libs/services/src/lib/stream-probe.service.ts +++ b/libs/services/src/lib/stream-probe.service.ts @@ -37,9 +37,35 @@ export class StreamProbeService { ); } - /** A cached result for this URL, if one is still fresh. */ - peek(url: string): VodSourceProbeResult | null { - const entry = this.cache.get(url); + /** + * The key an answer is stored under. + * + * Not the URL alone: the same stream URL can be shared by two playlists + * that require different headers, and one of them answering 403 says + * nothing about the other. Reusing that result would state a source is + * dead without ever having asked it. + */ + private cacheKey( + url: string, + method: 'GET' | 'HEAD', + headers?: StreamProbeHeaders + ): string { + return [ + url, + method, + headers?.userAgent ?? '', + headers?.referer ?? '', + headers?.origin ?? '', + ].join('\u0000'); + } + + /** A cached result for this exact request, if one is still fresh. */ + peek( + url: string, + method: 'GET' | 'HEAD' = 'HEAD', + headers?: StreamProbeHeaders + ): VodSourceProbeResult | null { + const entry = this.cache.get(this.cacheKey(url, method, headers)); if (!entry || Date.now() - entry.storedAt > CACHE_TTL_MS) { return null; } @@ -61,20 +87,21 @@ export class StreamProbeService { return { status: 'unknown' }; } - const cached = this.peek(url); + const key = this.cacheKey(url, method, headers); + const cached = this.peek(url, method, headers); if (cached) { return cached; } - const pending = this.inFlight.get(url); + const pending = this.inFlight.get(key); if (pending) { return pending; } const request = this.runProbe(url, method, headers).finally(() => { - this.inFlight.delete(url); + this.inFlight.delete(key); }); - this.inFlight.set(url, request); + this.inFlight.set(key, request); return request; } @@ -126,7 +153,10 @@ export class StreamProbeService { // caching it would keep a perfectly good source looking unverified // for the whole TTL. if (result.status !== 'unknown') { - this.cache.set(url, { result, storedAt: Date.now() }); + this.cache.set(this.cacheKey(url, method, headers), { + result, + storedAt: Date.now(), + }); } return result;