-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat: Recording pipeline optimisations and fail-safes #1470
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.
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
10 files reviewed, 1 comment
|
|
||
| if let Err(e) = self.validate_init_segment() { | ||
| tracing::error!("{e}"); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: validation logs critical error but doesn't stop segment writing
If init.mp4 is missing/corrupt, subsequent segments will be unplayable. Consider returning Result from this function and handling the error in the caller to prevent creating unusable segments.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/enc-ffmpeg/src/mux/segmented_stream.rs
Line: 319:322
Comment:
**logic:** validation logs critical error but doesn't stop segment writing
If `init.mp4` is missing/corrupt, subsequent segments will be unplayable. Consider returning `Result` from this function and handling the error in the caller to prevent creating unusable segments.
How can I resolve this? If you propose a fix, please make it concise.
Lots of different optimisations to the recording pipeline, with added fail-safes.
Greptile Summary
This PR significantly improves the recording pipeline's resilience and recoverability through defensive error handling and enhanced metadata tracking.
Key improvements:
Duration::ZEROorDuration::MAX) instead of crashing.validate_init_segment()to detect missing/corruptinit.mp4files that would make segments unplayable, with critical logging.Technical notes:
saturating_suband checked arithmetic with explicit fallbacksConfidence Score: 4/5
crates/enc-ffmpeg/src/mux/segmented_stream.rs- validate_init_segment() error handling behaviorImportant Files Changed
Sequence Diagram
sequenceDiagram participant App as Recording App participant Pipeline as Output Pipeline participant PauseTracker as Pause Tracker participant Encoder as Segmented Encoder participant FS as File System participant Recovery as Recovery Manager Note over App,Recovery: Recording Start App->>Pipeline: Start recording with config Pipeline->>Encoder: Initialize encoder (base_path, video_config) Encoder->>FS: Create base directory Encoder->>FS: Write init.mp4 header Encoder->>Encoder: Store codec_info (width, height, fps, etc) Encoder->>FS: Write manifest.json (v5, codec_info included) Note over App,Recovery: Recording Frames loop For each frame App->>Pipeline: Process frame (timestamp) Pipeline->>PauseTracker: adjust(timestamp) alt Paused PauseTracker-->>Pipeline: None (skip frame) else Clock skew detected PauseTracker->>PauseTracker: Handle timestamp anomaly PauseTracker->>PauseTracker: Log warning, use fallback PauseTracker-->>Pipeline: adjusted_timestamp else Normal PauseTracker-->>Pipeline: adjusted_timestamp end alt Has adjusted timestamp Pipeline->>Encoder: queue_frame(frame, adjusted_timestamp) alt Encoder mutex poisoned Encoder->>Encoder: Detect poisoned mutex Encoder->>Encoder: Log warning Encoder-->>Pipeline: Skip frame gracefully else Normal encoding Encoder->>Encoder: Encode frame Encoder->>FS: Write to segment file end end end Note over App,Recovery: Segment Completion Encoder->>Encoder: Detect segment duration threshold Encoder->>FS: Finalize current segment Encoder->>Encoder: validate_init_segment() alt Init segment missing/corrupt Encoder->>Encoder: Log CRITICAL error end Encoder->>FS: Update manifest.json Encoder->>FS: Write in-progress manifest Note over App,Recovery: Recording Stop / Crash alt Normal stop App->>Pipeline: Stop recording Pipeline->>Encoder: finish_with_timestamp() Encoder->>FS: Finalize all segments Encoder->>FS: Write final manifest (is_complete: true) else Crash/Interrupt Note over Pipeline,FS: Incomplete segments left on disk end Note over App,Recovery: Recovery Flow Recovery->>FS: Scan for incomplete recordings Recovery->>FS: Load recording-meta.json alt Status requires recovery Recovery->>FS: Read manifest.json (v5 supported) Recovery->>Recovery: Parse codec_info from manifest Recovery->>FS: Validate init.mp4 exists Recovery->>FS: Check segment completeness Recovery->>Recovery: Build recoverable video Recovery->>FS: Update recording status end