Refactor video writer to use imageio instead of skvideo #1900
Conversation
WalkthroughThe recent changes enhance video handling in the application by replacing the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ExportClip
participant VideoWriter
User->>ExportClip: Initiate export
ExportClip->>VideoWriter: Check for ffmpeg availability
VideoWriter-->>ExportClip: Return availability status
alt ffmpeg available
ExportClip->>VideoWriter: Encode video with imageio
else ffmpeg not available
ExportClip->>User: Fallback to AVI using OpenCV
end
VideoWriter->>ExportClip: Confirm encoding completion
ExportClip-->>User: Export finished
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://github.com/avatars/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- sleap/gui/dialogs/export_clip.py (1 hunks)
- sleap/io/videowriter.py (4 hunks)
Additional context used
Ruff
sleap/io/videowriter.py
144-144:
imageio_ffmpegimported but unusedRemove unused import:
imageio_ffmpeg(F401)
Additional comments not posted (4)
sleap/gui/dialogs/export_clip.py (1)
14-23: LGTM! Transition toffmpegviaimageiois clear.The changes correctly update the logic and user messages to reflect the use of
ffmpegthroughimageio, aligning with the PR objectives.sleap/io/videowriter.py (3)
15-15: Importimageio.v2is necessary for new functionality.The import of
imageio.v2is required for the updated video writing functionality.
36-55: Robust transition toimageioforffmpegchecks.The
safe_builderandcan_use_ffmpegmethods correctly transition the dependency check fromskvideotoimageio, ensuring robust functionality.
Line range hint
120-173:VideoWriterImageioclass implementation is sound.The class effectively utilizes
imageiofor video writing, aligning with the refactor objectives. The methods are correctly implemented.Tools
Ruff
144-144:
imageio_ffmpegimported but unusedRemove unused import:
imageio_ffmpeg(F401)
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1900 +/- ##
===========================================
+ Coverage 73.30% 74.47% +1.16%
===========================================
Files 134 135 +1
Lines 24087 24791 +704
===========================================
+ Hits 17658 18462 +804
+ Misses 6429 6329 -100 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- environment.yml (1 hunks)
- environment_mac.yml (1 hunks)
- environment_no_cuda.yml (1 hunks)
- sleap/gui/commands.py (1 hunks)
- sleap/gui/dialogs/export_clip.py (1 hunks)
- sleap/io/videowriter.py (4 hunks)
- tests/io/test_videowriter.py (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- sleap/gui/dialogs/export_clip.py
Additional context used
Ruff
sleap/io/videowriter.py
100-100:
imageio_ffmpegimported but unusedRemove unused import:
imageio_ffmpeg(F401)
Additional comments not posted (11)
environment_mac.yml (1)
15-15: Addimageio-ffmpegdependency for video processing.The addition of
imageio-ffmpegis appropriate for enabling video processing withimageio. This aligns with the PR objectives to transition fromskvideotoimageio.environment_no_cuda.yml (1)
16-16: Addimageio-ffmpegdependency for video processing.The addition of
imageio-ffmpegis consistent with the changes in other environment files, supporting video processing withimageio.environment.yml (1)
15-15: Addimageio-ffmpegdependency for video processing.The addition of
imageio-ffmpegis necessary for video processing withimageio, aligning with the PR objectives to replaceskvideo.tests/io/test_videowriter.py (3)
Line range hint
5-19: Test functiontest_video_writeris well-structured.The function correctly tests the
VideoWriterby ensuring it writes a video file successfully.
Line range hint
22-36: Test functiontest_cv_video_writeris well-structured.The function correctly tests the
VideoWriterOpenCVby ensuring it writes a video file successfully.
44-67: Comprehensive test forVideoWriterImageio.The function thoroughly tests video writing and verifies attributes like height, width, fps, filename, crf, and preset.
sleap/io/videowriter.py (4)
36-37: Update tosafe_buildermethod is appropriate.The method now correctly uses
VideoWriterImageiowhenffmpegis available, aligning with the refactoring objectives.
Line range hint
76-120: Initialization logic forVideoWriterImageiois well-structured.The constructor initializes necessary attributes and sets up the
imageiowriter with appropriate parameters.Tools
Ruff
100-100:
imageio_ffmpegimported but unusedRemove unused import:
imageio_ffmpeg(F401)
126-129: Methodsadd_frameandcloseare correctly implemented.The
add_framemethod usesappend_data, and theclosemethod ensures proper closure of the writer.
42-55: Verifycan_use_ffmpegmethod functionality.The method attempts to import
imageio_ffmpegand check its version, which is a valid approach to determineffmpegavailability.Verification successful
can_use_ffmpegmethod is functioning correctly and integrated properly.The method is used to check for
ffmpegavailability and conditionally adjust functionality, such as user messages, video writer selection, and file naming. The codebase uses it appropriately to ensure smooth operations based onffmpegpresence.
sleap/gui/dialogs/export_clip.py: Determines the message about video encoding.sleap/io/videowriter.py: Chooses betweenVideoWriterImageioandVideoWriterOpenCV.sleap/gui/commands.py: Influences default output filename.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the `can_use_ffmpeg` method functionality by checking for `imageio_ffmpeg` usage. # Test: Search for usage of `can_use_ffmpeg` across the codebase. ast-grep --lang python --pattern $'VideoWriter.can_use_ffmpeg()'Length of output: 291
Script:
#!/bin/bash # Description: Gather context around the usage of `can_use_ffmpeg` in the identified files. # Get context around the usage in sleap/gui/dialogs/export_clip.py rg -A 5 -B 5 'VideoWriter.can_use_ffmpeg()' sleap/gui/dialogs/export_clip.py # Get context around the usage in sleap/io/videowriter.py rg -A 5 -B 5 'VideoWriter.can_use_ffmpeg()' sleap/io/videowriter.py # Get context around the usage in sleap/gui/commands.py rg -A 5 -B 5 'VideoWriter.can_use_ffmpeg()' sleap/gui/commands.pyLength of output: 1365
sleap/gui/commands.py (1)
1332-1333: Update toaskfunction logic is appropriate.The function now defaults to
.aviwhenffmpegis available, aligning with the transition fromskvideo.Also applies to: 1335-1335
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- sleap/io/videowriter.py (4 hunks)
Additional context used
Ruff
sleap/io/videowriter.py
100-100:
imageio_ffmpegimported but unusedRemove unused import:
imageio_ffmpeg(F401)
Additional comments not posted (6)
sleap/io/videowriter.py (6)
15-15: Import statement forimageio.v2is necessary.The addition of
imageio.v2is appropriate for replacingskvideowithimageio.
36-37: Changes tosafe_buildermethod are appropriate.The method now correctly checks for
ffmpegavailability and usesVideoWriterImageioif available.
42-55: Changes tocan_use_ffmpegmethod are correct.The method now checks for
imageio_ffmpegavailability, which is essential for usingimageio.
Line range hint
76-120:
Introduction ofVideoWriterImageioclass is well-implemented.The class effectively uses
imageioto handle video writing, replacing the deprecatedskvideo.Tools
Ruff
100-100:
imageio_ffmpegimported but unusedRemove unused import:
imageio_ffmpeg(F401)
125-126:add_framemethod implementation is correct.The method uses
append_datato add frames, which aligns withimageiousage.
129-129:closemethod implementation is appropriate.The method ensures the writer is properly closed, which is crucial for resource management.
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
sleap/io/videowriter.py (1)
36-38: Use logging instead of print.Replace
- print("USING IMAGEIO") + import logging + logging.info("USING IMAGEIO")
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- sleap/gui/commands.py (1 hunks)
- sleap/io/videowriter.py (4 hunks)
Additional context used
Ruff
sleap/io/videowriter.py
101-101:
imageio_ffmpegimported but unusedRemove unused import:
imageio_ffmpeg(F401)
Additional comments not posted (4)
sleap/io/videowriter.py (3)
15-15: Import Approved:imageio.v2.The import of
imageio.v2is necessary for the transition fromskvideotoimageio.
Line range hint
77-130: Implementation Approved:VideoWriterImageio.The
VideoWriterImageioclass is well-implemented, leveragingimageiofor video writing.Tools
Ruff
101-101:
imageio_ffmpegimported but unusedRemove unused import:
imageio_ffmpeg(F401)
43-56: Remove unused importimageio_ffmpeg.The import statement for
imageio_ffmpegis not used in the code and should be removed to clean up the imports.- import imageio_ffmpeg as ffmpegLikely invalid or redundant comment.
sleap/gui/commands.py (1)
1332-1335: Logic and Comment Change Approved.The logic for determining the default output filename is appropriate, and the comment change improves readability.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- sleap/gui/dialogs/export_clip.py (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- sleap/gui/dialogs/export_clip.py
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- tests/io/test_videowriter.py (2 hunks)
Additional context used
Ruff
tests/io/test_videowriter.py
2-2:
cv2imported but unusedRemove unused import:
cv2(F401)
Additional comments not posted (3)
tests/io/test_videowriter.py (3)
Line range hint
6-19:
LGTM!The test for
VideoWriteris well-structured and ensures functionality.Tools
Ruff
2-2:
cv2imported but unusedRemove unused import:
cv2(F401)
Line range hint
22-35:
LGTM!The test for
VideoWriterOpenCVis well-structured and ensures functionality.Tools
Ruff
2-2:
cv2imported but unusedRemove unused import:
cv2(F401)
45-68: LGTM!The test for
VideoWriterImageiois comprehensive and ensures both functionality and attribute verification.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- sleap/io/videowriter.py (4 hunks)
- tests/io/test_videowriter.py (2 hunks)
Additional context used
Ruff
sleap/io/videowriter.py
101-101:
imageio_ffmpegimported but unusedRemove unused import:
imageio_ffmpeg(F401)
Additional comments not posted (5)
tests/io/test_videowriter.py (2)
45-69: LGTM! Comprehensive test for AVI writing.The test function effectively verifies the functionality of
VideoWriterImageiofor AVI format, ensuring all critical attributes are checked.
71-101: LGTM! Effective test for handling odd-sized frames.The test function ensures that
VideoWriterImageiocorrectly processes frames with non-standard dimensions, maintaining expected attributes.sleap/io/videowriter.py (3)
Line range hint
77-119:
Ensure correct initialization and parameter usage.The
VideoWriterImageioclass is well-implemented, leveragingimageiofor video writing. Ensure that all parameters, such ascrfandpreset, are correctly utilized and documented.Tools
Ruff
101-101:
imageio_ffmpegimported but unusedRemove unused import:
imageio_ffmpeg(F401)
101-101: Remove unused import:imageio_ffmpeg.The import statement for
imageio_ffmpegis not used directly in the code and should be removed to clean up the imports.- import imageio_ffmpeg as ffmpegLikely invalid or redundant comment.
Tools
Ruff
101-101:
imageio_ffmpegimported but unusedRemove unused import:
imageio_ffmpeg(F401)
36-38: Ensureffmpegavailability check is robust.The
safe_buildermethod now usescan_use_ffmpegto decide on the writer implementation. Ensure thatffmpegis correctly detected in all environments.Verification successful
can_use_ffmpegfunction is robustly implemented.The
can_use_ffmpegfunction insleap/io/videowriter.pycorrectly handlesImportErrorand checks for theffmpegversion, ensuring reliable detection offfmpegavailability.
- Located in
sleap/io/videowriter.pyScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that `can_use_ffmpeg` correctly detects `ffmpeg` availability. # Test: Check for `ffmpeg` detection logic in the codebase. Expect: Correct handling of `ImportError` and version check. ast-grep --lang python --pattern $'def can_use_ffmpeg() { try { import imageio_ffmpeg as ffmpeg } catch ImportError { return False } try { $_ = ffmpeg.get_ffmpeg_version() if $_ { return True } } catch Exception { return False } return False }'Length of output: 964
Script:
#!/bin/bash # Description: Verify that `can_use_ffmpeg` correctly detects `ffmpeg` availability. # Corrected Test: Check for `ffmpeg` detection logic in the codebase. Expect: Correct handling of `ImportError` and version check. ast-grep --lang python --pattern $'def can_use_ffmpeg():\n try:\n import imageio_ffmpeg as ffmpeg\n except ImportError:\n return False\n try:\n version = ffmpeg.get_ffmpeg_version()\n if version:\n return True\n except Exception:\n return False\n return False\n'Length of output: 312
Script:
#!/bin/bash # Description: Search for the `can_use_ffmpeg` function in the codebase. # Use `rg` to search for the function definition across the repository. rg 'def can_use_ffmpeg' -A 10Length of output: 654
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- tests/io/test_videowriter.py (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- tests/io/test_videowriter.py
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (1)
sleap/io/videowriter.py (1)
102-102: Clarify the necessity of theimageio_ffmpegimport.The import statement for
imageio_ffmpegis flagged as unused, but it is necessary for checking theffmpegversion. Ensure this is documented to avoid confusion.# Import necessary for checking ffmpeg version import imageio_ffmpeg as ffmpegTools
Ruff
102-102:
imageio_ffmpegimported but unusedRemove unused import:
imageio_ffmpeg(F401)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- sleap/io/videowriter.py (4 hunks)
Additional context used
Ruff
sleap/io/videowriter.py
102-102:
imageio_ffmpegimported but unusedRemove unused import:
imageio_ffmpeg(F401)
Additional comments not posted (2)
sleap/io/videowriter.py (2)
129-129: LGTM! Ensure proper resource management inclose.The
closemethod correctly closes the writer. Ensure that this is called in all code paths to prevent resource leaks.
126-126: Ensure color conversion consistency inadd_frame.The
add_framemethod converts BGR to RGB if specified. Ensure that this behavior aligns with the expected input format forimageio.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- sleap/io/videowriter.py (4 hunks)
Additional context used
Ruff
sleap/io/videowriter.py
100-100:
imageio_ffmpegimported but unusedRemove unused import:
imageio_ffmpeg(F401)
Additional comments not posted (6)
sleap/io/videowriter.py (6)
36-37: LGTM! Safe builder logic is sound.The
safe_buildermethod correctly checks forffmpegavailability and defaults toVideoWriterOpenCVif not available.
42-55: Ensure robust exception handling incan_use_ffmpeg.The current implementation uses a broad
Exceptioncatch, which can obscure specific errors. Consider catching more specific exceptions to improve error handling.- except Exception: + except (ImportError, AttributeError):
93-118: Consider parameter validation in__init__.The constructor of
VideoWriterImageioinitializes several parameters. Adding validation for these parameters (e.g., non-negative dimensions, valid FPS) could prevent runtime errors.if height <= 0 or width <= 0: raise ValueError("Height and width must be positive integers.") if fps <= 0: raise ValueError("FPS must be a positive number.")Tools
Ruff
100-100:
imageio_ffmpegimported but unusedRemove unused import:
imageio_ffmpeg(F401)
123-124: LGTM! Frame addition logic is correct.The
add_framemethod correctly handles BGR to RGB conversion and appends frames usingimageio.
127-127: LGTM! Writer closure is correctly implemented.The
closemethod correctly closes theimageiowriter.
100-100: Retain necessary importimageio_ffmpeg.The import statement for
imageio_ffmpegis necessary for checkingffmpegavailability in thecan_use_ffmpegmethod.Tools
Ruff
100-100:
imageio_ffmpegimported but unusedRemove unused import:
imageio_ffmpeg(F401)
eberrigan
deleted the
elizabeth/Refactor-VideoWriter-to-use-imageio-instead-of-skvideo
branch
Description
skvideois no longer supported and results in a syntax error with newer versions of python (>3.7). We are updating all of our dependencies--see Update python versions and dependencies #1841.imageio.v2with an ffmpeg backend instead.VideoWriteris re-written and tested to useimageio.v2.VideoWriter.safe_builderchecks ifffmpegis available. It should be since now we installimageio-ffmpegin the environment.VideoWriterImageio. This test should always pass sinceimageio-ffmpegis now in the test environment.safe_buildermethod is tested. It should always useimageionow thatimageio-ffmpegis in the environment.was removed from
output_paramssinceimagio-ffmpegalready resizes : "WARNING:imageio_ffmpeg:IMAGEIO FFMPEG_WRITER WARNING: input image is not divisible by macro_block_size=16, resizing from (406, 720) to (416, 720) to ensure video compatibility with most codecs and players. To prevent resizing, make your input image divisible by the macro_block_size or set the macro_block_size to 1 (risking incompatibility)."imagio-ffmpegresizes automatically correctly for videos with odd number of pixels for height and width.imagio.v2full documentation is not easy to find.The writing parameters are here for convenience from "\miniforge\pkgs\imageio-2.34.2-pyh12aca89_0\site-packages\imageio\plugins"
"""
Parameters for writing
fps : scalar
The number of frames per second. Default 10.
codec : str
the video codec to use. Default 'libx264', which represents the
widely available mpeg4. Except when saving .wmv files, then the
defaults is 'msmpeg4' which is more commonly supported for windows
quality : float | None
Video output quality. Default is 5. Uses variable bit rate. Highest
quality is 10, lowest is 0. Set to None to prevent variable bitrate
flags to FFMPEG so you can manually specify them using output_params
instead. Specifying a fixed bitrate using 'bitrate' disables this
parameter.
bitrate : int | None
Set a constant bitrate for the video encoding. Default is None causing
'quality' parameter to be used instead. Better quality videos with
smaller file sizes will result from using the 'quality' variable
bitrate parameter rather than specifying a fixed bitrate with this
parameter.
pixelformat: str
The output video pixel format. Default is 'yuv420p' which most widely
supported by video players.
input_params : list
List additional arguments to ffmpeg for input file options (i.e. the
stream that imageio provides).
output_params : list
List additional arguments to ffmpeg for output file options.
(Can also be provided as
ffmpeg_paramsfor backwards compatibility)Example ffmpeg arguments to use only intra frames and set aspect ratio:
['-intra', '-aspect', '16:9']
ffmpeg_log_level: str
Sets ffmpeg output log level. Default is "warning".
Values can be "quiet", "panic", "fatal", "error", "warning", "info"
"verbose", or "debug". Also prints the FFMPEG command being used by
imageio if "info", "verbose", or "debug".
macro_block_size: int
Size constraint for video. Width and height, must be divisible by this
number. If not divisible by this number imageio will tell ffmpeg to
scale the image up to the next closest size
divisible by this number. Most codecs are compatible with a macroblock
size of 16 (default), some can go smaller (4, 8). To disable this
automatic feature set it to None or 1, however be warned many players
can't decode videos that are odd in size and some codecs will produce
poor results or fail. See https://en.wikipedia.org/wiki/Macroblock.
audio_path : str | None
Audio path of any audio that needs to be written. Defaults to nothing,
so no audio will be written. Please note, when writing shorter video
than the original, ffmpeg will not truncate the audio track; it
will maintain its original length and be longer than the video.
audio_codec : str | None
The audio codec to use. Defaults to nothing, but if an audio_path has
been provided ffmpeg will attempt to set a default codec.
Types of changes
Does this address any currently open issues?
[list open issues here]
Outside contributors checklist
Thank you for contributing to SLEAP!
❤️
Summary by CodeRabbit
Summary by CodeRabbit
New Features
imageiofor AVI file support.VideoWriterImageioclass to improve video writing functionality.Bug Fixes
ffmpeg, clarifying encoding options.Tests
VideoWriterImageioclass, ensuring robustness in video writing processes.