fix(video): recover playback after surface stalls - #15
Merged
programmersd21 merged 1 commit intoAug 11, 2026
Conversation
Reviewer's GuideThis PR changes video playback to treat compositor timeouts and recoverable surface errors as non-fatal, adding a small retry/reconfigure loop and extending FrameStatus and renderer surface error handling so playback can recover after stalls while still stopping on fatal errors. Sequence diagram for the new video playback recovery loopsequenceDiagram
participant play_video
participant Surface
participant LivePacer
participant VideoPlayback
play_video->>play_video: video_present_action(status)
alt Presented
play_video->>play_video: continue playback loop
else Retry
play_video->>LivePacer: wait_until(Instant + 100ms)
else Reconfigure
play_video->>Surface: configure(device, SurfaceConfiguration)
play_video->>LivePacer: wait_until(Instant + 100ms)
else [Err]
play_video->>VideoPlayback: stop()
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
📝 WalkthroughWalkthroughChangesFrame status recovery
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant VideoPlayback
participant render_frame
participant Surface
participant LivePacer
VideoPlayback->>render_frame: render video frame
render_frame-->>VideoPlayback: FrameStatus
alt TimedOut
VideoPlayback->>LivePacer: wait 100 ms
LivePacer-->>VideoPlayback: retry timing
else Outdated or Lost
VideoPlayback->>Surface: reconfigure surface
Surface-->>VideoPlayback: reconfigured surface
else Presented
VideoPlayback-->>VideoPlayback: continue playback
end
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@wallr-core/src/daemon/mod.rs`:
- Around line 1297-1308: Update the surface recovery configuration in play_video
to use the current surface dimensions rather than the stale commit.width and
commit.height values from CommitData. Reuse the live output dimensions
maintained by update_output, or restart playback when that output changes, while
preserving the existing format and recovery behavior.
In `@wallr-core/src/renderer/mod.rs`:
- Around line 426-432: Add #[non_exhaustive] to the public FrameStatus enum so
future variants do not break external exhaustive matches; otherwise mark this
API change as breaking before release.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 30031ded-9266-42fd-88ed-11fa5e0dbaa9
📒 Files selected for processing (2)
wallr-core/src/daemon/mod.rswallr-core/src/renderer/mod.rs
Comment on lines
+1297
to
+1308
| surface.configure( | ||
| &renderer.device, | ||
| &wgpu::SurfaceConfiguration { | ||
| usage: wgpu::TextureUsages::RENDER_ATTACHMENT, | ||
| format: commit.format, | ||
| width: commit.width, | ||
| height: commit.height, | ||
| present_mode: wgpu::PresentMode::Fifo, | ||
| alpha_mode: wgpu::CompositeAlphaMode::Opaque, | ||
| view_formats: vec![], | ||
| desired_maximum_frame_latency: 2, | ||
| }, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Map surface configuration and RenderState update paths.
ast-grep outline wallr-core/src/daemon/mod.rs --items all --type impl,function
rg -n -C8 '\.configure\s*\(|SurfaceConfiguration|self\.(format|width|height)\s*=|commit\.(format|width|height)' wallr-core/src
Repository: programmersd21/wallr
Length of output: 200
🏁 Script executed:
set -euo pipefail
printf '%s\n' '--- candidate files ---'
git ls-files 'wallr-core/src/*' | sed -n '1,120p'
printf '%s\n' '--- surface-related code ---'
rg -n -C10 '(\.configure\s*\(|SurfaceConfiguration|commit\.(format|width|height)|format\s*=|width\s*=|height\s*=|alpha_mode|present_mode)' wallr-core/src/daemon/mod.rs wallr-core/src 2>/dev/null | sed -n '1,320p'
Repository: programmersd21/wallr
Length of output: 23742
🏁 Script executed:
set -euo pipefail
printf '%s\n' '--- RenderState and CommitData definitions ---'
rg -n -C12 'struct (RenderState|CommitData)|enum VideoPresentAction|fn (play_video|render_video|video_present_action)' wallr-core/src/daemon/mod.rs
printf '%s\n' '--- relevant implementation sections ---'
sed -n '300,540p' wallr-core/src/daemon/mod.rs
sed -n '680,875p' wallr-core/src/daemon/mod.rs
sed -n '1180,1335p' wallr-core/src/daemon/mod.rs
Repository: programmersd21/wallr
Length of output: 28056
🏁 Script executed:
set -euo pipefail
printf '%s\n' '--- complete RenderState fields ---'
sed -n '635,675p' wallr-core/src/daemon/mod.rs
printf '%s\n' '--- render-state creation and format selection ---'
rg -n -C18 'create_render_state_for_output_sync|SurfaceCapabilities|formats|RenderState \{' wallr-core/src/daemon/mod.rs wallr-core/src/renderer/mod.rs
printf '%s\n' '--- frame status and surface acquisition ---'
rg -n -C20 'enum FrameStatus|fn render_frame|SurfaceError|surface\.get_current_texture|SurfaceConfiguration' wallr-core/src/renderer/mod.rs
Repository: programmersd21/wallr
Length of output: 34068
🏁 Script executed:
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
s = Path("wallr-core/src/daemon/mod.rs").read_text()
checks = {
"commit snapshots dimensions": "width: self.width,\n height: self.height," in s,
"play_video renders with snapshot dimensions": "width: commit.width,\n height: commit.height," in s,
"recovery config uses snapshot dimensions": "format: commit.format,\n width: commit.width,\n height: commit.height," in s,
"resize path updates live state dimensions": "lock.width = new_width;\n lock.height = new_height;" in s,
"resize path configures current dimensions": "width: new_width,\n height: new_height," in s,
"resize path does not bump playback generation": "lock.playback_gen" not in s[s.find("fn update_output"):s.find("fn output_destroyed")],
}
for name, result in checks.items():
print(f"{name}: {'YES' if result else 'NO'}")
if not all(checks.values()):
raise SystemExit("unexpected source shape")
PY
Repository: programmersd21/wallr
Length of output: 431
Use current surface dimensions during video recovery.
update_output can reconfigure the surface while play_video still uses the snapshot in CommitData. A resize can therefore make recovery restore stale dimensions and repeat Outdated or Lost errors. Use live dimensions or restart playback when the output changes.
🤖 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 `@wallr-core/src/daemon/mod.rs` around lines 1297 - 1308, Update the surface
recovery configuration in play_video to use the current surface dimensions
rather than the stale commit.width and commit.height values from CommitData.
Reuse the live output dimensions maintained by update_output, or restart
playback when that output changes, while preserving the existing format and
recovery behavior.
Comment on lines
+426
to
+432
| /// Whether a frame was presented or the surface needs recovery. | ||
| #[derive(Debug, Clone, Copy, PartialEq, Eq)] | ||
| pub enum FrameStatus { | ||
| Presented, | ||
| TimedOut, | ||
| Outdated, | ||
| Lost, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Inspect internal uses and public exports of FrameStatus.
rg -n -C3 '\bFrameStatus\b|\brender_frame\s*\(' wallr-core/src
# Inspect package versioning and release policy metadata.
fd -a '^Cargo\.toml$' . -E target -0 |
xargs -0 -r -n1 sh -c 'printf "\n--- %s ---\n" "$1"; sed -n "1,180p" "$1"' shRepository: programmersd21/wallr
Length of output: 17757
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- public module/export surface ---'
rg -n -C4 '^(pub\s+)?mod renderer|pub\s+use.*renderer|FrameStatus|non_exhaustive' wallr-core/src wallr-core/README.md README.md
printf '%s\n' '--- change introducing the enum variants ---'
git diff -- wallr-core/src/renderer/mod.rs wallr-core/src/lib.rs
printf '%s\n' '--- release and compatibility guidance ---'
rg -n -i -C3 'semver|breaking|release|api|public|library|version' README.md wallr-core/README.md .github 2>/dev/null || true
printf '%s\n' '--- package metadata ---'
sed -n '1,100p' wallr-core/Cargo.tomlRepository: programmersd21/wallr
Length of output: 16294
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
path = Path("wallr-core/src/renderer/mod.rs")
text = path.read_text()
match = re.search(r'pub enum FrameStatus\s*\{(.*?)\n\}', text, re.S)
if not match:
raise SystemExit("FrameStatus declaration not found")
body = match.group(1)
variants = re.findall(r'^\s*([A-Z][A-Za-z0-9_]*)\s*,', body, re.M)
print("FrameStatus variants:", variants)
print("has #[non_exhaustive]:", bool(re.search(r'#\[non_exhaustive\]\s*pub enum FrameStatus', text)))
PYRepository: programmersd21/wallr
Length of output: 255
Add #[non_exhaustive] or release this as a breaking API change.
wallr-core exposes renderer publicly. Adding variants to FrameStatus breaks external exhaustive matches.
🤖 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 `@wallr-core/src/renderer/mod.rs` around lines 426 - 432, Add #[non_exhaustive]
to the public FrameStatus enum so future variants do not break external
exhaustive matches; otherwise mark this API change as breaking before release.
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.
Summary
Root cause
During suspend,
get_current_texture()can time out while the compositor is not presenting. Wallr treated every non-presented frame as fatal and stopped the decoder, leaving the last frame frozen after resume.Validation
cargo fmt --all --checkcargo check --workspacecargo clippy --workspace --all-targets -- -D warningscargo test --workspace(47 passed)Summary by Sourcery
Improve video playback resilience to compositor surface stalls by recovering from non-fatal presentation failures instead of permanently stopping playback.
Bug Fixes:
Enhancements:
Tests:
Summary by CodeRabbit
Bug Fixes
Reliability