Skip to content

feat(alerts): video player for detection images#224

Open
ThbltLmr wants to merge 12 commits into
mainfrom
feat/alerts-video-player
Open

feat(alerts): video player for detection images#224
ThbltLmr wants to merge 12 commits into
mainfrom
feat/alerts-video-player

Conversation

@ThbltLmr

@ThbltLmr ThbltLmr commented May 14, 2026

Copy link
Copy Markdown
Collaborator

Summary

Replace the static detection list with a video-player UI for browsing
detection frames, backed by a paginated fetch that lets the player mount
on the first page while later pages stream in.

  • Player UI: play/pause, step ±1 frame, playback speed (1×/2×/4×), scrub via slider. Auto-seeks once per sequence to the first confident detection.
  • Pagination: useAllDetections fans out parallel page requests via TanStack useQueries; UI mounts as soon as any page resolves and surfaces a localized error (not a stuck spinner) when later pages fail.
  • Preloader: sliding-window prefetch around the playhead (5 back / 20 forward) so scrubbing and playback don't flicker.

Component tree

AlertImages ─────────── owns sequence-level state (lastSeenAt poll,
│                       displayBbox toggle, download menu)
│
├── useAllDetections ── fan-out: N parallel useQueries, one per page;
│                       returns merged detections + loadedCount/totalCount
│                       + isLoading/isError
│
└── AlertImagesPlayer ─ video-style controls; renders the current frame
    │                   and drives currentIndex via autoplay / stepping
    │                   / slider
    │
    ├── useImagePreloader ── prefetches a sliding window of urls around
    │                        currentIndex; dedup + cache-key reset on
    │                        sequence change
    │
    └── DetectionImageWithBoundingBox ── renders the selected frame
                                         with bbox overlay

OcclusionMaskModal ──── separate consumer; reuses useAllDetections
                        gated by \`open ? detectionsCount : 0\` so the
                        page fan-out is skipped when closed

Demo

2026-05-14.16-59-30.mp4

@vercel

vercel Bot commented May 14, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
new-pyro-platform Ready Ready Preview, Comment Jun 28, 2026 9:41am

@ThbltLmr

Copy link
Copy Markdown
Collaborator Author

Video demo - Updated

Shows:

  • updated layout
  • tablet / small screen layout
  • network tab: loading of first batch of images on page load, then moving window of images, new batch of images is the user moves to a new timestamp
  • speed selection, arrows to move +/- 1 and +/- 10
2026-06-16.23-15-55.mp4

@ThbltLmr ThbltLmr marked this pull request as ready for review June 16, 2026 21:23
newLastSeenAt &&
isStrictlyAfter(lastSeenAt, newLastSeenAt)
) {
// LastSeenAt has changed since last time

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not keeping the comments

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally I'm not a fan of comments (I think they often mean the code is not clear enough 😅), so I tend to delete them when I see them
reverted in docs: restore comments!

</MenuItem>
))}
</Menu>
<Typography variant="body2" sx={{ whiteSpace: 'nowrap' }}>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest to remove it for now, because counting doesn't seem to be a relevant information for users (only for developpers !)

aria-label={t(isPlaying ? 'buttonPause' : 'buttonPlay')}
size="large"
sx={{ border: `1px solid ${theme.palette.grey[500]}` }}
<Stack

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to split this component into two component and isolate actions buttons and player

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in refactor(controls): move controls to a dedicated components

There is a lot of shared state (controls drive the state of the player and vice-versa), so it means drilling a lot of props. Maybe we can refactor with a context or compound component: I've tried it in refactor(alerts): restructure player as a compound component, it's more code but the component props are simpler. What do you think?

@ThbltLmr

Copy link
Copy Markdown
Collaborator Author

Video demo - Updated

Shows:

  • updated layout
  • tablet / small screen layout
  • network tab: loading of first batch of images on page load, then moving window of images, new batch of images is the user moves to a new timestamp
  • speed selection, arrows to move +/- 1 and +/- 10
2026-06-16.23-15-55.mp4

@ThbltLmr ThbltLmr closed this Jun 28, 2026
@ThbltLmr ThbltLmr reopened this Jun 28, 2026
Reshape the alert images player into a compound component backed by a
feature-scoped context, mirroring the Live/context convention
(Context + Provider + use* hook).

- AlertPlayerProvider owns all playback state (index, play/pause, speed,
  derived selection/marks, autoplay, keyboard shortcuts, preloading)
- AlertPlayer.Image and AlertPlayer.Controls read it via useAlertPlayer(),
  removing the prop drilling that ran through the old player
- The tree now names the domain: a part that owns the image/detection,
  distinct from the controls

NOTE: this is a proposal to gauge whether we want to adopt the compound
component + feature-scoped context pattern here. It is the first compound
component in the codebase, so it is up for discussion before we commit to
it more broadly. Behaviour is unchanged; build, lint and tests pass.
@ThbltLmr

Copy link
Copy Markdown
Collaborator Author

Screenshot with the crop available:
image

@ThbltLmr ThbltLmr requested a review from m-julio June 28, 2026 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants