fix(portals): let the height veto a width match, and hold the failure state

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 <noreply@anthropic.com>
This commit is contained in:
4grayandClaude Opus 5 committed 2026-07-28 05:51:51 +02:00
1 parent 2915771b0f
commit 7459f993e2
5 files changed
+70 -11

No files matched your search

+4 -1
View File
@@ -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.
@@ -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.
@@ -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(
@@ -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();
});
});
@@ -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 {