mirror of
https://github.com/4gray/iptvnator.git
synced 2026-10-15 21:56:18 -08:00
chore(playback): address MKV review feedback
This commit is contained in:
4gray committed
1 parent
c13b781af0
commit
aa9c9c6f7e
3 files changed
+4
-461
No files matched your search
@@ -1,342 +0,0 @@
|
||||
# Native Matroska Inline Playback 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:** Let the Video.js inline player attempt MKV playback through Chromium's native Matroska pipeline while preserving codec-dependent diagnostics and external-player fallback.
|
||||
|
||||
**Architecture:** Keep `WebPlayerViewComponent` as the single Video.js source-type router and keep `getPlaybackMediaExtensionFromUrl()` as the canonical parser for path and query metadata. Add only an explicit MKV MIME branch; ArtPlayer and HTML5 remain on their existing native-video paths, and native playback errors remain authoritative.
|
||||
|
||||
**Tech Stack:** Angular 21, TypeScript, Jest/Nx, Electron 41/Chromium 146, agent-browser CDP
|
||||
|
||||
---
|
||||
|
||||
## Task 1: Add failing Video.js MIME regression tests
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `libs/ui/playback/src/lib/web-player-view/web-player-view.component.spec.ts`
|
||||
- Test: `libs/ui/playback/src/lib/web-player-view/web-player-view.component.spec.ts`
|
||||
|
||||
- [x] **Step 1: Add a path-extension regression test**
|
||||
|
||||
Add this test beside the existing query-declared HLS test:
|
||||
|
||||
```ts
|
||||
it('uses the Matroska mime type for MKV paths', () => {
|
||||
const streamUrl = 'https://example.com/archive/movie.mkv';
|
||||
|
||||
component.setVjsOptions(streamUrl);
|
||||
|
||||
expect(component.vjsOptions.sources).toEqual([
|
||||
{
|
||||
src: streamUrl,
|
||||
type: 'video/matroska',
|
||||
},
|
||||
]);
|
||||
});
|
||||
```
|
||||
|
||||
- [x] **Step 2: Add a query-metadata regression test**
|
||||
|
||||
```ts
|
||||
it('uses the Matroska mime type for query-declared MKV streams', () => {
|
||||
const streamUrl =
|
||||
'https://example.com/play?container=mkv&token=signed';
|
||||
|
||||
component.setVjsOptions(streamUrl);
|
||||
|
||||
expect(component.vjsOptions.sources).toEqual([
|
||||
{
|
||||
src: streamUrl,
|
||||
type: 'video/matroska',
|
||||
},
|
||||
]);
|
||||
});
|
||||
```
|
||||
|
||||
- [x] **Step 3: Run the focused spec and verify RED**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
pnpm nx test ui-playback --skip-nx-cache --runInBand --runTestsByPath libs/ui/playback/src/lib/web-player-view/web-player-view.component.spec.ts
|
||||
```
|
||||
|
||||
Expected: the two new expectations fail because the actual source type is still `video/mp4`; existing tests continue to pass.
|
||||
|
||||
## Task 2: Route MKV sources as Matroska
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `libs/ui/playback/src/lib/web-player-view/web-player-view.component.ts`
|
||||
- Test: `libs/ui/playback/src/lib/web-player-view/web-player-view.component.spec.ts`
|
||||
|
||||
- [x] **Step 1: Add the smallest production change**
|
||||
|
||||
Change the MIME decision to:
|
||||
|
||||
```ts
|
||||
const mimeType =
|
||||
extension === 'm3u' || extension === 'm3u8'
|
||||
? 'application/x-mpegURL'
|
||||
: extension === 'ts' || !extension
|
||||
? 'video/mp2t'
|
||||
: extension === 'mkv'
|
||||
? 'video/matroska'
|
||||
: 'video/mp4';
|
||||
```
|
||||
|
||||
Do not add `canPlayType()` preflight or a codec list: the actual native load result remains authoritative.
|
||||
|
||||
- [x] **Step 2: Run the focused spec and verify GREEN**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
pnpm nx test ui-playback --skip-nx-cache --runInBand --runTestsByPath libs/ui/playback/src/lib/web-player-view/web-player-view.component.spec.ts
|
||||
```
|
||||
|
||||
Expected: all tests in the spec pass.
|
||||
|
||||
- [x] **Step 3: Run the complete affected unit-test target**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
pnpm nx test ui-playback --skip-nx-cache --runInBand
|
||||
```
|
||||
|
||||
Expected: PASS.
|
||||
|
||||
## Task 3: Document the native MKV contract
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `docs/architecture/embedded-inline-playback.md`
|
||||
|
||||
- [x] **Step 1: Extend the codec/container diagnostics section**
|
||||
|
||||
After the URL extension metadata paragraph, document:
|
||||
|
||||
```md
|
||||
MKV sources are attempted through Chromium's native Matroska path. Video.js
|
||||
receives `video/matroska` for `.mkv` URLs and explicit query metadata such as
|
||||
`extension=mkv` or `container=mkv`; ArtPlayer and HTML5 continue to use their
|
||||
native video paths. This is container support rather than a universal codec
|
||||
guarantee: native source/decode failures still produce the existing diagnostic
|
||||
and explicit MPV/VLC fallback.
|
||||
```
|
||||
|
||||
- [x] **Step 2: Verify Markdown and whitespace**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
git diff --check
|
||||
git diff -- docs/architecture/embedded-inline-playback.md
|
||||
```
|
||||
|
||||
Expected: no whitespace errors; the diff describes only the new MKV contract.
|
||||
|
||||
## Task 4: Verify the Electron runtime with a supported MKV
|
||||
|
||||
**Files:**
|
||||
|
||||
- Temporary fixture only: `/tmp/iptvnator-mkv-inline-playback/video.mkv`
|
||||
- No committed binary fixture
|
||||
|
||||
- [x] **Step 1: Download a small H.264/AAC Matroska fixture**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
mkdir -p /tmp/iptvnator-mkv-inline-playback
|
||||
curl --fail --location https://remotion.media/video.mkv --output /tmp/iptvnator-mkv-inline-playback/video.mkv
|
||||
```
|
||||
|
||||
Record the downloaded SHA-256 in the verification notes:
|
||||
|
||||
```bash
|
||||
shasum -a 256 /tmp/iptvnator-mkv-inline-playback/video.mkv
|
||||
```
|
||||
|
||||
- [x] **Step 2: Start an isolated IPTVnator Electron runtime**
|
||||
|
||||
If the standard IPTVnator development ports are free, run in a long-lived
|
||||
terminal:
|
||||
|
||||
```bash
|
||||
IPTVNATOR_TRACE_RENDERER_CONSOLE=1 pnpm nx serve electron-backend
|
||||
```
|
||||
|
||||
Wait for the IPTVnator renderer target on `127.0.0.1:9222`. If another
|
||||
worktree already owns the development ports, do not stop it; instead launch
|
||||
the repository's Electron 41 binary with a temporary BrowserWindow harness on
|
||||
an unused CDP port and remove the harness after the smoke.
|
||||
|
||||
- [x] **Step 3: Exercise Chromium's native Matroska pipeline in an Electron renderer**
|
||||
|
||||
Use the repository's documented Electron CDP workflow and `agent-browser` to
|
||||
attach to the IPTVnator page. In the renderer:
|
||||
|
||||
1. Read `/tmp/iptvnator-mkv-inline-playback/video.mkv` through a temporary
|
||||
localhost range server that returns `Content-Type: video/matroska`.
|
||||
2. Create a temporary `<video muted autoplay>` smoke element using that URL.
|
||||
3. Wait for `loadedmetadata`.
|
||||
4. Assert `videoWidth > 0`, `duration > 0`, and `currentTime` advances.
|
||||
5. Remove the temporary element and stop the server.
|
||||
|
||||
This is a runtime capability check, while the focused component tests prove
|
||||
that the Video.js application path supplies the same `video/matroska` MIME.
|
||||
|
||||
- [x] **Step 4: Reconfirm unsupported-MKV fallback coverage**
|
||||
|
||||
Run the existing WebPlayerView spec containing the synthetic native MKV error:
|
||||
|
||||
```bash
|
||||
pnpm nx test ui-playback --skip-nx-cache --runInBand --runTestsByPath libs/ui/playback/src/lib/web-player-view/web-player-view.component.spec.ts
|
||||
```
|
||||
|
||||
Expected: PASS, including the existing MKV diagnostic and MPV fallback
|
||||
assertions.
|
||||
|
||||
## Task 5: Run final validation
|
||||
|
||||
**Files:**
|
||||
|
||||
- Verify only
|
||||
|
||||
- [x] **Step 1: Verify workspace discovery**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
pnpm nx show projects
|
||||
```
|
||||
|
||||
Expected: projects are discovered successfully.
|
||||
|
||||
- [x] **Step 2: Run affected lint**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
pnpm nx lint ui-playback --skip-nx-cache
|
||||
```
|
||||
|
||||
Expected: PASS.
|
||||
|
||||
- [x] **Step 3: Run web type checking**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
pnpm run typecheck:web
|
||||
```
|
||||
|
||||
Expected: PASS.
|
||||
|
||||
- [x] **Step 4: Inspect the complete change**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
git diff --check
|
||||
git diff --stat origin/master...HEAD
|
||||
git status --short
|
||||
```
|
||||
|
||||
Expected: no whitespace errors and only the design, plan, implementation,
|
||||
regression tests, and canonical playback documentation are in scope.
|
||||
|
||||
## Task 6: Perform local Codex review and address findings
|
||||
|
||||
**Files:**
|
||||
|
||||
- Review all branch changes against `origin/master`
|
||||
|
||||
- [x] **Step 1: Commit the implementation checkpoint**
|
||||
|
||||
Stage only the MKV plan, implementation, tests, and documentation, then commit:
|
||||
|
||||
```bash
|
||||
git add docs/superpowers/plans/2026-07-19-mkv-inline-playback.md \
|
||||
docs/architecture/embedded-inline-playback.md \
|
||||
libs/ui/playback/src/lib/web-player-view/web-player-view.component.ts \
|
||||
libs/ui/playback/src/lib/web-player-view/web-player-view.component.spec.ts
|
||||
git commit -m "fix(playback): route MKV sources as Matroska"
|
||||
```
|
||||
|
||||
- [x] **Step 2: Run local Codex code review**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
codex review --base origin/master
|
||||
```
|
||||
|
||||
Classify every finding by severity and verify it against the code. Fix all
|
||||
confirmed correctness, regression, security, or maintainability findings before
|
||||
publication. Re-run the targeted tests, lint, type check, and review after any
|
||||
material correction.
|
||||
|
||||
- [x] **Step 3: Confirm a clean, reviewed branch**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
git status --short
|
||||
git log --oneline --decorate origin/master..HEAD
|
||||
```
|
||||
|
||||
Expected: clean worktree and a focused two-commit branch (design plus
|
||||
implementation), unless the review required a clearly named follow-up commit.
|
||||
|
||||
## Task 7: Push and open the pull request
|
||||
|
||||
**Files:**
|
||||
|
||||
- GitHub branch and PR metadata only
|
||||
|
||||
- [ ] **Step 1: Confirm GitHub authentication**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
gh auth status
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Push the reviewed branch**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
git push --set-upstream origin agent/mkv-inline-playback
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Open a focused draft PR**
|
||||
|
||||
Create a draft PR targeting `master` with title:
|
||||
|
||||
```text
|
||||
fix(playback): route MKV sources as Matroska
|
||||
```
|
||||
|
||||
The body must summarize:
|
||||
|
||||
- Video.js now sends `.mkv` and query-declared MKV as `video/matroska`;
|
||||
- ArtPlayer/HTML5 behavior and error-driven fallback are unchanged;
|
||||
- codec support remains Chromium/platform-dependent;
|
||||
- automated test, lint, type-check, Electron smoke, and local Codex review
|
||||
results;
|
||||
- the temporary runtime fixture URL and SHA-256.
|
||||
|
||||
- [ ] **Step 4: Verify the published PR**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
gh pr view --json number,title,url,isDraft,baseRefName,headRefName,state
|
||||
```
|
||||
|
||||
Expected: an OPEN draft PR from `agent/mkv-inline-playback` into `master`.
|
||||
@@ -1,115 +0,0 @@
|
||||
# Native Matroska Inline Playback Design
|
||||
|
||||
## Context
|
||||
|
||||
IPTVnator's three embedded web players ultimately use Chromium's
|
||||
`HTMLVideoElement` for native-file playback. ArtPlayer already maps `.mkv`
|
||||
sources to `video/matroska`, and the HTML5 player assigns the source URL
|
||||
directly to its video element. Video.js is the inconsistent path:
|
||||
`WebPlayerViewComponent` currently classifies every non-HLS, non-MPEG-TS source
|
||||
as `video/mp4`, so an MKV URL is advertised to Video.js with the wrong
|
||||
container type.
|
||||
|
||||
Electron 41 embeds Chromium 146, whose native media pipeline recognizes
|
||||
Matroska. Successful playback still depends on the codecs inside the container
|
||||
and on platform decoder availability. The browser players must therefore try a
|
||||
correctly described MKV source and retain the existing error-driven MPV/VLC
|
||||
fallback rather than promising universal Matroska codec support.
|
||||
|
||||
## Goals
|
||||
|
||||
- Pass `.mkv` sources to Video.js as the registered MIME type
|
||||
`video/matroska`.
|
||||
- Recognize both path extensions and explicit IPTV query metadata such as
|
||||
`?extension=mkv` or `?container=mkv`.
|
||||
- Keep ArtPlayer and HTML5 on their existing native-video paths.
|
||||
- Preserve the existing browser playback error classification and explicit
|
||||
MPV/VLC fallback after a real source or decode failure.
|
||||
- Treat Electron as the validated runtime while leaving PWA playback as
|
||||
best-effort according to the user's browser.
|
||||
- Add regression coverage and perform a real Electron playback smoke before
|
||||
publication.
|
||||
|
||||
## Non-Goals
|
||||
|
||||
- Adding a JavaScript Matroska demuxer, transcoder, or remuxing backend.
|
||||
- Claiming support for every codec that Matroska can contain.
|
||||
- Automatically switching the saved player or silently launching an external
|
||||
player.
|
||||
- Changing Embedded MPV or external MPV/VLC playback.
|
||||
- Adding a codec list to the source MIME string when the provider has not
|
||||
supplied authoritative codec metadata.
|
||||
|
||||
## Selected Approach
|
||||
|
||||
Extend the existing source-type decision in `WebPlayerViewComponent` with an
|
||||
explicit MKV branch:
|
||||
|
||||
```text
|
||||
m3u / m3u8 -> application/x-mpegURL
|
||||
ts / no extension -> video/mp2t
|
||||
mkv -> video/matroska
|
||||
other -> video/mp4
|
||||
```
|
||||
|
||||
`getPlaybackMediaExtensionFromUrl()` remains the canonical extension parser, so
|
||||
signed URLs and query-declared formats use the same rule as normal `.mkv`
|
||||
paths. No preflight `canPlayType()` gate is added: without the actual codec list
|
||||
it can only provide a container-level guess and would duplicate the native
|
||||
player's authoritative load result.
|
||||
|
||||
ArtPlayer's existing `video/matroska` custom type continues to call its native
|
||||
source path. The HTML5 player continues to assign the URL directly and lets the
|
||||
response MIME type and Chromium sniffing participate in source selection.
|
||||
|
||||
## Error Handling
|
||||
|
||||
The current diagnostics run only after a native media error. They do not reject
|
||||
MKV before loading. A native `MEDIA_ERR_SRC_NOT_SUPPORTED` for an MKV source
|
||||
therefore remains classified as `unsupported-container`, while decode failures
|
||||
remain `media-decode-error`. Electron may continue to offer explicit MPV/VLC
|
||||
fallback actions; PWA builds continue to offer retry and copy/help behavior.
|
||||
|
||||
This preserves useful fallback for unsupported MKV codec combinations such as
|
||||
platform-dependent HEVC, AC-3/E-AC-3, DTS, or legacy MPEG-4 variants.
|
||||
|
||||
## Testing
|
||||
|
||||
### Automated regression coverage
|
||||
|
||||
- Add a `WebPlayerViewComponent` test proving a `.mkv` path produces a
|
||||
Video.js source with `type: 'video/matroska'`.
|
||||
- Add a second test proving query-declared MKV produces the same type.
|
||||
- Retain the existing ArtPlayer test for `.mkv -> video/matroska`.
|
||||
- Retain the existing native-error diagnostic test proving unsupported MKV
|
||||
still recommends the external fallback after an actual error.
|
||||
|
||||
### Runtime validation
|
||||
|
||||
Serve a small local MKV with byte-range support and
|
||||
`Content-Type: video/matroska`, then verify in the Electron runtime that:
|
||||
|
||||
- Video.js receives `video/matroska`;
|
||||
- metadata loads and playback time advances for a Chromium-supported codec
|
||||
combination;
|
||||
- no playback diagnostic appears for the supported fixture;
|
||||
- an unsupported fixture or synthetic native error still surfaces the existing
|
||||
fallback rather than hanging the player.
|
||||
|
||||
If a redistributable fixture cannot be committed without adding unnecessary
|
||||
binary weight, keep the automated contract tests in-tree and document the
|
||||
temporary local fixture used for the Electron smoke in the PR.
|
||||
|
||||
## Documentation
|
||||
|
||||
Update the codec/container diagnostics section of
|
||||
`docs/architecture/embedded-inline-playback.md` to record that MKV is attempted
|
||||
through Chromium's native Matroska path, that Video.js uses
|
||||
`video/matroska`, and that codec-dependent failures still use the existing
|
||||
MPV/VLC fallback.
|
||||
|
||||
## Rollback
|
||||
|
||||
The change is isolated to Video.js MIME routing. Reverting the explicit MKV
|
||||
branch restores the previous `video/mp4` fallback without affecting ArtPlayer,
|
||||
HTML5, Embedded MPV, or external players.
|
||||
@@ -157,9 +157,7 @@ export class WebPlayerViewComponent {
|
||||
this.settings()?.player ??
|
||||
VideoPlayer.VideoJs
|
||||
);
|
||||
readonly recordingFolder = computed(
|
||||
() => this.settings()?.recordingFolder ?? ''
|
||||
);
|
||||
readonly recordingFolder = computed(() => this.settings()?.recordingFolder ?? '');
|
||||
|
||||
constructor() {
|
||||
effect(() => {
|
||||
@@ -180,7 +178,9 @@ export class WebPlayerViewComponent {
|
||||
? 'application/x-mpegURL'
|
||||
: extension === 'ts' || !extension
|
||||
? 'video/mp2t'
|
||||
: extension === 'mkv' ? 'video/matroska' : 'video/mp4';
|
||||
: extension === 'mkv'
|
||||
? 'video/matroska'
|
||||
: 'video/mp4';
|
||||
|
||||
this.vjsOptions = {
|
||||
isLive,
|
||||
|
||||
Reference in new issue
Block a user