From bf13849d696d3af661b1b42b386b2008ced2bced Mon Sep 17 00:00:00 2001 From: 4gray <4gray@users.noreply.github.com> Date: Thu, 30 Jul 2026 19:55:35 +0200 Subject: [PATCH] fix(playback): avoid false codec diagnostics (#1314) * docs(playback): design accurate native diagnostics * docs(playback): plan accurate native diagnostics * fix(playback): classify native source errors from evidence * fix(playback): preserve Video.js HTTP error context * fix(playback): show explicit HTTP playback errors * docs(playback): document native error evidence --- .changes/playback-accurate-source-errors.md | 7 + docs/architecture/embedded-inline-playback.md | 7 + ...30-accurate-native-playback-diagnostics.md | 743 ++++++++++++++++++ ...rate-native-playback-diagnostics-design.md | 146 ++++ .../playback-diagnostics.model.ts | 8 + .../playback-diagnostics.util.spec.ts | 105 ++- .../playback-diagnostics.util.ts | 46 +- .../vjs-player/vjs-player.component.spec.ts | 35 +- .../src/lib/vjs-player/vjs-player.types.ts | 3 +- .../web-player-view-diagnostics.utils.ts | 16 +- .../web-player-view.component.spec.ts | 46 ++ 11 files changed, 1156 insertions(+), 6 deletions(-) create mode 100644 .changes/playback-accurate-source-errors.md create mode 100644 docs/superpowers/plans/2026-07-30-accurate-native-playback-diagnostics.md create mode 100644 docs/superpowers/specs/2026-07-30-accurate-native-playback-diagnostics-design.md diff --git a/.changes/playback-accurate-source-errors.md b/.changes/playback-accurate-source-errors.md new file mode 100644 index 000000000..5ed9b8a70 --- /dev/null +++ b/.changes/playback-accurate-source-errors.md @@ -0,0 +1,7 @@ +--- +type: fix +area: playback +issues: [1159] +--- + +Unavailable streams no longer appear as unsupported codecs. When Video.js exposes a server error such as HTTP 404, the player shows that status; otherwise ambiguous source errors remain unidentified instead of guessing. diff --git a/docs/architecture/embedded-inline-playback.md b/docs/architecture/embedded-inline-playback.md index 3acd31898..1ad8ef2f8 100644 --- a/docs/architecture/embedded-inline-playback.md +++ b/docs/architecture/embedded-inline-playback.md @@ -328,6 +328,13 @@ Supported diagnostic codes are: - `drm-or-encryption` - `unknown-playback-error` +Native `MediaError` code 4 alone is not codec evidence. A source with a known +browser-incompatible container remains `unsupported-container`; otherwise, a +code 4 error without stronger evidence is `unknown-playback-error`. An explicit +Video.js HTTP error is `network-error` and shows its status. Because an HTTP +status is server/network evidence rather than decoding evidence, external +decoding is not presented as a likely fix. + `network-error` is reserved for provider/network loading failures. Browser security failures such as CORS, mixed content, Content Security Policy, and private-network-access blocks are classified as `browser-access-error` so the UI can explain that the browser player was blocked before playback reached decoding. mpegts.js `Early-EOF` failures on MPEG-TS streams are classified as `media-decode-error` instead of generic `network-error`. These failures usually mean the fetch stream ended before mpegts.js expected a complete transport stream, and external players may still handle the same URL more tolerant of short reads or malformed TS boundaries. diff --git a/docs/superpowers/plans/2026-07-30-accurate-native-playback-diagnostics.md b/docs/superpowers/plans/2026-07-30-accurate-native-playback-diagnostics.md new file mode 100644 index 000000000..24ba01e4e --- /dev/null +++ b/docs/superpowers/plans/2026-07-30-accurate-native-playback-diagnostics.md @@ -0,0 +1,743 @@ +# Accurate Native Playback Diagnostics Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Fix issue #1159 so unavailable HLS sources are never misreported as unsupported codecs, while retaining an explicit HTTP error status exposed by Video.js. + +**Architecture:** Extend the shared native-error input and normalized diagnostic with two safe structured fields: HTTP status and a bounded Video.js error type. Give explicit HTTP evidence precedence, keep independently known unsupported containers, and classify every other native code-4 failure as unknown; reuse the existing network/unknown UI copy and render `HTTP ` through existing diagnostic metadata and details surfaces. + +**Tech Stack:** Angular 21, TypeScript, Video.js 8, Jest through Nx, ngx-translate JSON catalogs, Markdown release notes. + +--- + +### Task 0: Bootstrap the Nx workspace + +**Files:** +- Verify only: `package.json` +- Verify only: `pnpm-lock.yaml` + +- [ ] **Step 1: Install the locked dependencies** + +Run: + +```bash +pnpm install --frozen-lockfile +``` + +Expected: exit 0 without changing `pnpm-lock.yaml`. + +- [ ] **Step 2: Verify Nx project discovery** + +Run: + +```bash +pnpm nx show projects +``` + +Expected: exit 0 and output containing `ui-playback`, `web`, and `web-e2e`. + +### Task 1: Make native classification evidence-based + +**Files:** +- Modify: `libs/ui/playback/src/lib/playback-diagnostics/playback-diagnostics.model.ts` +- Modify: `libs/ui/playback/src/lib/playback-diagnostics/playback-diagnostics.util.ts` +- Test: `libs/ui/playback/src/lib/playback-diagnostics/playback-diagnostics.util.spec.ts` + +- [ ] **Step 1: Write the failing classifier regressions** + +In `playback-diagnostics.util.spec.ts`, replace the MPEG-TS native code-4 +expectation with an unknown diagnosis and add the HTTP/opaque HLS cases: + +```typescript +it('keeps ambiguous MPEG-TS native source failures unknown', () => { + const issue = classifyNativePlaybackIssue( + { code: 4, message: 'source not supported' }, + createPlaybackSourceMetadata({ + url: 'https://example.com/live/stream', + mimeType: 'video/mp2t', + player: 'videojs', + }) + ); + + expect(issue.code).toBe(PlaybackDiagnosticCode.UnknownPlaybackError); + expect(issue.container).toBe('mp2t'); + expect(issue.externalFallbackRecommended).toBe(false); +}); + +it('classifies an explicit Video.js HTTP failure as a network error', () => { + const issue = classifyNativePlaybackIssue( + { + code: 4, + message: 'The media could not be loaded', + status: 404, + metadata: { errorType: 'networkrequestfailed' }, + }, + createPlaybackSourceMetadata({ + url: 'https://example.com/missing/playlist.m3u8', + mimeType: 'application/x-mpegURL', + player: 'videojs', + }) + ); + + expect(issue).toEqual( + expect.objectContaining({ + code: PlaybackDiagnosticCode.NetworkError, + httpStatus: 404, + nativeErrorType: 'networkrequestfailed', + externalFallbackRecommended: false, + }) + ); +}); + +it('keeps native HLS code 4 unknown when no HTTP or codec evidence exists', () => { + const issue = classifyNativePlaybackIssue( + { + code: 4, + message: + 'The media could not be loaded, either because the server or network failed or because the format is not supported.', + }, + createPlaybackSourceMetadata({ + url: 'https://example.com/live/playlist.m3u8', + mimeType: 'application/x-mpegURL', + player: 'videojs', + }) + ); + + expect(issue.code).toBe(PlaybackDiagnosticCode.UnknownPlaybackError); + expect(issue.externalFallbackRecommended).toBe(false); + expect(issue.httpStatus).toBeUndefined(); +}); + +it('does not treat opaque status zero or unsafe metadata as HTTP evidence', () => { + const issue = classifyNativePlaybackIssue( + { + code: 4, + status: 0, + metadata: { errorType: 'request failed: token=secret value' }, + }, + createPlaybackSourceMetadata({ + url: 'https://example.com/live/playlist.m3u8', + mimeType: 'application/x-mpegURL', + player: 'videojs', + }) + ); + + expect(issue.code).toBe(PlaybackDiagnosticCode.UnknownPlaybackError); + expect(issue.httpStatus).toBeUndefined(); + expect(issue.nativeErrorType).toBeUndefined(); +}); +``` + +Keep the existing `.mkv` and `video/x-msvideo` tests unchanged so known +unsupported containers remain covered. + +- [ ] **Step 2: Run the focused classifier spec to verify RED** + +Run: + +```bash +NODE_OPTIONS=--experimental-vm-modules \ +node node_modules/jest/bin/jest.js \ + --config jest.web-esm.workspace.ts \ + --runTestsByPath \ + libs/ui/playback/src/lib/playback-diagnostics/playback-diagnostics.util.spec.ts \ + --runInBand +``` + +Expected: FAIL because code 4 still becomes `unsupported-codec` and the +normalized diagnostic does not retain `httpStatus` or `nativeErrorType`. + +- [ ] **Step 3: Extend the diagnostic input and output contracts** + +In `playback-diagnostics.model.ts`, replace `NativePlaybackErrorInput` with: + +```typescript +export interface NativePlaybackErrorMetadataInput { + readonly errorType?: unknown; +} + +export interface NativePlaybackErrorInput { + readonly code?: number; + readonly message?: string; + readonly status?: number; + readonly metadata?: NativePlaybackErrorMetadataInput; +} +``` + +Add these optional properties beside the existing native error properties in +`PlaybackDiagnostic`: + +```typescript +readonly httpStatus?: number; +readonly nativeErrorType?: string; +``` + +- [ ] **Step 4: Implement safe evidence extraction and classification** + +In `playback-diagnostics.util.ts`, add: + +```typescript +const MIN_HTTP_ERROR_STATUS = 400; +const MAX_HTTP_ERROR_STATUS = 599; +const NATIVE_ERROR_TYPE_PATTERN = /^[A-Za-z0-9._:-]{1,128}$/; + +function getHttpErrorStatus( + error: NativePlaybackErrorInput | MediaError | null | undefined +): number | undefined { + if (!error || !('status' in error)) { + return undefined; + } + + const status = error.status; + return typeof status === 'number' && + Number.isInteger(status) && + status >= MIN_HTTP_ERROR_STATUS && + status <= MAX_HTTP_ERROR_STATUS + ? status + : undefined; +} + +function getNativeErrorType( + error: NativePlaybackErrorInput | MediaError | null | undefined +): string | undefined { + if (!error || !('metadata' in error)) { + return undefined; + } + + const errorType = error.metadata?.errorType; + return typeof errorType === 'string' && + NATIVE_ERROR_TYPE_PATTERN.test(errorType) + ? errorType + : undefined; +} +``` + +At the start of `classifyNativePlaybackIssue`, extract the two values: + +```typescript +const httpStatus = getHttpErrorStatus(error); +const nativeErrorType = getNativeErrorType(error); +``` + +Before the existing code-2 branch, add: + +```typescript +if (httpStatus !== undefined) { + return createPlaybackDiagnostic({ + code: DiagnosticCode.NetworkError, + source: DiagnosticSource.Native, + metadata, + httpStatus, + nativeErrorCode, + nativeErrorMessage, + nativeErrorType, + }); +} +``` + +Pass `nativeErrorType` through every remaining native diagnostic creation. +Change the code-4 branch to: + +```typescript +if (nativeErrorCode === SOURCE_NOT_SUPPORTED_CODE) { + return createPlaybackDiagnostic({ + code: isLikelyContainerIssue(metadata) + ? DiagnosticCode.UnsupportedContainer + : DiagnosticCode.UnknownPlaybackError, + source: DiagnosticSource.Native, + metadata, + nativeErrorCode, + nativeErrorMessage, + nativeErrorType, + }); +} +``` + +Extend `createPlaybackDiagnostic` options and return value with: + +```typescript +readonly httpStatus?: number; +readonly nativeErrorType?: string; +``` + +and copy both values into the resulting `PlaybackDiagnostic`. + +- [ ] **Step 5: Run the classifier spec to verify GREEN** + +Run the focused command from step 2 again. + +Expected: PASS, including the HLS 404, ambiguous HLS, status-zero, MPEG-TS, +and known-container cases. + +- [ ] **Step 6: Commit the classifier behavior** + +```bash +git add \ + libs/ui/playback/src/lib/playback-diagnostics/playback-diagnostics.model.ts \ + libs/ui/playback/src/lib/playback-diagnostics/playback-diagnostics.util.ts \ + libs/ui/playback/src/lib/playback-diagnostics/playback-diagnostics.util.spec.ts +git commit -m "fix(playback): classify native source errors from evidence" +``` + +Expected: one commit containing the normalized contract, classification rules, +and their regression coverage. + +### Task 2: Verify and align the Video.js HTTP boundary + +**Files:** +- Modify: `libs/ui/playback/src/lib/vjs-player/vjs-player.types.ts` +- Test: `libs/ui/playback/src/lib/vjs-player/vjs-player.component.spec.ts` + +- [ ] **Step 1: Add the Video.js boundary regression** + +Import `NativePlaybackErrorInput` in `vjs-player.component.spec.ts`, change the +harness `currentError` type to `NativePlaybackErrorInput | null`, and add: + +```typescript +it('preserves safe Video.js HTTP context in playback diagnostics', () => { + const issues: Array = []; + component.playbackIssue.subscribe((issue) => issues.push(issue)); + render({ + sources: [ + { + src: 'https://example.test/missing/playlist.m3u8', + type: 'application/x-mpegURL', + }, + ], + }); + harness.currentError = { + code: 4, + message: 'The media could not be loaded', + status: 404, + metadata: { errorType: 'networkrequestfailed' }, + }; + + harness.emit('error'); + + expect(issues.at(-1)).toEqual( + expect.objectContaining({ + code: 'network-error', + source: 'native', + sourceUrl: 'https://example.test/missing/playlist.m3u8', + httpStatus: 404, + nativeErrorType: 'networkrequestfailed', + externalFallbackRecommended: false, + }) + ); +}); +``` + +Keep the existing MKV unsupported-container test. + +- [ ] **Step 2: Run the focused component integration spec** + +Run: + +```bash +NODE_OPTIONS=--experimental-vm-modules \ +node node_modules/jest/bin/jest.js \ + --config jest.web-esm.workspace.ts \ + --runTestsByPath \ + libs/ui/playback/src/lib/vjs-player/vjs-player.component.spec.ts \ + --runInBand +``` + +Expected: PASS after Task 1. JavaScript already passes the complete error +object at runtime; this regression proves that the component does not rebuild +or strip it before classification. + +- [ ] **Step 3: Reuse the shared native-error contract** + +At the top of `vjs-player.types.ts`, add: + +```typescript +import type { NativePlaybackErrorInput } from '../playback-diagnostics/playback-diagnostics.model'; +``` + +Replace the `error` signature with: + +```typescript +error: () => NativePlaybackErrorInput | null; +``` + +No component implementation change is required because +`VjsPlayerComponent.handleVideoJsError` already passes the complete +`player.error()` object to `classifyNativePlaybackIssue`. + +- [ ] **Step 4: Verify the aligned Video.js type contract** + +Run: + +```bash +NODE_OPTIONS=--experimental-vm-modules \ +node node_modules/jest/bin/jest.js \ + --config jest.web-esm.workspace.ts \ + --runTestsByPath \ + libs/ui/playback/src/lib/vjs-player/vjs-player.component.spec.ts \ + --runInBand +pnpm exec tsc -p libs/ui/playback/tsconfig.lib.json --noEmit +``` + +Expected: both commands pass for the HTTP integration case, existing +source/reset behavior, and the production library type graph. The behavioral +RED/GREEN proof lives in Task 1; this task aligns the TypeScript declaration +with the already-tested runtime payload. + +- [ ] **Step 5: Commit the Video.js boundary** + +```bash +git add \ + libs/ui/playback/src/lib/vjs-player/vjs-player.types.ts \ + libs/ui/playback/src/lib/vjs-player/vjs-player.component.spec.ts +git commit -m "fix(playback): preserve Video.js HTTP error context" +``` + +Expected: one commit containing only the Video.js type boundary and component +regression. + +### Task 3: Render explicit HTTP evidence + +**Files:** +- Modify: `libs/ui/playback/src/lib/web-player-view/web-player-view-diagnostics.utils.ts` +- Test: `libs/ui/playback/src/lib/web-player-view/web-player-view.component.spec.ts` + +- [ ] **Step 1: Write the failing rendered-diagnostic regression** + +Add this helper in `web-player-view.component.spec.ts`: + +```typescript +function createHttpDiagnostic(): PlaybackDiagnostic { + return { + code: PlaybackDiagnosticCode.NetworkError, + source: PlaybackDiagnosticSource.Native, + sourceUrl: 'https://example.com/missing/playlist.m3u8', + container: 'm3u8', + mimeType: 'application/x-mpegURL', + player: 'videojs', + audioCodecs: [], + videoCodecs: [], + httpStatus: 404, + nativeErrorCode: 4, + nativeErrorMessage: 'The media could not be loaded', + nativeErrorType: 'networkrequestfailed', + externalFallbackRecommended: false, + }; +} +``` + +Add this component test: + +```typescript +it('renders explicit HTTP evidence without recommending an external player', () => { + runtimeCapabilities.supportsManagedExternalPlayers = true; + fixture.detectChanges(); + const issue = createHttpDiagnostic(); + + component.handlePlaybackIssue(issue); + fixture.detectChanges(); + + const banner = fixture.debugElement.query( + By.css('[data-test-id="playback-diagnostic-banner"]') + ); + const mpvButton = fixture.debugElement.query( + By.css('[data-test-id="playback-fallback-mpv"]') + ); + + expect(banner.nativeElement.textContent).toContain('HTTP 404'); + expect(mpvButton).toBeNull(); + expect(component.getDiagnosticMeta(issue)).toBe('HTTP 404'); + expect(component.getDiagnosticDetails(issue)).toEqual( + expect.arrayContaining([ + { + labelKey: 'PLAYBACK_DIAGNOSTICS.DETAIL_ERROR_DETAILS', + value: 'HTTP 404 · networkrequestfailed', + }, + ]) + ); +}); +``` + +- [ ] **Step 2: Run the focused view spec to verify RED** + +Run: + +```bash +NODE_OPTIONS=--experimental-vm-modules \ +node node_modules/jest/bin/jest.js \ + --config jest.web-esm.workspace.ts \ + --runTestsByPath \ + libs/ui/playback/src/lib/web-player-view/web-player-view.component.spec.ts \ + --runInBand +``` + +Expected: FAIL because the metadata still renders `m3u8` and the details helper +does not include the structured HTTP context. + +- [ ] **Step 3: Format HTTP metadata and safe error details** + +At the start of `getDiagnosticMeta`, add: + +```typescript +if (issue.httpStatus !== undefined) { + return `HTTP ${issue.httpStatus}`; +} +``` + +Replace the final error-details item in `getDiagnosticDetails` with: + +```typescript +{ + labelKey: 'PLAYBACK_DIAGNOSTICS.DETAIL_ERROR_DETAILS', + value: formatDiagnosticErrorDetails(issue), +}, +``` + +Add: + +```typescript +function formatDiagnosticErrorDetails(issue: PlaybackDiagnostic): string { + return [ + issue.httpStatus === undefined ? '' : `HTTP ${issue.httpStatus}`, + issue.nativeErrorType ?? '', + issue.details ?? '', + ] + .filter((value) => value.length > 0) + .join(' · '); +} +``` + +Keep the existing translation keys and template unchanged. + +- [ ] **Step 4: Run the view spec to verify GREEN** + +Run the focused command from step 2 again. + +Expected: PASS with visible `HTTP 404`, no MPV action, and the combined safe +details row. + +- [ ] **Step 5: Run all affected unit tests and lint** + +Run: + +```bash +pnpm nx test ui-playback +pnpm nx lint ui-playback +``` + +Expected: both commands exit 0 with no failed tests or lint errors. + +- [ ] **Step 6: Commit the rendered evidence** + +```bash +git add \ + libs/ui/playback/src/lib/web-player-view/web-player-view-diagnostics.utils.ts \ + libs/ui/playback/src/lib/web-player-view/web-player-view.component.spec.ts +git commit -m "fix(playback): show explicit HTTP playback errors" +``` + +Expected: one commit containing the diagnostic presentation and component +coverage. + +### Task 4: Document the behavior and add the release note + +**Files:** +- Modify: `docs/architecture/embedded-inline-playback.md` +- Create: `.changes/playback-accurate-source-errors.md` + +- [ ] **Step 1: Update the canonical playback documentation** + +Add this paragraph after the supported diagnostic-code list in +`docs/architecture/embedded-inline-playback.md`: + +```markdown +Native `MediaError` code 4 is not codec evidence by itself. A source already +known to use a browser-incompatible container remains +`unsupported-container`; otherwise the native failure stays +`unknown-playback-error`. When Video.js exposes an explicit HTTP error status, +the failure is classified as `network-error`, the status is shown in the +diagnostic, and an external decoder is not presented as a likely fix for the +same failed request. +``` + +- [ ] **Step 2: Add the user-facing release note** + +Create `.changes/playback-accurate-source-errors.md` with: + +```markdown +--- +type: fix +area: playback +issues: [1159] +--- + +Unavailable streams no longer appear as unsupported codecs. When Video.js +exposes a server error such as HTTP 404, the player shows that status; otherwise +ambiguous source errors remain unidentified instead of guessing. +``` + +- [ ] **Step 3: Validate documentation and release metadata** + +Run: + +```bash +git diff --check +pnpm run release:notes:validate +pnpm run i18n:check +``` + +Expected: all commands exit 0. The release note validates, and no translation +catalog drift is introduced because the implementation reuses existing keys. + +- [ ] **Step 4: Commit documentation and release note** + +```bash +git add \ + docs/architecture/embedded-inline-playback.md \ + .changes/playback-accurate-source-errors.md +git commit -m "docs(playback): document native error evidence" +``` + +Expected: one commit containing the canonical behavior contract and the +issue-linked release note. + +### Task 5: Complete verification and local Codex review + +**Files:** +- Verify: all files changed from `origin/master` + +- [ ] **Step 1: Run the final affected validation ladder** + +Run: + +```bash +pnpm nx test ui-playback +pnpm nx lint ui-playback +pnpm run typecheck:web +pnpm run i18n:check +pnpm run release:notes:validate +git diff --check origin/master...HEAD +git status --short +``` + +Expected: every command exits 0, all `ui-playback` tests pass, and the worktree +is clean. E2E is not required because no player selection, route, interaction, +or overlay layout changes; classifier, boundary, and rendered output are +covered by focused component tests. + +- [ ] **Step 2: Dispatch a fresh local Codex reviewer** + +Resolve the exact review range: + +```bash +git rev-parse origin/master +git rev-parse HEAD +``` + +Dispatch a fresh reviewer agent with no implementation-history context and +this request: + +```text +Review origin/master...HEAD for issue #1159 against +docs/superpowers/specs/2026-07-30-accurate-native-playback-diagnostics-design.md. +Focus on correctness, regressions, unsafe disclosure, TypeScript/runtime +contract mismatches, and missing tests. Report only actionable P0/P1/P2 +findings with exact file and line references; do not report style-only nits. +Do not modify files. +``` + +Expected: a local Codex review report before any push or PR creation. + +- [ ] **Step 3: Verify and resolve every P1/P2 finding** + +For each reported finding: + +1. Reproduce or prove it from the code and tests. +2. Add or update a regression test first when behavior changes. +3. Run the focused test and observe the expected RED result. +4. Apply the smallest valid fix. +5. Run the focused test to GREEN. + +If the review has confirmed P1/P2 findings, stage the tracked files changed by +the verified fixes and commit them with: + +```bash +git add -u +git commit -m "fix(playback): address local review findings" +``` + +If a finding is invalid, record the concrete code/test evidence for rejecting +it in the task summary instead of changing the implementation. + +- [ ] **Step 4: Re-review after fixes** + +If step 3 changed any file, send the same reviewer a follow-up request to +re-check the new `origin/master...HEAD` diff for remaining P0/P1/P2 findings. +Repeat steps 3 and 4 until no confirmed P1/P2 findings remain. + +- [ ] **Step 5: Re-run final verification after review** + +Run the complete validation command set from step 1 again. + +Expected: all commands exit 0 after the final review changes, and +`git status --short` is empty. + +### Task 6: Push and create the focused pull request + +**Files:** +- Verify only: `.github/pull_request_template.md` when present + +- [ ] **Step 1: Inspect the final commit range** + +Run: + +```bash +git log --oneline origin/master..HEAD +git diff --stat origin/master...HEAD +git status --short --branch +``` + +Expected: only the design/plan, focused diagnostic implementation, regression +tests, canonical documentation, and release note are present; the worktree is +clean. + +- [ ] **Step 2: Push the branch** + +Run: + +```bash +git push -u origin agent/fix-playback-diagnostic-1159 +``` + +Expected: push succeeds and configures the upstream branch. + +- [ ] **Step 3: Create the ready pull request** + +Create a non-draft PR with title: + +```text +fix(playback): avoid false codec diagnostics +``` + +Use this body: + +```markdown +## Summary + +- stop treating an ambiguous native `MediaError` code 4 as proof of an unsupported codec +- preserve and show explicit Video.js HTTP error statuses such as 404 +- keep confirmed container/codec diagnostics and external fallback behavior evidence-based + +## Testing + +- `pnpm nx test ui-playback` +- `pnpm nx lint ui-playback` +- `pnpm run typecheck:web` +- `pnpm run i18n:check` +- `pnpm run release:notes:validate` +- local Codex review completed with no unresolved P1/P2 findings + +Closes #1159 +``` + +Expected: a ready PR targeting `master`, created only after local review and +final validation succeed. diff --git a/docs/superpowers/specs/2026-07-30-accurate-native-playback-diagnostics-design.md b/docs/superpowers/specs/2026-07-30-accurate-native-playback-diagnostics-design.md new file mode 100644 index 000000000..c8291bbff --- /dev/null +++ b/docs/superpowers/specs/2026-07-30-accurate-native-playback-diagnostics-design.md @@ -0,0 +1,146 @@ +# Accurate Native Playback Diagnostics + +## Context + +Issue #1159 reports that an unavailable HLS URL is presented as an unsupported +codec. Video.js can surface a failed request as `MediaError` code 4 +(`MEDIA_ERR_SRC_NOT_SUPPORTED`), but that code does not prove that the source's +codec is incompatible. The current native classifier treats every code-4 +failure that is not a known unsupported container as `unsupported-codec`, so an +`.m3u8` request with an HTTP failure receives misleading codec wording and a +native-player recommendation. + +The local Video.js boundary also narrows `player.error()` to `code` and +`message`, even though Video.js errors may expose an HTTP `status` and +structured `metadata`. That prevents the shared diagnostic from retaining an +explicit response status such as 404. + +## Goals + +- Never claim that an ambiguous native code-4 failure proves an unsupported + codec. +- Preserve a valid HTTP error status reported by Video.js and classify it as a + provider/network loading failure. +- Show the HTTP status prominently and in technical details without adding a + new top-level diagnostic code or new translated prose. +- Keep confirmed unsupported-container and independently confirmed + unsupported-codec diagnostics unchanged. +- Add regression coverage for the issue's HLS/404 shape. + +## Non-goals + +- Redesign the complete playback diagnostic taxonomy. +- Capture the richer hls.js, Shaka, mpegts.js, Embedded MPV, MPV, or VLC error + models. +- Add confidence levels, per-player fallback likelihood, recoverable warning + history, stream probes, or automatic player failover. +- Persist or correlate playback attempts across engines. +- Infer an HTTP status, CORS failure, codec failure, or provider outage when the + runtime does not expose structured evidence. + +## Diagnostic Contract + +Extend the native error input with the safe Video.js fields needed by the +classifier: + +- optional numeric `status`; +- optional metadata containing a bounded vendor `errorType`. + +Extend `PlaybackDiagnostic` with: + +- optional `httpStatus`; +- optional `nativeErrorType`. + +Only integer HTTP error statuses from 400 through 599 are accepted as evidence. +Status `0`, missing values, success statuses, redirects, strings, and +out-of-range values remain unknown. Arbitrary metadata, response bodies, +headers, and URLs are not copied into the diagnostic because they may contain +credentials or provider data. A metadata error type is retained only when it +matches the vendor-identifier form `[A-Za-z0-9._:-]{1,128}`; every other value +is ignored rather than truncated or rendered. + +## Classification Rules + +Native classification uses the following precedence: + +1. An explicit valid HTTP error status produces `network-error`, retains the + status, and does not recommend an external player. The same URL and request + context are expected to fail independently of the decoder. +2. Native code 2 keeps the existing network/browser-access classification. +3. Native code 3 keeps the existing media-decode classification. +4. Native code 4 remains `unsupported-container` only when source metadata + independently identifies a container already known to be unsuitable for the + browser path. +5. Every other native code-4 failure becomes `unknown-playback-error` and does + not recommend an external player. The UI must not substitute a codec or + network guess. +6. Other native errors keep the existing unknown classification. + +`unsupported-codec` remains available when codec incompatibility is supported +by independent evidence, such as the HLS incompatible-codec error details or +the existing manifest codec capability check. + +## Video.js Boundary + +The focused `VideoJsPlayer` error type retains `status` and the safe metadata +error type instead of narrowing the error to `code` and `message`. +`VjsPlayerComponent` continues to pass `player.error()` into the shared native +classifier; no Video.js-specific classifier or duplicate UI path is added. + +This PR does not subscribe to additional VHS request events. If Video.js does +not expose an HTTP status on its terminal error, the result deliberately stays +ambiguous. + +## User Interface + +An HTTP-backed `network-error` uses the existing translated network title and +description. Its visible diagnostic metadata shows `HTTP ` before +container or MIME information, so the issue's unavailable HLS source displays +`HTTP 404` rather than `m3u8`. The existing translated “Error details” row +combines the safe HTTP status and Video.js error type when present, avoiding +new translation keys. + +An ambiguous code-4 failure uses the existing unknown-playback title and +description. Because no external fallback is recommended without codec, +container, decode, or browser-access evidence, the surface retains retry/copy +actions but does not present MPV or VLC as a likely fix. + +No new translation keys or layout changes are required. + +## Testing + +Use test-driven development: + +- Add a classifier regression proving that code 4 plus `.m3u8` and status 404 + becomes `network-error`, retains `httpStatus: 404`, and does not recommend an + external player. +- Add a classifier regression proving that code 4 plus `.m3u8` without a valid + status becomes `unknown-playback-error`, not `unsupported-codec`. +- Prove that status `0` is not presented as an HTTP response. +- Keep the existing known-container code-4 case as + `unsupported-container`. +- Update the Video.js component test to prove that `status` and the safe + metadata error type cross the component boundary. +- Update focused diagnostic-view tests to prove that `HTTP 404` is visible and + the new technical fields are rendered. + +Run the `ui-playback` unit test and lint targets, the workspace typecheck +target, i18n drift validation, and release-note validation. No new E2E flow is +required because the diagnostic overlay layout and user interaction are +unchanged; the classifier, component boundary, and rendered metadata are +covered by focused unit/component tests. + +## Documentation And Release Note + +Update `docs/architecture/embedded-inline-playback.md` to document the +evidence requirement for native code 4 and preservation of explicit HTTP +statuses. Add one `fix` release note under `.changes/` for issue #1159. + +## Alternatives Considered + +- **Copy-only fix:** Mapping every code-4 failure to the existing unknown text + would remove the false codec claim, but would continue discarding an explicit + 404 and provide less useful support information. +- **Full diagnostic redesign:** Adding the complete HTTP/manifest/decode + taxonomy and all player adapters would improve coverage, but it is too broad + for the focused regression fix and will be handled in separate work. diff --git a/libs/ui/playback/src/lib/playback-diagnostics/playback-diagnostics.model.ts b/libs/ui/playback/src/lib/playback-diagnostics/playback-diagnostics.model.ts index 1079ab2f1..01a75c62a 100644 --- a/libs/ui/playback/src/lib/playback-diagnostics/playback-diagnostics.model.ts +++ b/libs/ui/playback/src/lib/playback-diagnostics/playback-diagnostics.model.ts @@ -54,9 +54,15 @@ export interface PlaybackSourceMetadata { readonly videoCodecs: readonly string[]; } +export interface NativePlaybackErrorMetadataInput { + readonly errorType?: unknown; +} + export interface NativePlaybackErrorInput { readonly code?: number; readonly message?: string; + readonly status?: number; + readonly metadata?: NativePlaybackErrorMetadataInput; } export interface HlsPlaybackErrorInput { @@ -88,6 +94,8 @@ export interface PlaybackDiagnostic { readonly details?: string; readonly nativeErrorCode?: number; readonly nativeErrorMessage?: string; + readonly httpStatus?: number; + readonly nativeErrorType?: string; readonly externalFallbackRecommended: boolean; } diff --git a/libs/ui/playback/src/lib/playback-diagnostics/playback-diagnostics.util.spec.ts b/libs/ui/playback/src/lib/playback-diagnostics/playback-diagnostics.util.spec.ts index 0dbf6413f..5fcaba8b6 100644 --- a/libs/ui/playback/src/lib/playback-diagnostics/playback-diagnostics.util.spec.ts +++ b/libs/ui/playback/src/lib/playback-diagnostics/playback-diagnostics.util.spec.ts @@ -88,7 +88,7 @@ describe('playback diagnostics', () => { expect(issue.container).toBe('x-msvideo'); }); - it('does not classify MPEG-TS MIME-only failures as unsupported containers', () => { + it('keeps MPEG-TS MIME-only source failures unknown without codec evidence', () => { const issue = classifyNativePlaybackIssue( { code: 4, message: 'source not supported' }, createPlaybackSourceMetadata({ @@ -98,8 +98,109 @@ describe('playback diagnostics', () => { }) ); - expect(issue.code).toBe(PlaybackDiagnosticCode.UnsupportedCodec); + expect(issue.code).toBe(PlaybackDiagnosticCode.UnknownPlaybackError); expect(issue.container).toBe('mp2t'); + expect(issue.externalFallbackRecommended).toBe(false); + }); + + it('classifies native HTTP source failures from a safe status and error type', () => { + const issue = classifyNativePlaybackIssue( + { + code: 4, + message: 'source not supported', + status: 404, + metadata: { errorType: 'networkrequestfailed' }, + }, + createPlaybackSourceMetadata({ + url: 'https://example.com/live/missing.m3u8', + player: 'videojs', + }) + ); + + expect(issue.code).toBe(PlaybackDiagnosticCode.NetworkError); + expect(issue.httpStatus).toBe(404); + expect(issue.nativeErrorType).toBe('networkrequestfailed'); + expect(issue.externalFallbackRecommended).toBe(false); + }); + + it('keeps native code four HLS source failures unknown without status or container evidence', () => { + const issue = classifyNativePlaybackIssue( + { code: 4, message: 'source not supported' }, + createPlaybackSourceMetadata({ + url: 'https://example.com/live/missing.m3u8', + player: 'videojs', + }) + ); + + expect(issue.code).toBe(PlaybackDiagnosticCode.UnknownPlaybackError); + expect(issue.externalFallbackRecommended).toBe(false); + expect(issue.httpStatus).toBeUndefined(); + }); + + it('does not retain unsafe native status or metadata error types', () => { + const issue = classifyNativePlaybackIssue( + { + code: 4, + message: 'source not supported', + status: 0, + metadata: { errorType: 'request failed: token=secret value' }, + }, + createPlaybackSourceMetadata({ + url: 'https://example.com/live/missing.m3u8', + player: 'videojs', + }) + ); + + expect(issue.code).toBe(PlaybackDiagnosticCode.UnknownPlaybackError); + expect(issue.httpStatus).toBeUndefined(); + expect(issue.nativeErrorType).toBeUndefined(); + expect(issue.externalFallbackRecommended).toBe(false); + }); + + it.each([ + { status: 399, accepted: false }, + { status: 400, accepted: true }, + { status: 599, accepted: true }, + { status: 600, accepted: false }, + { status: 404.5, accepted: false }, + ])('accepts native HTTP status $status only when it is a 4xx or 5xx integer', ({ + status, + accepted, + }) => { + const issue = classifyNativePlaybackIssue( + { code: 4, status }, + createPlaybackSourceMetadata({ + url: 'https://example.com/live/missing.m3u8', + player: 'videojs', + }) + ); + + expect(issue.code).toBe( + accepted + ? PlaybackDiagnosticCode.NetworkError + : PlaybackDiagnosticCode.UnknownPlaybackError + ); + expect(issue.httpStatus).toBe(accepted ? status : undefined); + }); + + it.each([ + { length: 128, accepted: true }, + { length: 129, accepted: false }, + ])('retains native error type identifiers up to $length characters', ({ + length, + accepted, + }) => { + const errorType = 'a'.repeat(length); + const issue = classifyNativePlaybackIssue( + { code: 4, metadata: { errorType } }, + createPlaybackSourceMetadata({ + url: 'https://example.com/live/missing.m3u8', + player: 'videojs', + }) + ); + + expect(issue.code).toBe(PlaybackDiagnosticCode.UnknownPlaybackError); + expect(issue.nativeErrorType).toBe(accepted ? errorType : undefined); }); it('classifies HLS network errors without claiming codec incompatibility', () => { diff --git a/libs/ui/playback/src/lib/playback-diagnostics/playback-diagnostics.util.ts b/libs/ui/playback/src/lib/playback-diagnostics/playback-diagnostics.util.ts index 298f054bb..33efff939 100644 --- a/libs/ui/playback/src/lib/playback-diagnostics/playback-diagnostics.util.ts +++ b/libs/ui/playback/src/lib/playback-diagnostics/playback-diagnostics.util.ts @@ -32,6 +32,7 @@ export { const SOURCE_NOT_SUPPORTED_CODE = 4; const DECODE_ERROR_CODE = 3; const NETWORK_ERROR_CODE = 2; +const NATIVE_ERROR_TYPE_PATTERN = /^[A-Za-z0-9._:-]{1,128}$/; export function classifyNativePlaybackIssue( error: NativePlaybackErrorInput | MediaError | null | undefined, @@ -39,8 +40,25 @@ export function classifyNativePlaybackIssue( ): PlaybackDiagnostic { const nativeErrorCode = error?.code; const nativeErrorMessage = error?.message || undefined; + const nativeErrorInput = error as NativePlaybackErrorInput | null | undefined; + const httpStatus = getNativeHttpStatus(nativeErrorInput?.status); + const nativeErrorType = getNativeErrorType( + nativeErrorInput?.metadata?.errorType + ); const lowerNativeErrorMessage = nativeErrorMessage?.toLowerCase() ?? ''; + if (httpStatus !== undefined) { + return createPlaybackDiagnostic({ + code: DiagnosticCode.NetworkError, + source: DiagnosticSource.Native, + metadata, + nativeErrorCode, + nativeErrorMessage, + httpStatus, + nativeErrorType, + }); + } + if (nativeErrorCode === NETWORK_ERROR_CODE) { // Native MediaError details are often opaque for browser security // failures. Only classify browser access when the runtime exposes a @@ -53,6 +71,7 @@ export function classifyNativePlaybackIssue( metadata, nativeErrorCode, nativeErrorMessage, + nativeErrorType, }); } @@ -63,6 +82,7 @@ export function classifyNativePlaybackIssue( metadata, nativeErrorCode, nativeErrorMessage, + nativeErrorType, }); } @@ -70,11 +90,12 @@ export function classifyNativePlaybackIssue( return createPlaybackDiagnostic({ code: isLikelyContainerIssue(metadata) ? DiagnosticCode.UnsupportedContainer - : DiagnosticCode.UnsupportedCodec, + : DiagnosticCode.UnknownPlaybackError, source: DiagnosticSource.Native, metadata, nativeErrorCode, nativeErrorMessage, + nativeErrorType, }); } @@ -84,6 +105,7 @@ export function classifyNativePlaybackIssue( metadata, nativeErrorCode, nativeErrorMessage, + nativeErrorType, }); } @@ -237,6 +259,8 @@ export function createPlaybackDiagnostic(options: { readonly details?: string; readonly nativeErrorCode?: number; readonly nativeErrorMessage?: string; + readonly httpStatus?: number; + readonly nativeErrorType?: string; /** Overrides the code-derived recommendation, e.g. when external players * are known to be unable to handle the stream either. */ readonly externalFallbackRecommended?: boolean; @@ -248,6 +272,8 @@ export function createPlaybackDiagnostic(options: { details, nativeErrorCode, nativeErrorMessage, + httpStatus, + nativeErrorType, } = options; return { @@ -262,12 +288,30 @@ export function createPlaybackDiagnostic(options: { details: details || undefined, nativeErrorCode, nativeErrorMessage, + httpStatus, + nativeErrorType, externalFallbackRecommended: options.externalFallbackRecommended ?? isExternalFallbackRecommended(code), }; } +function getNativeHttpStatus(status: unknown): number | undefined { + return typeof status === 'number' && + Number.isInteger(status) && + status >= 400 && + status <= 599 + ? status + : undefined; +} + +function getNativeErrorType(errorType: unknown): string | undefined { + return typeof errorType === 'string' && + NATIVE_ERROR_TYPE_PATTERN.test(errorType) + ? errorType + : undefined; +} + function isExternalFallbackRecommended(code: PlaybackDiagnosticCode): boolean { return ( code === DiagnosticCode.UnsupportedContainer || diff --git a/libs/ui/playback/src/lib/vjs-player/vjs-player.component.spec.ts b/libs/ui/playback/src/lib/vjs-player/vjs-player.component.spec.ts index c05fc65b2..b6fdac918 100644 --- a/libs/ui/playback/src/lib/vjs-player/vjs-player.component.spec.ts +++ b/libs/ui/playback/src/lib/vjs-player/vjs-player.component.spec.ts @@ -1,5 +1,6 @@ import { ComponentFixture, TestBed } from '@angular/core/testing'; import { By } from '@angular/platform-browser'; +import type { NativePlaybackErrorInput } from '../playback-diagnostics/playback-diagnostics.model'; import type { PlaybackDiagnostic } from '../playback-diagnostics/playback-diagnostics.util'; import type { VjsPlayerComponent as VjsPlayerComponentInstance } from './vjs-player.component'; import type { VideoJsPlayer } from './vjs-player.types'; @@ -200,6 +201,38 @@ describe('VjsPlayerComponent', () => { ); }); + it('preserves Video.js HTTP error context in a playback diagnostic', () => { + const issues: Array = []; + component.playbackIssue.subscribe((issue) => issues.push(issue)); + render({ + sources: [ + { + src: 'https://example.test/missing/playlist.m3u8', + type: 'application/x-mpegURL', + }, + ], + }); + harness.currentError = { + code: 4, + message: 'The media could not be loaded', + status: 404, + metadata: { errorType: 'networkrequestfailed' }, + }; + + harness.emit('error'); + + expect(issues.at(-1)).toEqual( + expect.objectContaining({ + code: 'network-error', + source: 'native', + sourceUrl: 'https://example.test/missing/playlist.m3u8', + httpStatus: 404, + nativeErrorType: 'networkrequestfailed', + externalFallbackRecommended: false, + }) + ); + }); + it('rebinds native ended handling after playerreset', () => { const events: string[] = []; component.playbackEnded.subscribe(() => events.push('ended')); @@ -278,7 +311,7 @@ function createPlayerHarness() { let volumeValue = 0.5; const harness = { currentVideo: document.createElement('video'), - currentError: null as { code?: number; message?: string } | null, + currentError: null as NativePlaybackErrorInput | null, paused: true, pauseCompletesImmediately: true, ready: () => undefined, diff --git a/libs/ui/playback/src/lib/vjs-player/vjs-player.types.ts b/libs/ui/playback/src/lib/vjs-player/vjs-player.types.ts index 5c4b64149..a6a2d25c3 100644 --- a/libs/ui/playback/src/lib/vjs-player/vjs-player.types.ts +++ b/libs/ui/playback/src/lib/vjs-player/vjs-player.types.ts @@ -1,4 +1,5 @@ import type videoJs from 'video.js'; +import type { NativePlaybackErrorInput } from '../playback-diagnostics/playback-diagnostics.model'; export type VideoPlayerSource = { src: string; @@ -83,7 +84,7 @@ export type VideoJsPlayer = Omit< textTracks: () => VideoJsTextTrackList | null; tech: (options?: unknown) => VideoJsTech | null; getChild: (name: string) => VideoJsControlChild | null; - error: () => { code?: number; message?: string } | null; + error: () => NativePlaybackErrorInput | null; }; export function getVideoJsTechVideo( diff --git a/libs/ui/playback/src/lib/web-player-view/web-player-view-diagnostics.utils.ts b/libs/ui/playback/src/lib/web-player-view/web-player-view-diagnostics.utils.ts index d852115a0..60d5b8800 100644 --- a/libs/ui/playback/src/lib/web-player-view/web-player-view-diagnostics.utils.ts +++ b/libs/ui/playback/src/lib/web-player-view/web-player-view-diagnostics.utils.ts @@ -28,6 +28,10 @@ export function getDiagnosticDescriptionKey( } export function getDiagnosticMeta(issue: PlaybackDiagnostic): string { + if (issue.httpStatus !== undefined) { + return `HTTP ${issue.httpStatus}`; + } + const codecs = [...issue.videoCodecs, ...issue.audioCodecs].join(', '); if (codecs) { return codecs; @@ -82,11 +86,21 @@ export function getDiagnosticDetails( }, { labelKey: 'PLAYBACK_DIAGNOSTICS.DETAIL_ERROR_DETAILS', - value: issue.details ?? '', + value: formatDiagnosticErrorDetails(issue), }, ].filter(({ value }) => value.trim().length > 0); } +function formatDiagnosticErrorDetails(issue: PlaybackDiagnostic): string { + return [ + issue.httpStatus !== undefined ? `HTTP ${issue.httpStatus}` : '', + issue.nativeErrorType ?? '', + issue.details ?? '', + ] + .filter((value) => value.trim().length > 0) + .join(' · '); +} + function getDiagnosticTranslationBase(issue: PlaybackDiagnostic): string { switch (issue.code) { case PlaybackDiagnosticCode.UnsupportedContainer: diff --git a/libs/ui/playback/src/lib/web-player-view/web-player-view.component.spec.ts b/libs/ui/playback/src/lib/web-player-view/web-player-view.component.spec.ts index d74d113c9..340e2d423 100644 --- a/libs/ui/playback/src/lib/web-player-view/web-player-view.component.spec.ts +++ b/libs/ui/playback/src/lib/web-player-view/web-player-view.component.spec.ts @@ -289,6 +289,34 @@ describe('WebPlayerViewComponent', () => { ]); }); + it('renders explicit HTTP evidence without recommending an external fallback', () => { + runtimeCapabilities.supportsManagedExternalPlayers = true; + const issue = createHttpDiagnostic(); + + fixture.detectChanges(); + component.handlePlaybackIssue(issue); + fixture.detectChanges(); + + const banner = fixture.debugElement.query( + By.css('[data-test-id="playback-diagnostic-banner"]') + ); + const mpvButton = fixture.debugElement.query( + By.css('[data-test-id="playback-fallback-mpv"]') + ); + + expect(banner.nativeElement.textContent).toContain('HTTP 404'); + expect(mpvButton).toBeNull(); + expect(component.getDiagnosticMeta(issue)).toBe('HTTP 404'); + expect(component.getDiagnosticDetails(issue)).toEqual( + expect.arrayContaining([ + { + labelKey: 'PLAYBACK_DIAGNOSTICS.DETAIL_ERROR_DETAILS', + value: 'HTTP 404 · networkrequestfailed', + }, + ]) + ); + }); + it('keeps query-declared HLS streams on the HLS mime type', () => { const streamUrl = 'https://example.com/play?extension=m3u8&token=signed'; @@ -831,3 +859,21 @@ function createNetworkDiagnostic(): PlaybackDiagnostic { externalFallbackRecommended: false, }; } + +function createHttpDiagnostic(): PlaybackDiagnostic { + return { + code: PlaybackDiagnosticCode.NetworkError, + source: PlaybackDiagnosticSource.Native, + sourceUrl: 'https://example.com/live/missing.m3u8', + container: 'm3u8', + mimeType: 'application/x-mpegURL', + player: 'videojs', + audioCodecs: [], + videoCodecs: [], + nativeErrorCode: 4, + nativeErrorMessage: 'source not supported', + httpStatus: 404, + nativeErrorType: 'networkrequestfailed', + externalFallbackRecommended: false, + }; +}