fix(video): fail probe when decoder reports failure - #29
Conversation
Reviewer's GuideThe standalone video probe now reports PASS only when it decodes more than 30 frames and ends in a non-failed decoder state, using one state snapshot for output and evaluation; it also propagates probe failure through a nonzero exit status and adds regression tests for the decision boundaries. Flow diagram for video probe pass and exit statusflowchart TD
A[Start video_probe] --> B[Decode video and count frames]
B --> C["state = decoder.decoder_state()"]
C --> D[Print diagnostics using state]
D --> E{"probe_passed(count, state)"}
E -->|count > 30 and state != Failed| F[Print PASS]
F --> G[Return ExitCode SUCCESS]
E -->|Otherwise| H[Print FAIL]
H --> I[Return ExitCode FAILURE]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review. 📝 WalkthroughWalkthroughThe video probe now returns an exit code. Decoder initialization failure returns failure. Otherwise, the probe reports decoder state and returns success only when more than 30 frames were decoded and the state is not ChangesVideo probe result
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: 🔵 Low · up to A long video can pass the probe’s frame threshold before a later decoding error is reported, allowing a false PASS. The issue is confined to the diagnostic probe and would require decoder synchronization changes, so the PR is mergeable with bounded follow-up. Architecture SummaryArchitecture risk: 🟡 Medium · up to The change affects 1 system. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
Reliability and maintainability
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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}
2 participants
Summary
Small follow-up to #28 addressing the remaining CodeRabbit finding.
result: PASS.Verification
cargo fmt --all --checkcargo test -p wallr-core --example video_probecargo clippy -p wallr-core --example video_probe -- -D warningsgit diff --checkThis changes only the standalone diagnostic probe; wallpaper playback is unaffected.
Summary by Sourcery
Ensure video probe results require both sufficient decoded frames and a healthy decoder state, and propagate failures through the process exit status.
Bug Fixes:
Enhancements:
Tests:
Summary by CodeRabbit