Skip to content

perf(thumbnails): libav decode + first-frame default + count knob (#364)#406

Merged
matteius merged 8 commits intomainfrom
fix/thumbnails-cpu-364
Apr 23, 2026
Merged

perf(thumbnails): libav decode + first-frame default + count knob (#364)#406
matteius merged 8 commits intomainfrom
fix/thumbnails-cpu-364

Conversation

@matteius
Copy link
Copy Markdown
Contributor

Summary

Closes #364. Three changes attacking the CPU bottleneck reporters hit on slow hardware (Atom D245, Pi, etc.):

  • Libav in-process decode replaces fork+execvp(\"ffmpeg\") in thumbnail_thread.c. The ~900 ms process-launch overhead was dominating the pipeline on the reporter's CPU. We now open the file with avformat_open_input, seek BACKWARD to the nearest keyframe, decode one frame, scale to 320 px with sws_scale, and reuse the existing ffmpeg_encode_jpeg() helper.
  • Mount-time thumbnail is now index 0 with seek_seconds=0 — no -ss at all. Previously index 0 was "1s in to avoid black intros", which forced a seek on the card users see by default. The frontend grid's initial load switched from index 1 (middle) → index 0 to match. Combined with (1), this is the single-biggest win for initial grid render.
  • New thumbnails_per_recording setting (1 or 3, default 3). In 1-mode the backend rejects index>0 with 403 and the frontend skips hover preload + cycling entirely — no extra ffmpeg work ever happens. Surfaced via a dropdown in Settings → Storage, gated on the existing "enable grid view thumbnails" toggle.

Also removes the dead fork+execvp copy of generate_thumbnail() that had been sitting unused in api_handlers_recordings_thumbnail.c.

Test plan

  • Build passes on Linux (cmake --build .) — verified locally
  • Frontend build passes (npm run build in web/) — verified locally
  • test_config unit tests pass — verified locally (45/45)
  • Manual: load /recordings.html grid view, verify cards populate (thumbnails still render, now via libav)
  • Manual: hover a card in default 3-mode, verify cycling still works after initial frame loads
  • Manual: Settings → Storage → set "Thumbnails per Recording" to 1, reload recordings grid, verify only one thumbnail request per card and no hover cycling
  • Manual on slow CPU (Atom / Pi): confirm grid render time drops noticeably vs. previous fork+exec path

🤖 Generated with Claude Code

Three wins for the CPU bottleneck the reporter hit on an Atom D245:

1. Libav in-process decode replaces fork+execvp("ffmpeg"). On slow
   hardware the ~900 ms process-launch cost was dominating the
   thumbnail pipeline. thumbnail_thread.c now opens the file, seeks
   BACKWARD to the nearest keyframe, decodes one frame, scales to
   320 px with sws_scale, and reuses ffmpeg_encode_jpeg().

2. The mount-time thumbnail is now index 0 with seek_seconds=0 — no
   -ss at all. Previously index 0 was "1s in to avoid black intros",
   which added an unnecessary seek to the card users see by default.
   The frontend grid also switched its initial load from index 1
   (middle) → index 0.

3. New thumbnails_per_recording setting (1 or 3, default 3). In 1-mode
   the backend rejects index>0 with 403 and the frontend skips hover
   preload + cycling entirely — ideal for slow-CPU users who just want
   a card preview and don't care about scrubbing. Surfaced via a
   dropdown in the Storage settings tab, gated behind the existing
   "enable grid view thumbnails" toggle.

Also removes the dead fork+execvp copy of generate_thumbnail() that
sat unused in api_handlers_recordings_thumbnail.c.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR reduces recordings-grid thumbnail CPU overhead by moving thumbnail decoding in-process (libav), defaulting the initial grid thumbnail to the first frame (no seek), and adding a setting to limit thumbnails per recording to avoid generating hover frames on slow hardware.

Changes:

  • Replace fork/exec ffmpeg thumbnail generation with in-process libav decode + swscale + existing JPEG encoder helper.
  • Change mount-time thumbnail selection to index 0 (seek_seconds=0) and update grid behavior accordingly.
  • Add thumbnails_per_recording (1 or 3) across backend config + settings API + frontend settings UI, with backend enforcement (403 for index>0 when set to 1).

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
web/public/locales/en.json Adds i18n strings for the new “Thumbnails per Recording” setting.
web/js/components/preact/settings/StorageTab.jsx Adds a dropdown for thumbnailsPerRecording, disabled when thumbnails are disabled.
web/js/components/preact/recordings/RecordingsGrid.jsx Switches initial thumbnail to frame 0; gates hover preload/cycling based on thumbnailsPerRecording.
web/js/components/preact/SettingsView.jsx Adds thumbnailsPerRecording to settings state and JSON mapping.
web/js/components/preact/RecordingsView.jsx Fetches thumbnails_per_recording and passes it down to the grid.
src/web/thumbnail_thread.c Implements libav-based frame decode + scaling + JPEG output for thumbnail generation.
src/web/api_handlers_settings.c Exposes and persists thumbnails_per_recording via settings GET/POST.
src/web/api_handlers_recordings_thumbnail.c Enforces thumbnails_per_recording=1 by rejecting index>0; changes index 0 seek to 0s.
src/core/config.c Adds default/load/save support for thumbnails_per_recording in INI config.
include/core/config.h Adds thumbnails_per_recording to the global config struct.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/web/thumbnail_thread.c Outdated
Comment thread src/web/thumbnail_thread.c Outdated
matteius and others added 2 commits April 23, 2026 04:42
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/web/thumbnail_thread.c
Comment thread src/web/thumbnail_thread.c Outdated
Comment thread src/web/thumbnail_thread.c Outdated
Comment thread web/js/components/preact/recordings/RecordingsGrid.jsx Outdated
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +199 to 205
// Guard against degenerate frames (corrupt/unsupported streams can produce
// width==0 or height==0, which would cause a divide-by-zero below).
if (frame->width <= 0 || frame->height <= 0) {
log_warn("Thumbnail: decoded frame has invalid dimensions (%dx%d) for %s",
frame->width, frame->height, input_path);
goto done;
}
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

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

out_h is computed using frame->height * out_w / frame->width without validating decoded dimensions. If frame->width (or frame->height) is 0 (corrupt/unsupported stream), this will divide-by-zero and can crash the process. Add a guard after decode to ensure frame->width > 0 && frame->height > 0 (and bail out if not) before computing output dimensions.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@copilot apply changes based on this feedback

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The frame->width <= 0 || frame->height <= 0 guard was added in a prior commit (f09d3a4) and is already in place — the division at line 224 is protected. No further change needed here.

Comment on lines +100 to +111
const [rawIsHovering, setRawIsHovering] = useState(false);
const isHovering = hoverFramesEnabled && rawIsHovering;
const setIsHovering = useCallback((nextIsHovering) => {
setRawIsHovering((prevIsHovering) => {
const resolvedIsHovering =
typeof nextIsHovering === 'function'
? nextIsHovering(prevIsHovering)
: nextIsHovering;

return hoverFramesEnabled ? resolvedIsHovering : false;
});
}, [hoverFramesEnabled]);
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

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

setIsHovering is recreated when hoverFramesEnabled changes (dependency array includes it). Later in this component, handleMouseEnter/handleMouseLeave are memoized with empty dependency arrays but call setIsHovering, so they can capture a stale closure and keep enabling hover cycling even after switching to 1-thumb mode (or vice versa). Ensure those mouse handlers depend on setIsHovering (or use a ref/inline handlers) so behavior tracks the current thumbnailsPerRecording value.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@copilot apply changes based on this feedback

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Fixed in 3c81509. Both handleMouseEnter and handleMouseLeave now list setIsHovering in their dependency arrays, so they always close over the current function reference (which is recreated whenever hoverFramesEnabled changes) and correctly track the live thumbnailsPerRecording value.

@matteius matteius merged commit c092f28 into main Apr 23, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants