frontend: video-manager: Rework thumbnails#3839
Merged
patrickelectric merged 4 commits intobluerobotics:1.4from Mar 20, 2026
Merged
frontend: video-manager: Rework thumbnails#3839patrickelectric merged 4 commits intobluerobotics:1.4from
patrickelectric merged 4 commits intobluerobotics:1.4from
Conversation
…d fix blob URL cleanup Debounce stopGetThumbnailForDevice by 15 seconds to ride through transient backend restarts without interrupting thumbnail polling. Move blob URL revocation to beforeDestroy to prevent premature cleanup.
…state Move thumbnail sources, busy set, and OneMoreTime task to a window-global singleton that persists across hot module reloads. Add reentrant guard to prevent duplicate fetchThumbnails execution from the double @module decorator. Reduce polling delay to 100ms and preserve last valid thumbnail on 503 errors.
…nail controls Default to paused thumbnail fetching to avoid unnecessary resource usage. Add snapshot (single-fetch) and play/pause (continuous 1s polling) controls. Show a warning that thumbnails use stream resources and may affect video quality. Replace v-avatar with a 16:9 aspect ratio frame for consistent placeholder and image dimensions.
Reviewer's GuideReworks frontend video thumbnail handling by introducing manual snapshot/continuous controls with status/latency overlay, centralizing thumbnail polling into an HMR-safe singleton with better blob URL lifecycle, and extending the video stream creation/controls dialogs with new config flags and proper form state reset. Sequence diagram for manual single-shot thumbnail fetchsequenceDiagram
actor User
participant VideoControlsDialog
participant VideoThumbnail
participant VideoStore
participant ThumbnailState as ThumbnailFetchState
participant Scheduler as OneMoreTime_task
participant Backend as BackendAPI
User->>VideoControlsDialog: Open controls dialog
VideoControlsDialog->>VideoThumbnail: render with register=true disabled=false
User->>VideoThumbnail: Click single-shot button
VideoThumbnail->>VideoThumbnail: fetchSingleThumbnail()
VideoThumbnail->>VideoThumbnail: snapshot_in_progress = true
VideoThumbnail->>VideoStore: startGetThumbnailForDevice(source)
VideoStore->>ThumbnailState: sources.add(source)
alt task paused or stopped
VideoStore->>Scheduler: start or resume()
end
loop every 1s
Scheduler->>VideoStore: fetchThumbnails()
VideoStore->>ThumbnailState: check inProgress flag
alt not inProgress
VideoStore->>ThumbnailState: inProgress = true
VideoStore->>Backend: GET /thumbnail?source=source
Backend-->>VideoStore: image blob, status, latency
VideoStore->>VideoStore: revoke old blob URL
VideoStore->>VideoStore: set thumbnails[source] = {source,status,roundtripMs}
VideoStore->>ThumbnailState: busy.delete(source)
VideoStore->>ThumbnailState: inProgress = false
else inProgress
VideoStore-->>Scheduler: skip cycle
end
end
participant UpdateTask as VideoThumbnail_update_task
UpdateTask->>VideoThumbnail: updateThumbnail()
VideoThumbnail->>VideoStore: thumbnails.get(source)
VideoStore-->>VideoThumbnail: Thumbnail
VideoThumbnail->>VideoThumbnail: load Image(result.source)
VideoThumbnail->>VideoThumbnail: last_fetch_ms = result.roundtripMs
VideoThumbnail->>VideoThumbnail: thumbnail = result
alt snapshot_in_progress
VideoThumbnail->>VideoThumbnail: snapshot_in_progress = false
VideoThumbnail->>VideoThumbnail: snapshot_cooldown = true
Note over VideoThumbnail: internal timeout 1s to clear cooldown
VideoThumbnail->>VideoStore: stopGetThumbnailForDevice(source)
VideoStore->>ThumbnailState: sources.delete(source)
alt sources.size == 0
VideoStore->>Scheduler: stop()
end
end
Sequence diagram for debounced unregister of continuous thumbnailssequenceDiagram
participant Parent as ParentComponent
participant VideoThumbnail
participant VideoStore
participant ThumbnailState as ThumbnailFetchState
participant Scheduler as OneMoreTime_task
rect rgb(235, 245, 255)
Parent->>VideoThumbnail: mount with register=true
alt continuous_mode initially true
VideoThumbnail->>VideoStore: startGetThumbnailForDevice(source)
VideoStore->>ThumbnailState: sources.add(source)
VideoStore->>Scheduler: start or resume()
end
end
rect rgb(255, 245, 235)
Parent->>VideoThumbnail: set register=false
VideoThumbnail->>VideoThumbnail: watcher register(false,true)
VideoThumbnail->>VideoThumbnail: clearTimeout(stopDebounceTimer)
VideoThumbnail->>VideoThumbnail: stopDebounceTimer = setTimeout(15s)
end
Note over VideoThumbnail: during debounce window
alt re-register before timeout
Parent->>VideoThumbnail: set register=true
VideoThumbnail->>VideoThumbnail: watcher register(true,false)
VideoThumbnail->>VideoThumbnail: clearTimeout(stopDebounceTimer)
VideoThumbnail->>VideoThumbnail: stopDebounceTimer = undefined
alt continuous_mode
VideoThumbnail->>VideoStore: startGetThumbnailForDevice(source)
end
else timeout fires
VideoThumbnail->>VideoStore: stopGetThumbnailForDevice(source)
VideoThumbnail->>VideoThumbnail: continuous_mode = false
VideoStore->>ThumbnailState: sources.delete(source)
alt sources empty
VideoStore->>Scheduler: stop()
end
end
rect rgb(245, 245, 245)
VideoThumbnail->>VideoThumbnail: beforeDestroy()
VideoThumbnail->>VideoThumbnail: clearTimeout(stopDebounceTimer)
VideoThumbnail->>VideoStore: thumbnails.get(source)
VideoStore-->>VideoThumbnail: Thumbnail
alt blob URL exists
VideoThumbnail->>VideoThumbnail: URL.revokeObjectURL(blobUrl)
end
VideoThumbnail->>VideoStore: stopGetThumbnailForDevice(source)
end
Class diagram for updated video thumbnail handling and pollingclassDiagram
class VideoThumbnail {
+String source
+String width
+Boolean register
+Boolean disabled
-- state --
-Thumbnail thumbnail
-OneMoreTime update_task
-Number stopDebounceTimer
-Boolean continuous_mode
-Boolean snapshot_in_progress
-Boolean snapshot_cooldown
-Number last_fetch_ms
-- computed --
+Boolean not_available
+Boolean fetching
+Boolean idle_placeholder
+Boolean is_pirate_mode
-- methods --
+updateThumbnail()
+fetchSingleThumbnail()
+toggleContinuous()
}
class Thumbnail {
+String source
+Number status
+Number roundtripMs
}
class ThumbnailFetchState {
+OneMoreTime task
+Set~String~ sources
+Set~String~ busy
+Boolean inProgress
}
class VideoStore {
+Map~String,Thumbnail~ thumbnails
+Boolean updating_streams
-- actions --
+fetchThumbnails()
+startGetThumbnailForDevice(source)
+stopGetThumbnailForDevice(source)
-- mutations --
+setUpdatingStreams(updating)
}
class VideoControlsDialog {
+Boolean thumbnailRegister
+Boolean thumbnailDisabled
}
class VideoStreamCreationDialog {
+Boolean show
+String stream_name
+String selected_encode
+Object selected_size
+Number selected_interval
+Array~String~ stream_endpoints
+Boolean is_thermal
+Boolean is_disable_lazy
+Boolean is_disable_mavlink
+Boolean is_disable_thumbnails
+Boolean is_disable_zenoh
+resetFormFromStream()
}
class Settings {
+Boolean is_pirate_mode
}
class BackAxios {
+get(url,config)
}
%% Relationships
VideoThumbnail --> VideoStore : uses video
VideoThumbnail --> Thumbnail : displays
VideoStore --> Thumbnail : stores
VideoStore --> ThumbnailFetchState : uses singleton
ThumbnailFetchState --> OneMoreTime : schedules
VideoThumbnail --> Settings : reads
VideoControlsDialog --> VideoThumbnail : renders
VideoStreamCreationDialog --> VideoStore : configures streams
VideoStore --> BackAxios : fetchThumbnails
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
…xtended config booleans and fix form state reset Add disable_lazy, disable_thumbnails, and disable_zenoh checkboxes to the extra configuration panel. Fix stale form state on dialog reopen by re-syncing all fields from the stream prop via a watcher.
joaoantoniocardoso
force-pushed
the
1.4-rework-thumbnails
branch
from
29cb767 to
8853715
Compare
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- In
VideoThumbnail.updateThumbnail, failures to load the image or non-200 thumbnail responses never clearsnapshot_in_progressor updatelast_fetch_ms, which can leave the UI stuck in a fetching state; consider handlingimg.onerrorand non-200 statuses to reset the snapshot flags and surface the latest status/latency. - The HMR-safe thumbnail polling singleton relies on
OneMoreTimeinternals (isPaused,isRunning,timeoutId) viaas any; it would be more robust to rely only on public APIs or track the task state in your ownthumbnailStateflags to avoid breakage ifOneMoreTimechanges.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `VideoThumbnail.updateThumbnail`, failures to load the image or non-200 thumbnail responses never clear `snapshot_in_progress` or update `last_fetch_ms`, which can leave the UI stuck in a fetching state; consider handling `img.onerror` and non-200 statuses to reset the snapshot flags and surface the latest status/latency.
- The HMR-safe thumbnail polling singleton relies on `OneMoreTime` internals (`isPaused`, `isRunning`, `timeoutId`) via `as any`; it would be more robust to rely only on public APIs or track the task state in your own `thumbnailState` flags to avoid breakage if `OneMoreTime` changes.
## Individual Comments
### Comment 1
<location path="core/frontend/src/components/video-manager/VideoThumbnail.vue" line_range="186-187" />
<code_context>
},
beforeDestroy() {
+ clearTimeout(this.stopDebounceTimer)
+ const blobUrl = video.thumbnails.get(this.source)?.source
+ if (blobUrl !== undefined) {
+ URL.revokeObjectURL(blobUrl)
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** Revoking the shared blob URL in the component can break other thumbnails using the same source.
Because the store shares `video.thumbnails` entries across `VideoThumbnail` instances for the same `source`, revoking the URL in `beforeDestroy` will invalidate it for any other component still using it. Since `fetchThumbnails` already revokes old URLs when replacing them, this per-component `URL.revokeObjectURL` should likely be removed and the store should own blob URL lifecycle centrally.
</issue_to_address>
### Comment 2
<location path="core/frontend/src/components/video-manager/VideoThumbnail.vue" line_range="3-12" />
<code_context>
:src="thumbnail?.source"
+ style="width: 100%; height: 100%; object-fit: cover;"
+ >
+ <div
+ v-if="is_pirate_mode && register"
+ class="thumbnail-overlay text-caption"
+ >
+ <template v-if="continuous_mode">
+ LIVE
+ <template v-if="last_fetch_ms !== null">
+ {{ last_fetch_ms }}ms
+ </template>
+ </template>
+ <template v-else-if="snapshot_in_progress">
+ fetching...
+ </template>
+ <template v-else-if="thumbnail">
+ <template v-if="last_fetch_ms !== null">
+ {{ last_fetch_ms }}ms
+ </template>
+ </template>
+ <template v-else>
+ idle
+ </template>
+ </div>
+ <div
+ v-if="register"
+ class="thumbnail-controls d-flex align-center"
>
</code_context>
<issue_to_address>
**issue (bug_risk):** Thumbnail controls are still active when thumbnails are disabled.
When `disabled` is true, the placeholder shows correctly, but the controls still render whenever `register` is true and still invoke `fetchSingleThumbnail` / `toggleContinuous`. This lets users trigger thumbnail fetching despite the disabled state. Please gate the controls on `!disabled` (e.g. `v-if="register && !disabled"`) or add a runtime guard in the click handlers so that `disabled` fully prevents thumbnail fetching.
</issue_to_address>
### Comment 3
<location path="core/frontend/src/store/video.ts" line_range="270-271" />
<code_context>
- } else {
- this.fetchThumbnailsTask.start()
+ thumbnailState.sources.add(source)
+ const task = thumbnailState.task as any
+ if (task.isPaused) {
+ thumbnailState.task.resume()
+ } else if (!task.isRunning && !task.timeoutId) {
</code_context>
<issue_to_address>
**suggestion:** Accessing internal `OneMoreTime` fields makes thumbnail scheduling brittle.
This relies on non-public `OneMoreTime` fields (`isPaused`, `isRunning`, `timeoutId`) via `as any`, so it will break if `OneMoreTime`’s internals change. Prefer using only the public API—for example, track running/paused state in `thumbnailState`, or extend `OneMoreTime` with proper accessors—instead of reaching into internal fields.
Suggested implementation:
```typescript
// eslint-disable-next-line class-methods-use-this
@Action
startGetThumbnailForDevice(source: string): void {
thumbnailState.sources.add(source)
const { task } = thumbnailState
if (thumbnailState.isPaused) {
task.resume()
thumbnailState.isPaused = false
thumbnailState.isRunning = true
} else if (!thumbnailState.isRunning) {
task.start()
thumbnailState.isRunning = true
}
}
```
To fully remove the dependency on internal `OneMoreTime` fields and make this robust:
1. Extend `thumbnailState` to track the task state explicitly, for example:
- Initialize `isRunning: boolean` and `isPaused: boolean` in the state definition for thumbnails.
2. Wherever `thumbnailState.task.start()`, `thumbnailState.task.pause()`, `thumbnailState.task.resume()`, or `thumbnailState.task.stop()` are called elsewhere in `video.ts` (or related modules), update `thumbnailState.isRunning` / `thumbnailState.isPaused` accordingly, e.g.:
- After `task.start()`: `isRunning = true`, `isPaused = false`
- After `task.pause()`: `isRunning = false`, `isPaused = true`
- After `task.resume()`: `isRunning = true`, `isPaused = false`
- After `task.stop()`: `isRunning = false`, `isPaused = false`
3. Ensure `thumbnailState.isRunning` and `thumbnailState.isPaused` are correctly initialized when the store/module is created (e.g. both `false`).
4. Remove any remaining `as any` casts and direct accesses to `OneMoreTime` internals (`isPaused`, `isRunning`, `timeoutId`) elsewhere in the codebase, replacing them with checks against the new `thumbnailState` flags.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+186
to
+187
| const blobUrl = video.thumbnails.get(this.source)?.source | ||
| if (blobUrl !== undefined) { |
There was a problem hiding this comment.
issue (bug_risk): Revoking the shared blob URL in the component can break other thumbnails using the same source.
Because the store shares video.thumbnails entries across VideoThumbnail instances for the same source, revoking the URL in beforeDestroy will invalidate it for any other component still using it. Since fetchThumbnails already revokes old URLs when replacing them, this per-component URL.revokeObjectURL should likely be removed and the store should own blob URL lifecycle centrally.
Comment on lines
+3
to
+12
| <div | ||
| class="thumbnail-frame d-flex align-center justify-center" | ||
| :style="{ width: width + 'px', aspectRatio: '16 / 9' }" | ||
| > | ||
| <span | ||
| v-if="not_available" | ||
| class="text-caption" | ||
| style="border: 2px dashed; opacity: 30%; padding: 20px;" | ||
| v-if="disabled" | ||
| class="text-caption text-center placeholder-box" | ||
| > | ||
| Preview not<br> | ||
| available<br> | ||
| Add a stream<br> | ||
| to have one | ||
| Thumbnails disabled<br> | ||
| for this source. |
There was a problem hiding this comment.
issue (bug_risk): Thumbnail controls are still active when thumbnails are disabled.
When disabled is true, the placeholder shows correctly, but the controls still render whenever register is true and still invoke fetchSingleThumbnail / toggleContinuous. This lets users trigger thumbnail fetching despite the disabled state. Please gate the controls on !disabled (e.g. v-if="register && !disabled") or add a runtime guard in the click handlers so that disabled fully prevents thumbnail fetching.
Comment on lines
+270
to
+271
| const task = thumbnailState.task as any | ||
| if (task.isPaused) { |
There was a problem hiding this comment.
suggestion: Accessing internal OneMoreTime fields makes thumbnail scheduling brittle.
This relies on non-public OneMoreTime fields (isPaused, isRunning, timeoutId) via as any, so it will break if OneMoreTime’s internals change. Prefer using only the public API—for example, track running/paused state in thumbnailState, or extend OneMoreTime with proper accessors—instead of reaching into internal fields.
Suggested implementation:
// eslint-disable-next-line class-methods-use-this
@Action
startGetThumbnailForDevice(source: string): void {
thumbnailState.sources.add(source)
const { task } = thumbnailState
if (thumbnailState.isPaused) {
task.resume()
thumbnailState.isPaused = false
thumbnailState.isRunning = true
} else if (!thumbnailState.isRunning) {
task.start()
thumbnailState.isRunning = true
}
}To fully remove the dependency on internal OneMoreTime fields and make this robust:
- Extend
thumbnailStateto track the task state explicitly, for example:- Initialize
isRunning: booleanandisPaused: booleanin the state definition for thumbnails.
- Initialize
- Wherever
thumbnailState.task.start(),thumbnailState.task.pause(),thumbnailState.task.resume(), orthumbnailState.task.stop()are called elsewhere invideo.ts(or related modules), updatethumbnailState.isRunning/thumbnailState.isPausedaccordingly, e.g.:- After
task.start():isRunning = true,isPaused = false - After
task.pause():isRunning = false,isPaused = true - After
task.resume():isRunning = true,isPaused = false - After
task.stop():isRunning = false,isPaused = false
- After
- Ensure
thumbnailState.isRunningandthumbnailState.isPausedare correctly initialized when the store/module is created (e.g. bothfalse). - Remove any remaining
as anycasts and direct accesses toOneMoreTimeinternals (isPaused,isRunning,timeoutId) elsewhere in the codebase, replacing them with checks against the newthumbnailStateflags.
patrickelectric
approved these changes
Mar 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cherry-picks to 1.4:
Summary by Sourcery
Rework video thumbnail fetching and controls, improve thumbnail polling state management, and extend video stream configuration options.
New Features:
Bug Fixes:
Enhancements: