Fixed the issue where recording could fail or audio & video could be out of sync in scenarios with bad frames/bad audio tracks#95
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves recording robustness when frames or audio packets are corrupted, aiming to keep outputs playable (even if degraded) and reduce A/V sync issues in both default and segment recording modes.
Changes:
- Adds cleanup for stale temporary audio artifacts and creates a temp-audio workspace for degraded recovery flows.
- Reworks default-mode finalization to compute timing from
frames_writtenand to rebuild audio from “kept” frame ranges when frames were skipped. - Adds video-only fallback outputs for both default-mode audio merge failures and segment-mode concatenation failures.
You can also share your feedback on Copilot code review. Take the survey.
| with open(concat_file, "w") as f: | ||
| for audio_file in audio_files: | ||
| # FFmpeg concat demuxer expects absolute paths | ||
| abs_path = os.path.abspath(audio_file) | ||
| f.write(f"file '{abs_path}'\n") | ||
| formatted_path = abs_path.replace("\\", "/") | ||
| f.write(f"file '{formatted_path}'\n") |
There was a problem hiding this comment.
The concat manifest writer doesn’t specify an encoding and doesn’t escape quote characters in paths. On Windows this can fail for non-ASCII paths (default code page) and any path containing a single-quote will break the file '...' syntax for FFmpeg’s concat demuxer. Write the manifest with encoding='utf-8' (and ideally newline='\n') and escape/quote paths using FFmpeg-compatible escaping rather than raw file '{path}'.
| except Exception as e: | ||
| print(f"[ERROR] Audio merge failed: {e}") | ||
| if self.temp_file and os.path.exists(self.temp_file): | ||
| print( | ||
| "[WARN] Falling back to video-only output for default-style recording." | ||
| ) | ||
| if not self._write_video_only_output( | ||
| self.temp_file, final_file_path | ||
| ): | ||
| self.main_window.display_messagebox_signal.emit( | ||
| "Recording Error", | ||
| f"Audio merge failed and video-only fallback also failed:\n{e}", | ||
| self.main_window, | ||
| ) |
There was a problem hiding this comment.
In default-style finalization, FileNotFoundError (FFmpeg missing) is currently handled by the generic except Exception and then retried in the video-only fallback, which will also fail and surface a misleading message (“Audio merge failed and video-only fallback also failed”). Consider catching FileNotFoundError explicitly here (as you already do in segment finalization) to show a clear “FFmpeg not found” error instead of implying an audio-specific failure.
Fixed the issue where recording could fail or audio and video could be out of sync in scenarios with bad frames/bad audio tracks, ensuring that both default and segment modes can output stably in a degraded manner and provide clear warnings.