fix(screenshots): retry timestamp offsets on FFmpeg black frame detection

[Aerglonus]

• Prevents failed screenshot generations by retrying frame extraction with slight timestamp offsets when FFmpeg renders a solid black image.

Summary by CodeRabbit

• Bug Fixes
  • Improved screenshot capture reliability by retrying when a black frame is detected.
  • Skips invalid negative retry timestamps and stops promptly for other capture errors.
  • Preserves more useful underlying error details when screenshot capture ultimately fails.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 093bf480-50d4-46e4-9057-60f995d76969

📥 Commits

Reviewing files that changed from the base of the PR and between 8865f4b and 84958cb.

📒 Files selected for processing (1)

• internal/services/screenshots/ffmpeg_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• internal/services/screenshots/ffmpeg_test.go

---

📝 Walkthrough

Walkthrough

Screenshot capture retries black frames at configured timestamp offsets. It skips negative timestamps and stops on non-black errors. Successful captures preserve libplacebo status. Final errors wrap underlying runner errors when available.

Changes

Screenshot capture

Layer / File(s)	Summary
Retry black frames and preserve execution errors internal/services/screenshots/ffmpeg.go	captureFrame delegates single-frame work to captureFrameSingle, retries black-image results at timestamp offsets, skips negative timestamps, and preserves underlying runner errors.
Validate timestamp-based recovery internal/services/screenshots/ffmpeg_test.go	The timestamp-sensitive runner returns black or valid PNG output based on -ss. The test verifies retry timestamps and non-empty output. Error-writing behavior is also covered by the runner.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant captureFrame
  participant captureFrameSingle
  participant ffmpegRunner
  captureFrame->>captureFrameSingle: Capture frame at timestamp
  captureFrameSingle->>ffmpegRunner: Run frame command
  ffmpegRunner-->>captureFrameSingle: Return frame or execution error
  captureFrameSingle-->>captureFrame: Report black frame
  captureFrame->>captureFrameSingle: Retry with timestamp offset

Loading

Poem

Poem

A rabbit saw a black frame hop by,
So timestamps leaped across the sky.
The PNG brightened, crisp and clear,
While runner errors stayed sincere.
Hop, hop—capture success is here!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: retrying timestamp offsets when FFmpeg detects a black screenshot frame.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

internal/services/screenshots/ffmpeg_test.go (1)

154-159: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Format the composite literal.

Run make fmt-go before commit. It will apply the repository formatter to this keyed composite literal.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/services/screenshots/ffmpeg_test.go` around lines 154 - 159, Format
the color.RGBA composite literal assigned to validPayload in testPNGBytes using
the repository’s Go formatter, make fmt-go, so its keyed fields follow the
project’s standard indentation and spacing.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/services/screenshots/ffmpeg_test.go`:
- Around line 171-180: Update the test using timestampSensitiveRunner to record
each parsed -ss timestamp, then assert that the attempted timestamps are exactly
1.000 followed by 2.000. Keep the existing output-file and non-empty assertions,
and apply the same verification to the additional test block noted in the
comment.

---

Nitpick comments:
In `@internal/services/screenshots/ffmpeg_test.go`:
- Around line 154-159: Format the color.RGBA composite literal assigned to
validPayload in testPNGBytes using the repository’s Go formatter, make fmt-go,
so its keyed fields follow the project’s standard indentation and spacing.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 6223d58c-d32d-4af0-af32-55b6d7cf44f8

📥 Commits

Reviewing files that changed from the base of the PR and between 47bdb91 and 8865f4b.

📒 Files selected for processing (2)

• internal/services/screenshots/ffmpeg.go
• internal/services/screenshots/ffmpeg_test.go