From 7459f993e2dcf65afa43c5f5a1046878797d6072 Mon Sep 17 00:00:00 2001 From: 4gray Date: Tue, 28 Jul 2026 05:51:51 +0200 Subject: [PATCH] fix(portals): let the height veto a width match, and hold the failure state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two follow-ups to the previous round, both in code it introduced. A width that matched exactly one sub-HD format ignored the height entirely, so 640x480 came back as 360p — a measurement the numbers contradict. The height now vetoes, but only in the direction that can be wrong: cropping removes lines, so a SHORTER frame is a letterboxed master of that format and the width still names it, while a taller one is a different shape and gets no tag. That keeps the reason width is preferred in the first place. And picking a source off the error screen cleared the failure state before the switch resolved, so an alternative that could not be resolved left the diagnostic on screen while the caption went back to claiming playback. The flag now clears only once a switch actually starts something. Co-Authored-By: Claude Opus 5 --- docs/architecture/vod-multi-source.md | 5 +++- .../vod-source-metadata.util.spec.ts | 16 ++++++++++++ .../multi-source/vod-source-metadata.util.ts | 26 +++++++++++++------ .../vod-details-route-caption.spec.ts | 24 +++++++++++++++++ .../vod-details-route.component.ts | 10 +++++-- 5 files changed, 70 insertions(+), 11 deletions(-) diff --git a/docs/architecture/vod-multi-source.md b/docs/architecture/vod-multi-source.md index 850b0a0d4..46bbde403 100644 --- a/docs/architecture/vod-multi-source.md +++ b/docs/architecture/vod-multi-source.md @@ -65,7 +65,10 @@ Three rules follow, and each is enforced in code rather than by convention: 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 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. 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 0e0bacdcc..094120b9d 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 @@ -148,6 +148,22 @@ describe('applyApiMetadata', () => { ).toBeUndefined(); }); + it('rejects a height the width cannot account for', () => { + // 640x480 is 4:3 VGA, not a letterboxed 360p — the width alone would + // have called it 360p and published that as a measurement. + expect( + applyApiMetadata(candidate(), { width: 640, height: 480 }).quality + ).toBeUndefined(); + }); + + it('still trusts the width when the frame is cropped', () => { + // Cropping only removes lines, so a short frame at 854 wide is a + // letterboxed 480p master and the width still names it. + expect( + applyApiMetadata(candidate(), { width: 854, height: 360 }).quality + ).toEqual({ value: '480p', provenance: 'api' }); + }); + it('emits nothing for a shape that is not a known format', () => { // 800 wide is neither 854x480 nor anything else in the table, and // 800x600 is certainly not 480 lines high. 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 c751e00db..7b26db6d1 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 @@ -268,18 +268,28 @@ function smallFormatQuality( if (byWidth.length === 0) { return null; } - if (byWidth.length === 1) { - return byWidth[0][2]; + + if (byWidth.length > 1) { + // One width, two formats (720): only the height separates them. + if (!isPositiveNumber(height)) { + return null; + } + const exact = byWidth.find(([, reference]) => + within5Percent(height, reference) + ); + return exact ? exact[2] : null; } - // One width, two formats: only the height separates them. - if (!isPositiveNumber(height)) { + // A single candidate still has to survive the height, when one is known. + // 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 + // (640x480 against 640x360) is a different shape entirely, and labelling + // it would state a measurement the numbers contradict. + const [, formatHeight, label] = byWidth[0]; + if (isPositiveNumber(height) && height > formatHeight * 1.05) { return null; } - const exact = byWidth.find(([, reference]) => - within5Percent(height, reference) - ); - return exact ? exact[2] : null; + return label; } function qualityFromDimensions( diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route-caption.spec.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route-caption.spec.ts index 580aea87a..7e88dc048 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route-caption.spec.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-details-route-caption.spec.ts @@ -288,4 +288,28 @@ describe('VodDetailsRouteComponent — source caption', () => { component.handleInlineTimeUpdate({ currentTime: 3, duration: 90 }); expect(component.activeSourceCaption()).not.toBeNull(); }); + + it('keeps the failure state when the chosen source will not resolve', async () => { + currentPlaylist.set({ id: 'playlist-1' }); + const component = fixture.componentInstance; + const playback = fixture.debugElement.injector.get( + VodDetailsPlaybackService + ); + withActiveSource('playlist-1', 650020); + playback.inlinePlayback.set({ + streamUrl: 'http://example.com/movie.mkv', + title: 'Example', + }); + await component.onPlaybackFailed(); + expect(component.activeSourceCaption()).toBeNull(); + + // Picking an alternative off the error screen that cannot be resolved + // leaves the diagnostic up — so the caption must stay away too. + jest.spyOn(component.multiSource, 'play').mockResolvedValue(false); + component.playFromSource('playlist-2:xtream:991'); + await Promise.resolve(); + await Promise.resolve(); + + expect(component.activeSourceCaption()).toBeNull(); + }); }); 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 9efec927a..6185bfa79 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 @@ -588,8 +588,14 @@ export class VodDetailsRouteComponent implements OnInit, OnDestroy { }); playFromSource(sourceId: string): void { - this.playbackFailed.set(false); - void this.multiSource.play(sourceId); + // Only once the switch actually starts something. A source picked off + // the error screen that cannot be resolved leaves the diagnostic up, + // and clearing eagerly would have the caption claim playback again. + void this.multiSource.play(sourceId).then((switched) => { + if (switched) { + this.playbackFailed.set(false); + } + }); } pinSource(sourceId: string): void {