feat(multiview): multiview grid for live TV - #1212
Conversation
|
@greptileai Please review this pull request |
8984f57 to
597682a
Compare
|
@greptileai Please review this pull request |
Greptile SummaryThis PR introduces a Multiview page (
Confidence Score: 5/5Safe to merge — the feature is entirely additive, isolated in its own Nx library, and guarded by lazy loading; it does not modify any existing playback or data-access paths. All issues raised in prior review rounds have been addressed: the sanitizeSlot validation now covers contentType and sourceType enums; mpegts non-fatal errors are filtered; the picker double-open guard is in place; the request-ID counter is monotonically global; and the UID-split guard is present. No new functional defects were found in this pass. No files require special attention. The new multiview library is self-contained and the only pre-existing file modifications (app.routes.ts, workspace-shell-route-state.service.ts) are minimal and clearly scoped. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant User
participant Page as MultiviewPageComponent
participant Dialog as ChannelPickerDialog
participant State as MultiviewStateService
participant Resolver as StreamResolverService
participant Tile as MultiviewTileComponent
participant Engine as MultiviewTileEngine
User->>Page: click "Add Channel"
Page->>Page: pickerOpen guard check
Page->>Dialog: MatDialog.open()
Dialog->>Dialog: loadItems() [UnifiedFavoritesDataService + UnifiedRecentDataService]
User->>Dialog: select channel
Dialog->>Page: afterClosed() → MultiviewSlotChannel
Page->>State: assign(index, channel)
State->>State: persist() → localStorage
State->>Page: slots() signal change
Page->>Page: syncResolutions(slots)
Page->>Resolver: resolvePlayback(item)
Resolver-->>Page: "{ streamUrl, userAgent, referer }"
Page->>Page: "updateResolution(uid, {status:'ready', playback})"
Page->>Tile: [playback] input binding
Tile->>Engine: "new MultiviewTileEngine({ video, url, onError })"
Engine->>Engine: start() — mpegts.js / hls.js / native fallback
Engine-->>Tile: video plays (muted)
User->>Tile: click (focusRequested)
Tile->>Page: focusRequested.emit()
Page->>State: focusAudio(index)
State->>Tile: "audioFocused input = true"
Tile->>Tile: "video.muted = false"
User->>Tile: dblclick (openInPlayerRequested)
Tile->>Page: openInPlayerRequested.emit()
Page->>Page: openInPlayer(slot) → buildLiveCollectionNavigationTarget
Page->>Page: router.navigate(target.link)
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant User
participant Page as MultiviewPageComponent
participant Dialog as ChannelPickerDialog
participant State as MultiviewStateService
participant Resolver as StreamResolverService
participant Tile as MultiviewTileComponent
participant Engine as MultiviewTileEngine
User->>Page: click "Add Channel"
Page->>Page: pickerOpen guard check
Page->>Dialog: MatDialog.open()
Dialog->>Dialog: loadItems() [UnifiedFavoritesDataService + UnifiedRecentDataService]
User->>Dialog: select channel
Dialog->>Page: afterClosed() → MultiviewSlotChannel
Page->>State: assign(index, channel)
State->>State: persist() → localStorage
State->>Page: slots() signal change
Page->>Page: syncResolutions(slots)
Page->>Resolver: resolvePlayback(item)
Resolver-->>Page: "{ streamUrl, userAgent, referer }"
Page->>Page: "updateResolution(uid, {status:'ready', playback})"
Page->>Tile: [playback] input binding
Tile->>Engine: "new MultiviewTileEngine({ video, url, onError })"
Engine->>Engine: start() — mpegts.js / hls.js / native fallback
Engine-->>Tile: video plays (muted)
User->>Tile: click (focusRequested)
Tile->>Page: focusRequested.emit()
Page->>State: focusAudio(index)
State->>Tile: "audioFocused input = true"
Tile->>Tile: "video.muted = false"
User->>Tile: dblclick (openInPlayerRequested)
Tile->>Page: openInPlayerRequested.emit()
Page->>Page: openInPlayer(slot) → buildLiveCollectionNavigationTarget
Page->>Page: router.navigate(target.link)
Reviews (8): Last reviewed commit: "merge: integrate latest upstream into mu..." | Re-trigger Greptile |
Greptile SummaryThis PR adds a TiviMate-style multiview grid page under
Confidence Score: 3/5The feature works correctly for HLS and native streams; the main risk is that TS-stream tiles in degraded network conditions will show spurious error overlays requiring manual retry, and rapid double-clicking the empty-slot button can open two overlapping dialogs. The mpegts error-forwarding path discards the fatal/non-fatal distinction that the HLS path preserves, meaning any transient mpegts.js event — including ones the player would have self-recovered from — immediately tears down the engine and presents an error overlay. For users on TS-heavy playlists with variable network quality this will be a noticeable reliability gap. The double-dialog issue on the add button is a secondary usability defect. Both are straightforward to fix, but until they are the feature has rough edges that could frustrate users of TS streams. multiview-tile-engine.ts (mpegts error handling) and multiview-page.component.html (add-button double-click guard) Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant User
participant PageComponent as MultiviewPageComponent
participant StateService as MultiviewStateService
participant Picker as ChannelPickerDialog
participant Resolver as StreamResolverService
participant TileComponent as MultiviewTileComponent
participant TileEngine as MultiviewTileEngine
User->>PageComponent: click Add Channel
PageComponent->>Picker: dialog.open()
Picker-->>PageComponent: result (MultiviewSlotChannel)
PageComponent->>StateService: assign(index, channel)
StateService-->>StateService: persist to localStorage
StateService-->>PageComponent: slots() signal updated
PageComponent->>Resolver: resolvePlayback(item)
Resolver-->>PageComponent: streamUrl, title
PageComponent->>TileComponent: [playback] input set
TileComponent->>TileEngine: new MultiviewTileEngine
TileEngine->>TileEngine: start() — mpegts / hls.js / native
User->>TileComponent: click (focus audio)
TileComponent->>PageComponent: focusRequested
PageComponent->>StateService: focusAudio(index)
User->>TileComponent: dblclick (open in player)
TileComponent->>PageComponent: openInPlayerRequested
PageComponent->>User: router.navigate (full player)
alt Playback error
TileEngine->>TileComponent: onError(diagnostic)
TileComponent->>PageComponent: playbackFailed
PageComponent->>PageComponent: "updateResolution status=error"
User->>PageComponent: retry
PageComponent->>Resolver: resolvePlayback re-resolves URL
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant User
participant PageComponent as MultiviewPageComponent
participant StateService as MultiviewStateService
participant Picker as ChannelPickerDialog
participant Resolver as StreamResolverService
participant TileComponent as MultiviewTileComponent
participant TileEngine as MultiviewTileEngine
User->>PageComponent: click Add Channel
PageComponent->>Picker: dialog.open()
Picker-->>PageComponent: result (MultiviewSlotChannel)
PageComponent->>StateService: assign(index, channel)
StateService-->>StateService: persist to localStorage
StateService-->>PageComponent: slots() signal updated
PageComponent->>Resolver: resolvePlayback(item)
Resolver-->>PageComponent: streamUrl, title
PageComponent->>TileComponent: [playback] input set
TileComponent->>TileEngine: new MultiviewTileEngine
TileEngine->>TileEngine: start() — mpegts / hls.js / native
User->>TileComponent: click (focus audio)
TileComponent->>PageComponent: focusRequested
PageComponent->>StateService: focusAudio(index)
User->>TileComponent: dblclick (open in player)
TileComponent->>PageComponent: openInPlayerRequested
PageComponent->>User: router.navigate (full player)
alt Playback error
TileEngine->>TileComponent: onError(diagnostic)
TileComponent->>PageComponent: playbackFailed
PageComponent->>PageComponent: "updateResolution status=error"
User->>PageComponent: retry
PageComponent->>Resolver: resolvePlayback re-resolves URL
end
Prompt To Fix All With AIFix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
libs/workspace/multiview/feature/src/lib/multiview-tile-engine.ts:110-127
**mpegts errors are not filtered by severity unlike the HLS path**
The HLS error handler guards with `if (!data.fatal) { return; }` so only fatal errors surface. The mpegts handler forwards every `mpegts.Events.ERROR` event unconditionally. mpegts.js can fire non-fatal ERROR events for transient network blips (e.g., temporary packet loss), which will immediately set the tile to the error state and destroy the engine — requiring a manual retry even for issues the player would have recovered from on its own. TS-stream tiles in poor-network environments will be far less resilient than HLS tiles.
### Issue 2 of 3
libs/workspace/multiview/feature/src/lib/multiview-page.component.html:57-65
**Double-click on "Add Channel" opens two concurrent picker dialogs**
`(click)="openPicker($index)"` fires twice during a native double-click (browsers emit `click`→`click`→`dblclick`). Each click triggers a separate `MatDialog.open()` call synchronously before the first dialog is even rendered, so the user ends up with two overlapping channel-picker dialogs. The second call could silently overwrite any channel selected in the first. Adding a guard like checking whether a dialog is already open for the same slot, or using `(click)` with a debounce, would prevent this.
### Issue 3 of 3
libs/workspace/multiview/feature/src/lib/multiview-channel-picker-dialog.component.ts:70
**Unused public injection of `TranslateService`**
`translate` is injected and exposed as a public field, but the template exclusively uses the `TranslatePipe` (`| translate`) and none of the component's methods reference `this.translate`. If it was intended for direct template use (e.g., `translate.instant(...)`) it is not currently wired up; if it was left over from an earlier implementation it can be removed to keep the component surface clean.
Reviews (2): Last reviewed commit: "chore(multiview): address lint findings ..." | Re-trigger Greptile |
Greptile SummaryThis PR introduces a new
Confidence Score: 3/5Mostly solid new feature, but the resolve-orchestration layer in MultiviewPageComponent has a counter-reset bug that can leave a tile playing a stale (or failed) stream URL after a quick remove-and-reassign cycle. The state service, tile engine, layouts, and picker dialog are all well-structured and well-tested. The main concern is in MultiviewPageComponent: the requestIds map is cleared on slot removal, so immediately re-assigning the same channel resets the counter to 1, matching the still-inflight original request's ID. A stale error or stale (expiring) Xtream/Stalker token URL could overwrite the fresh resolve result depending on timing — a realistic scenario under slow or flaky network conditions. A second gap is the unguarded UID split used for full-player navigation, which silently passes undefined as itemId if the stored UID is malformed. libs/workspace/multiview/feature/src/lib/multiview-page.component.ts — the request-ID counter reset in syncResolutions and the uid.split guard in openInPlayer both need attention before merging. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant User
participant MultiviewPage as MultiviewPageComponent
participant StateService as MultiviewStateService
participant Resolver as StreamResolverService
participant TileEngine as MultiviewTileEngine
participant Player as Full Player (Router)
User->>MultiviewPage: open picker (slot N)
MultiviewPage->>MultiviewPage: open ChannelPickerDialog
User->>MultiviewPage: select channel
MultiviewPage->>StateService: assign(index, channel)
StateService-->>MultiviewPage: slots() signal updated
MultiviewPage->>Resolver: resolvePlayback(item)
Resolver-->>MultiviewPage: streamUrl, userAgent, referer
MultiviewPage->>MultiviewPage: updateResolution(uid, ready)
MultiviewPage->>TileEngine: new MultiviewTileEngine
TileEngine->>TileEngine: start() mpegts/hls/native branch
TileEngine-->>MultiviewPage: onError(diagnostic) on fatal error
User->>MultiviewPage: click tile
MultiviewPage->>StateService: focusAudio(index)
StateService-->>MultiviewPage: audioFocusIndex() updated
MultiviewPage-->>TileEngine: "audioFocused=true, video.muted=false"
User->>MultiviewPage: double-click tile
MultiviewPage->>Player: router.navigate
User->>MultiviewPage: retry error state
MultiviewPage->>Resolver: resolvePlayback re-resolves URL
Resolver-->>MultiviewPage: fresh streamUrl
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant User
participant MultiviewPage as MultiviewPageComponent
participant StateService as MultiviewStateService
participant Resolver as StreamResolverService
participant TileEngine as MultiviewTileEngine
participant Player as Full Player (Router)
User->>MultiviewPage: open picker (slot N)
MultiviewPage->>MultiviewPage: open ChannelPickerDialog
User->>MultiviewPage: select channel
MultiviewPage->>StateService: assign(index, channel)
StateService-->>MultiviewPage: slots() signal updated
MultiviewPage->>Resolver: resolvePlayback(item)
Resolver-->>MultiviewPage: streamUrl, userAgent, referer
MultiviewPage->>MultiviewPage: updateResolution(uid, ready)
MultiviewPage->>TileEngine: new MultiviewTileEngine
TileEngine->>TileEngine: start() mpegts/hls/native branch
TileEngine-->>MultiviewPage: onError(diagnostic) on fatal error
User->>MultiviewPage: click tile
MultiviewPage->>StateService: focusAudio(index)
StateService-->>MultiviewPage: audioFocusIndex() updated
MultiviewPage-->>TileEngine: "audioFocused=true, video.muted=false"
User->>MultiviewPage: double-click tile
MultiviewPage->>Player: router.navigate
User->>MultiviewPage: retry error state
MultiviewPage->>Resolver: resolvePlayback re-resolves URL
Resolver-->>MultiviewPage: fresh streamUrl
Prompt To Fix All With AIFix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
libs/workspace/multiview/feature/src/lib/multiview-page.component.ts:152-169
**Request-ID counter resets on slot removal, enabling stale writes**
When a channel is removed, `requestIds.delete(uid)` resets the counter to zero. If the same channel is immediately re-assigned, the new `resolveItem` call computes `requestId = (undefined ?? 0) + 1 = 1` — the same value as the original (now-stale) inflight request. Both calls pass the `this.requestIds.get(item.uid) !== requestId` guard (`1 !== 1` is false for both), so whichever completes last wins. If the stale call fails after the fresh call already marked the tile as `ready`, the tile reverts to `error` state; for Xtream/Stalker where each `resolvePlayback` mints a new expiring token, the stale URL (written last) would be used. Preserving the high-water-mark counter even through removal — e.g. storing a large monotonic value per UID instead of deleting the entry — prevents the collision.
### Issue 2 of 3
libs/workspace/multiview/feature/src/lib/multiview-page.component.ts:159-168
`item.uid.split('::')[2]` is `undefined` when the UID has fewer than three `::` segments. `sanitizeSlot` validates that `uid` is a string but not its format, so a persisted or injected UID like `"m3u::playlist-1"` (missing the item segment) would produce `undefined` as `itemId`. Depending on how `buildLiveCollectionNavigationTarget` handles that, navigation may silently route to a wrong or broken path. An early guard prevents this.
```suggestion
openInPlayer(slot: MultiviewSlotChannel): void {
const item = slot.item;
const parts = item.uid.split('::');
const itemId = parts[2];
if (!itemId) {
return;
}
const target = buildLiveCollectionNavigationTarget({
mode: slot.origin,
sourceType: item.sourceType,
playlistId: item.playlistId,
itemId,
title: item.name,
imageUrl: item.logo,
});
```
### Issue 3 of 3
libs/workspace/multiview/feature/src/lib/multiview-page.component.html:58
The `playbackFailed` output emits a `PlaybackDiagnostic` but the binding omits `$event`, so the diagnostic is silently dropped. The component method `onTileFailed` currently ignores it entirely, which means there is no way to log or telemetry the actual failure reason. Even if the current error message is intentionally generic, passing the diagnostic through keeps the door open for future observability without a template change.
```suggestion
(playbackFailed)="onTileFailed(slot, $event)"
```
Reviews (3): Last reviewed commit: "chore(multiview): address lint findings ..." | Re-trigger Greptile |
|
@greptileai Please review this pull request |
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1212 +/- ##
==========================================
- Coverage 71.05% 63.61% -7.44%
==========================================
Files 40 680 +640
Lines 691 40017 +39326
Branches 87 8723 +8636
==========================================
+ Hits 491 25458 +24967
- Misses 176 11325 +11149
- Partials 24 3234 +3210
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…pegts errors Persisted multiview slots now require contentType 'live', a known sourceType enum value, and a playlistName string before restore, and the mpegts.js error handler mirrors the hls.js fatal guard so explicitly non-fatal payloads no longer put a tile into the error state.
|
@greptileai Please review this pull request |
|
@greptileai Please review this pull request |
- drop in-flight stream resolutions on destroy and never start new ones after navigating away, so no ephemeral portal sessions are created - make request ids monotonic across slots so a removed-and-re-added channel cannot collide with a stale in-flight request - track connection-limit hint dismissal per portal-account combination; a new same-account conflict after dismissal shows the hint again - guard open-in-player against malformed persisted uids - prevent double-click from opening two channel-picker dialogs - pass the tile playback diagnostic through and log it for observability - remove unused TranslateService injection from the picker dialog
|
@greptileai Please review this pull request |
|
@greptileai Please review this pull request |
Summary
Adds a Multiview page (
/workspace/multiview, Electron and PWA) that plays several live TV channels simultaneously in a grid. This implements the "Multiview" part of the PiP/Multiview roadmap item.UnifiedFavoritesDataService/UnifiedRecentDataService); playback URLs resolve through the existingStreamResolverService.resolvePlayback(so Stalkercreate_linkand Xtream URL construction are reused, not duplicated)MultiviewTileEngine: mpegts.js for raw TS, hls.js with small buffers, native<video>fallback). Deliberately not the fullWebPlayerViewComponentstack, which is a per-app singleton (global keyboard shortcuts, shared volume persistence, single fullscreen/PiP)localStorage(multiview-state-v1)Implementation
libs/workspace/multiview/feature(@iptvnator/workspace/multiview/feature), lazy-loaded route under/workspacegrid-area), state is a component-provided signal serviceMULTIVIEW.*,WORKSPACE.SHELL.RAIL_MULTIVIEW)CLAUDE.mdupdated (routes, libs list, Key Features section)Test plan
pnpm run lintandpnpm nx build webgreenKnown limitations (v1)