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 {