feat: Recording pipeline optimisations and fail-safes#1470
Merged
richiemcilroy merged 21 commits intomainfrom Dec 29, 2025
Merged
feat: Recording pipeline optimisations and fail-safes#1470richiemcilroy merged 21 commits intomainfrom
richiemcilroy merged 21 commits intomainfrom
Conversation
Comment on lines
+319
to
+322
|
|
||
| if let Err(e) = self.validate_init_segment() { | ||
| tracing::error!("{e}"); | ||
| } |
Contributor
There was a problem hiding this 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.
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.
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
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.
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