Skip to content

Create snapshot functionality from History view #22639

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nrlcode wants to merge 4 commits into
base: dev
Choose a base branch
from
+89 −8
Open

Create snapshot functionality from History view #22639

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c5e816f
History: add snapshot support in recording player
nrlcode Mar 25, 2026
84c1836
Snapshot: simplify control wiring and scope cleanup
nrlcode Mar 25, 2026
9e56382
Snapshot: guard against repeated download clicks
nrlcode Mar 26, 2026
bef2ecd
Snapshot: move spam-click guard to controls layer
nrlcode Mar 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 37 additions & 4 deletions web/src/components/player/HlsVideoPlayer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ import { ASPECT_VERTICAL_LAYOUT, RecordingPlayerError } from "@/types/record";
import { useTranslation } from "react-i18next";
import ObjectTrackOverlay from "@/components/overlay/ObjectTrackOverlay";
import { useIsAdmin } from "@/hooks/use-is-admin";
import {
downloadSnapshot,
generateSnapshotFilename,
grabVideoSnapshot,
} from "@/utils/snapshotUtil";

// Android native hls does not seek correctly
const USE_NATIVE_HLS = false;
Expand Down Expand Up @@ -58,6 +63,7 @@ type HlsVideoPlayerProps = {
isDetailMode?: boolean;
camera?: string;
currentTimeOverride?: number;
supportsSnapshot?: boolean;
transformedOverlay?: ReactNode;
};

