Fix save_video not creating video files - fixes issue 470#481
Conversation
Fixes #470 The issue was that cv2.VideoWriter.release() doesn't always properly flush to disk, especially on Windows, and there was no verification that the file was created successfully. Changes: - Try multiple codecs (avc1, mp4v, XVID) for better cross-platform compatibility - Verify VideoWriter is successfully opened before writing frames - Add file existence check after release to ensure video was saved - Add file size check to catch empty/corrupt videos - Provide clear error messages when video creation fails This ensures users get immediate feedback if video saving fails rather than silent failure with misleading success messages.
for more information, see https://pre-commit.ci
|
🚀 Deployed on https://69763de6105d64206084b082--opengeos.netlify.app |
There was a problem hiding this comment.
Pull request overview
Fixes the silent failure where save_video() reports success but no video file is written to disk by hardening images_to_video().
Changes:
- Add multi-codec fallback (
avc1,mp4v,XVID) when initializingcv2.VideoWriter. - Validate
VideoWriter.isOpened()before writing frames and raise a clear error when initialization fails. - Add post-save validation (file existence + non-zero size) to detect “successful” writes that produced no output.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if not video_writer or not video_writer.isOpened(): | ||
| raise RuntimeError( | ||
| f"Failed to initialize video writer for {output_video}. " | ||
| "Please ensure you have the necessary codecs installed." | ||
| ) |
There was a problem hiding this comment.
The RuntimeError on VideoWriter initialization failure currently attributes the problem to missing codecs, but VideoWriter.isOpened() can also fail when the output directory doesn’t exist or the path is not writable. Consider checking/creating the parent directory (or explicitly erroring on it) and including the list of attempted codecs + path in the error message so users can diagnose the real cause.
| # Verify the video file was created | ||
| if not os.path.exists(output_video): | ||
| raise RuntimeError( | ||
| f"Failed to create video file {output_video}. " | ||
| "The video writer completed but the file was not saved to disk." | ||
| ) | ||
|
|
||
| # Verify the file has content | ||
| if os.path.getsize(output_video) == 0: | ||
| raise RuntimeError( | ||
| f"Video file {output_video} was created but is empty. " | ||
| "This may indicate a codec problem." | ||
| ) |
There was a problem hiding this comment.
The new codec-fallback and post-save validations are a regression-prone bugfix but there’s no unit test coverage for images_to_video() in tests/test_common.py. Adding a small test that writes a few synthetic frames to a temp dir and asserts the output file exists and is non-empty (and ideally can be opened via cv2.VideoCapture) would help prevent reintroducing the silent failure from #470.
Summary
Fixes the issue where save_video displays success messages but doesn't actually create video files on disk.
Problem
The user reported that save_video outputs success messages like Video saved as out.mp4 and Saved video to out.mp4, but no files are created. This is a silent failure issue where cv2.VideoWriter.release does not always properly flush to disk, especially on Windows.
Solution
Changes Made
Modified images_to_video in samgeo/common.py to:
Testing
samgeo.SamGeo3Video().save_video()doesn't save the video to disk #470Closes
Fixes #470