mirror of
https://github.com/4gray/iptvnator.git
synced 2026-10-10 18:36:15 -08:00
fix(portals): stop claiming playback, a cached answer and a resolution
Three findings, all of them the same rule: never state as fact something the app has not established. The "Playing from …" caption appeared as soon as discovery marked a source active — before Play was pressed, and again after the player was closed. It now requires a player that is actually running. Probe answers were cached by URL alone, but the request now carries the playlist's headers. Two playlists sharing a stream URL could therefore be told the other's answer, marking a source dead without ever asking it. And any width below 900 was labelled 480p, published with `api` provenance: a 640x360 stream stated 480p as a fact, and a 720x576 PAL source likewise. Widths below HD only resolve with the height — 720 is NTSC 480p or PAL 576p — so an unrecognised shape now carries no quality tag at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
11e4e89d66
commit
7045f2d665
7 files changed
+181
-11
No files matched your search
@@ -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
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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)) {
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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;
|
||||
|
||||
Reference in new issue
Block a user