Expand All @@ -83,9 +89,10 @@ export default function HlsVideoPlayer({
isDetailMode = false,
camera,
currentTimeOverride,
supportsSnapshot = false,
transformedOverlay,
}: HlsVideoPlayerProps) {
const { t } = useTranslation("components/player");
const { t } = useTranslation(["components/player", "views/live"]);
const { data: config } = useSWR<FrigateConfig>("config");
const isAdmin = useIsAdmin();

Expand Down Expand Up @@ -264,13 +271,36 @@ export default function HlsVideoPlayer({
const getVideoTime = useCallback(() => {
const currentTime = videoRef.current?.currentTime;

if (!currentTime) {
if (currentTime == undefined) {
Copy link
Copy Markdown
Collaborator

@hawkeye217 hawkeye217 Mar 25, 2026

Choose a reason for hiding this comment

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

This change here seems unrelated to snapshots. Am I missing something?

Copy link
Copy Markdown
Author

@nrlcode nrlcode Mar 25, 2026

Choose a reason for hiding this comment

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

Agreed. That change was unrelated to snapshot scope. I reverted the logic back to its previous behavior so this PR only carries snapshot-specific changes.

return undefined;
}

return currentTime + inpointOffset;
}, [videoRef, inpointOffset]);

const handleSnapshot = useCallback(async () => {
Copy link
Copy Markdown
Collaborator

@hawkeye217 hawkeye217 Mar 26, 2026

Choose a reason for hiding this comment

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

We'll need some kind of loading guard that disables the button while the snapshot is being downloaded (like is done in LiveCameraView's snapshot handler), because a user could spam click the snapshot button, which in most cases will cause the browser to just download a bunch of black frames.

Copy link
Copy Markdown
Author

@nrlcode nrlcode Mar 26, 2026

Choose a reason for hiding this comment

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

Added in the guarding features!

const frameTime = getVideoTime();
const result = await grabVideoSnapshot(videoRef.current);

if (result.success) {
downloadSnapshot(
result.data.dataUrl,
generateSnapshotFilename(
camera ?? "recording",
currentTime ?? frameTime,
config?.ui?.timezone,
),
);
toast.success(t("snapshot.downloadStarted", { ns: "views/live" }), {
position: "top-center",
});
} else {
toast.error(t("snapshot.captureFailed", { ns: "views/live" }), {
position: "top-center",
});
}
}, [camera, config?.ui?.timezone, currentTime, getVideoTime, t, videoRef]);

return (
<TransformWrapper
minScale={1.0}
Expand All @@ -294,6 +324,7 @@ export default function HlsVideoPlayer({
seek: true,
playbackRate: true,
plusUpload: isAdmin && config?.plus?.enabled == true,
snapshot: supportsSnapshot,
fullscreen: supportsFullscreen,
}}
setControlsOpen={setControlsOpen}
Expand All @@ -320,7 +351,7 @@ export default function HlsVideoPlayer({
onUploadFrame={async () => {
const frameTime = getVideoTime();

if (frameTime && onUploadFrame) {
if (frameTime != undefined && onUploadFrame) {
Copy link
Copy Markdown
Collaborator

@hawkeye217 hawkeye217 Mar 25, 2026

Choose a reason for hiding this comment

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

This change also seems unrelated to snapshots.

Copy link
Copy Markdown
Author

@nrlcode nrlcode Mar 25, 2026

Choose a reason for hiding this comment

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

Reverted this to the previous check to keep scope tight and avoid unrelated behavioral changes in this snapshot PR.

const resp = await onUploadFrame(frameTime);

if (resp && resp.status == 200) {
Expand All @@ -334,6 +365,8 @@ export default function HlsVideoPlayer({
}
}
}}
onSnapshot={supportsSnapshot ? handleSnapshot : undefined}
snapshotTitle={t("snapshot.takeSnapshot", { ns: "views/live" })}
fullscreen={fullscreen}
toggleFullscreen={toggleFullscreen}
containerRef={containerRef}
Expand Down Expand Up @@ -465,7 +498,7 @@ export default function HlsVideoPlayer({

const frameTime = getVideoTime();

if (frameTime) {
if (frameTime != undefined) {
Copy link
Copy Markdown
Collaborator

@hawkeye217 hawkeye217 Mar 25, 2026

Choose a reason for hiding this comment

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

This change also seems unrelated to snapshots.

Copy link
Copy Markdown
Author

@nrlcode nrlcode Mar 25, 2026

Choose a reason for hiding this comment

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

Also reverted for the same reason

onTimeUpdate(frameTime);
}
}}
Expand Down
18 changes: 18 additions & 0 deletions web/src/components/player/VideoControls.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,14 @@ import {
import { cn } from "@/lib/utils";
import { FaCompress, FaExpand } from "react-icons/fa";
import { useTranslation } from "react-i18next";
import { TbCameraDown } from "react-icons/tb";

type VideoControls = {
volume?: boolean;
seek?: boolean;
playbackRate?: boolean;
plusUpload?: boolean;
snapshot?: boolean;
fullscreen?: boolean;
};

Expand All @@ -48,6 +50,7 @@ const CONTROLS_DEFAULT: VideoControls = {
seek: true,
playbackRate: true,
plusUpload: false,
snapshot: false,
fullscreen: false,
};
const PLAYBACK_RATE_DEFAULT = isSafari ? [0.5, 1, 2] : [0.5, 1, 2, 4, 8, 16];
Expand All @@ -71,6 +74,8 @@ type VideoControlsProps = {
onSeek: (diff: number) => void;
onSetPlaybackRate: (rate: number) => void;
onUploadFrame?: () => void;
onSnapshot?: () => void;
snapshotTitle?: string;
toggleFullscreen?: () => void;
containerRef?: React.MutableRefObject<HTMLDivElement | null>;
};
Expand All @@ -92,6 +97,8 @@ export default function VideoControls({
onSeek,
onSetPlaybackRate,
onUploadFrame,
onSnapshot,
snapshotTitle,
toggleFullscreen,
containerRef,
}: VideoControlsProps) {
Expand Down Expand Up @@ -292,6 +299,17 @@ export default function VideoControls({
fullscreen={fullscreen}
/>
)}
{features.snapshot && onSnapshot && (
<TbCameraDown
aria-label={snapshotTitle}
className="size-5 cursor-pointer"
title={snapshotTitle}
Copy link
Copy Markdown
Collaborator

@hawkeye217 hawkeye217 Mar 28, 2026

Choose a reason for hiding this comment

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

I'd remove this. None of the other buttons here have a title.

onClick={(e: React.MouseEvent<SVGElement>) => {
e.stopPropagation();
onSnapshot();
}}
/>
)}
{features.fullscreen && toggleFullscreen && (
<div className="cursor-pointer" onClick={toggleFullscreen}>
{fullscreen ? <FaCompress /> : <FaExpand />}
Expand Down
3 changes: 3 additions & 0 deletions web/src/components/player/dynamic/DynamicVideoPlayer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ type DynamicVideoPlayerProps = {
setFullResolution: React.Dispatch<React.SetStateAction<VideoResolutionType>>;
toggleFullscreen: () => void;
containerRef?: React.MutableRefObject<HTMLDivElement | null>;
supportsSnapshot?: boolean;
Copy link
Copy Markdown
Collaborator

@hawkeye217 hawkeye217 Mar 25, 2026

Choose a reason for hiding this comment

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

supportsSnapshot is a pure pass-through prop that adds noise here. Since it ends up as snapshot: supportsSnapshot in the features object inside HlsVideoPlayer, the cleaner approach is just to infer it from whether onSnapshot is provided - the same way plusUpload is already gated on onUploadFrame existing. That would eliminate this prop chain entirely.

I would just mirror what we already do with onUploadFrame.

Copy link
Copy Markdown
Author

@nrlcode nrlcode Mar 25, 2026

Choose a reason for hiding this comment

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

I removed the supportsSnapshot prop chain entirely and now infer snapshot support directly from onSnapshot presence in HlsVideoPlayer (same pattern as other callback-gated features like upload).

transformedOverlay?: ReactNode;
};
export default function DynamicVideoPlayer({
Expand All @@ -66,6 +67,7 @@ export default function DynamicVideoPlayer({
setFullResolution,
toggleFullscreen,
containerRef,
supportsSnapshot = false,
transformedOverlay,
}: DynamicVideoPlayerProps) {
const { t } = useTranslation(["components/player"]);
Expand Down Expand Up @@ -321,6 +323,7 @@ export default function DynamicVideoPlayer({
isDetailMode={isDetailMode}
camera={contextCamera || camera}
currentTimeOverride={currentTime}
supportsSnapshot={supportsSnapshot}
transformedOverlay={transformedOverlay}
/>
)}
Expand Down
34 changes: 26 additions & 8 deletions web/src/utils/snapshotUtil.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { baseUrl } from "@/api/baseUrl";
import { formatUnixTimestampToDateTime } from "@/utils/dateUtil";

type SnapshotResponse = {
dataUrl: string;
Expand Down Expand Up @@ -97,17 +98,34 @@ export function downloadSnapshot(dataUrl: string, filename: string): void {
}
}

export function generateSnapshotFilename(cameraName: string): string {
const timestamp = new Date().toISOString().replace(/[:.]/g, "-").slice(0, -5);
return `${cameraName}_snapshot_${timestamp}.jpg`;
export function generateSnapshotFilename(
cameraName: string,
timestampSeconds?: number,
timezone?: string,
): string {
const seconds = timestampSeconds ?? Date.now() / 1000;
const timestamp = formatUnixTimestampToDateTime(seconds, {
timezone,
date_format: "yyyy-MM-dd'T'HH-mm-ss",
});

const safeTimestamp =
timestamp === "Invalid time"
Copy link
Copy Markdown
Collaborator

@hawkeye217 hawkeye217 Mar 25, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

formatUnixTimestampToDateTime returns the string "Invalid time" on bad input, and this is meant for UI display. But you are pattern-matching on that magic string as error handling. If that return value ever changes, this silently breaks. I would just validate the input before calling the formatter instead, or don't use a formatter at all (since Live view's snapshot download has no formatting, either).

Copy link
Copy Markdown
Author

@nrlcode nrlcode Mar 25, 2026

Choose a reason for hiding this comment

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

Sounds good. I removed the check and now validate the input timestamp before calling the formatter (finite-number validation). If invalid/missing, it falls back to current time; if valid, it uses the playback timestamp. This avoids coupling to formatter UI text.

? new Date(seconds * 1000)
.toISOString()
.replace(/[:.]/g, "-")
.slice(0, -5)
: timestamp;
return `${cameraName}_snapshot_${safeTimestamp}.jpg`;
}

export async function grabVideoSnapshot(): Promise<SnapshotResult> {
export async function grabVideoSnapshot(
targetVideo?: HTMLVideoElement | null,
): Promise<SnapshotResult> {
try {
// Find the video element in the player
const videoElement = document.querySelector(
"#player-container video",
) as HTMLVideoElement;
const videoElement =
targetVideo ??
(document.querySelector("#player-container video") as HTMLVideoElement);

if (!videoElement) {
return {
Expand Down
1 change: 1 addition & 0 deletions web/src/views/recording/RecordingView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -839,6 +839,7 @@ export function RecordingView({
setFullResolution={setFullResolution}
toggleFullscreen={toggleFullscreen}
containerRef={mainLayoutRef}
supportsSnapshot={true}
/>
</div>
{isDesktop && effectiveCameras.length > 1 && (
Expand Down
Loading