fix(gif): add gif editing commands #887
Conversation
WalkthroughThis PR replaces the asciinema-automation dev dependency with pillow, introduces a new gif CLI group with reduce and info commands, and adds three GIF utility functions (trim_gif, get_gif_info, reduce_gif_frames) to support GIF processing workflows. The existing CLI also gains automatic GIF optimization after generation. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as gif CLI Group
participant Utils as secator.utils
participant Pillow
User->>CLI: $ secator gif reduce input.gif
CLI->>Utils: reduce_gif_frames(input, output, max_frames)
Utils->>Pillow: Open GIF, inspect frames
Pillow-->>Utils: Frame data
Utils->>Pillow: Sample frames, accelerate durations
Pillow-->>Utils: Optimized GIF
Utils-->>CLI: Success/Failure
CLI-->>User: Report result
User->>CLI: $ secator gif info input.gif
CLI->>Utils: get_gif_info(input)
Utils->>Pillow: Open GIF, fetch metadata
Pillow-->>Utils: width, height, frame_count, pixels
Utils-->>CLI: GIF info dict
CLI-->>User: Display GIF properties
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
secator/utils.py (1)
1103-1188: Frame reduction logic is correct but complex.The implementation properly samples frames evenly and adjusts durations to accelerate playback. A few observations:
- Line 1125-1127: Good early return when already below
max_frames- Lines 1151-1153: Duplicate removal could theoretically reduce the frame count below
max_frames, but this is unlikely in practice and acceptable- Line 1171: The minimum 10ms duration ensures GIF compatibility
The static analysis warnings are acceptable for this utility function.
Optional: Extract frame sampling logic
Consider extracting the frame sampling logic (lines 1143-1153) into a helper function for better readability:
def _sample_frame_indices(total_frames, max_frames): """Sample frame indices evenly across the total range.""" step = total_frames / max_frames indices = [] for i in range(max_frames): idx = int(i * step) if idx >= total_frames: idx = total_frames - 1 indices.append(idx) # Remove duplicates while preserving order seen = set() return [x for x in indices if not (x in seen or seen.add(x))]secator/cli.py (3)
469-483: GIF optimization integration looks good.The automatic GIF optimization after generation is a nice UX improvement. The conditional success message (only shown if
trim_gifreturnsTrue) appropriately handles cases where optimization isn't needed or fails.Optional: Reduce code duplication
The GIF optimization logic is duplicated for both
.tap(lines 469-472) and.tape(lines 480-483) code paths. Consider extracting it:def _optimize_gif_if_possible(output_gif): """Optimize GIF by trimming long pauses.""" with console.status('Optimizing GIF...'): if trim_gif(output_gif, output_gif): console.print(Info(message=f'Optimized GIF: {output_gif}'))Then use:
console.print(Info(message=f'Generated GIF: {output_gif}')) _optimize_gif_if_possible(output_gif)
531-561: Well-implemented reduce command with proper validation.The command thoroughly validates inputs and provides clear feedback. Good use of default output path with
_reducedsuffix.Minor cleanup: Remove unnecessary noqa directive
Line 533 has an unused
noqadirective that can be removed:-@click.option('--output', '-o', type=str, default=None, help='Output GIF file path (default: input file with _reduced suffix)') # noqa: E501 +@click.option('--output', '-o', type=str, default=None, help='Output GIF file path (default: input file with _reduced suffix)')The line is within the acceptable length limit.
563-590: Info command is well-implemented with clear output formatting.The command provides helpful GIF metadata in a user-friendly format. Input validation is consistent with the
reducecommand.Optional: Fix ambiguous character in docstring
Line 568 uses the multiplication sign
×which could be replaced withxfor better compatibility:- Displays the width, height, frame count, and total pixels (width × height × frames) + Displays the width, height, frame count, and total pixels (width x height x frames)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
pyproject.tomlsecator/cli.pysecator/utils.py
🧰 Additional context used
🧬 Code graph analysis (2)
secator/cli.py (1)
secator/utils.py (4)
vhs_tap_to_tape(1190-1316)trim_gif(1003-1069)reduce_gif_frames(1103-1187)get_gif_info(1072-1100)
secator/utils.py (1)
secator/cli.py (1)
info(565-589)
🪛 Ruff (0.14.10)
secator/cli.py
533-533: Unused noqa directive (non-enabled: E501)
Remove unused noqa directive
(RUF100)
568-568: Docstring contains ambiguous × (MULTIPLICATION SIGN). Did you mean x (LATIN SMALL LETTER X)?
(RUF002)
568-568: Docstring contains ambiguous × (MULTIPLICATION SIGN). Did you mean x (LATIN SMALL LETTER X)?
(RUF002)
secator/utils.py
1066-1066: Consider moving this statement to an else block
(TRY300)
1067-1067: Do not catch blind exception: Exception
(BLE001)
1068-1068: Use explicit conversion flag
Replace with conversion flag
(RUF010)
1098-1098: Do not catch blind exception: Exception
(BLE001)
1099-1099: Use explicit conversion flag
Replace with conversion flag
(RUF010)
1184-1184: Consider moving this statement to an else block
(TRY300)
1185-1185: Do not catch blind exception: Exception
(BLE001)
1186-1186: Use explicit conversion flag
Replace with conversion flag
(RUF010)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: integration (3.11, ubuntu-latest)
🔇 Additional comments (6)
pyproject.toml (1)
67-67: LGTM!The dependency replacement from
asciinema-automationtopillowappropriately supports the new GIF manipulation utilities introduced in this PR.secator/utils.py (3)
1003-1070: Solid implementation for GIF pause trimming.The function correctly uses
ImageChops.differenceto detect static frames and caps long pauses. The logic for accumulating and capping static duration is sound, though complex.The static analysis warnings (TRY300, BLE001, RUF010) are acceptable for this utility function:
- The
return Falseat line 1066 is reachable when no frames are collected- Catching broad
Exceptionis appropriate for graceful degradation- The f-string formatting is minor and doesn't affect functionality
1072-1101: LGTM!Clean and straightforward implementation for retrieving GIF metadata. The defensive error handling (returning
Noneon errors) is appropriate for this utility function.
1226-1226: Good addition for cleaner GIF output.Disabling cursor blink in the VHS tape header produces cleaner, less distracting GIF recordings.
secator/cli.py (2)
34-34: LGTM!The new GIF utility imports are correctly added to support the new CLI commands.
522-528: LGTM!The
gifcommand group is correctly defined with an appropriate check for the dev addon requirement.
Summary by CodeRabbit
Release Notes
New Features
gifCLI group withreduceandinfocommands for GIF manipulation and metadata display.Chores
✏️ Tip: You can customize this high-level summary in your review settings.