Pin FFmpeg version to 7.0 #286
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe changes in this pull request involve updates to the Changes
Sequence Diagram(s)sequenceDiagram
participant Tester
participant MediaHelper
Tester->>MediaHelper: Run convertShortAndLowFpsVideo
MediaHelper->>Tester: Return duration (900L)
Tester->>MediaHelper: Run convertVerticalWebmVideo
MediaHelper->>Tester: Return duration (1_970L)
Tester->>MediaHelper: Run convertGifVideo
MediaHelper->>Tester: Return duration (900L)
Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🔇 Additional comments (2)
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🧹 Outside diff range and nitpick comments (1)
src/test/java/com/github/stickerifier/stickerify/media/MediaHelperTest.java (1)
207-207: Precise adjustment to expected video duration is appropriate.The change from 2000L to 1970L milliseconds for the expected duration in the
convertVerticalWebmVideotest is a good adjustment to align with Ffmpeg 7.1's output. This precise tuning demonstrates attention to detail and should help prevent false negatives in the test.Consider adding a comment explaining why this specific duration (1970L) was chosen. This could help future maintainers understand the reasoning behind this precise value. For example:
// 1970L: Precise duration after conversion with Ffmpeg 7.1 assertVideoConsistency(result, 288, 512, 30F, 1970L);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- src/test/java/com/github/stickerifier/stickerify/media/MediaHelperTest.java (2 hunks)
🔇 Additional comments (3)
src/test/java/com/github/stickerifier/stickerify/media/MediaHelperTest.java (3)
Line range hint
1-391: Summary of changesThe modifications in this file consistently adjust the expected durations in video conversion tests to accommodate the behaviour changes in Ffmpeg 7.1. These changes appear to comprehensively address the regression issue mentioned in the PR objectives.
Key points:
- Short video durations have been reduced from 1000L to 900L milliseconds.
- A precise adjustment was made for the vertical video test (2000L to 1970L milliseconds).
- The changes are consistent across different video types (WebM, GIF).
These adjustments should resolve the failing tests caused by the Ffmpeg 7.1 update. The approach taken seems systematic and well-considered.
191-191: Adjustment to expected video duration looks appropriate.The change from 1000L to 900L milliseconds for the expected duration in the
convertShortAndLowFpsVideotest appears to be a necessary adjustment to accommodate the behaviour of Ffmpeg 7.1. This modification aligns with the PR objective of fixing the regression.To ensure this change is consistent across the codebase, please run the following script:
This will help identify any other tests that might require similar adjustments.
215-215: Consistent adjustment to expected GIF video duration.The change from 1000L to 900L milliseconds for the expected duration in the
convertGifVideotest is consistent with the earlier adjustment in theconvertShortAndLowFpsVideotest. This consistency suggests a systematic change in Ffmpeg 7.1's behaviour for short video conversions.To ensure this change is applied consistently, please run the following script:
This will help verify that all relevant tests have been updated consistently and identify any tests that might have been overlooked.
✅ Verification successful
Consistent adjustment to expected GIF video duration verified.
All relevant
assertVideoConsistencymethod calls inMediaHelperTest.javahave been updated to use900LforexpectedDuration. No instances of1000Lremain.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistent duration changes across different video types. echo "Checking for 900L durations:" rg --type java '900L.*expectedDuration' src/test/java echo "\nChecking for remaining 1000L durations that might need updating:" rg --type java '1000L.*expectedDuration' src/test/javaLength of output: 322
Script:
#!/bin/bash # Description: Check for expectedDuration assignments in test files with flexible spacing. echo "Searching for 'expectedDuration' set to 900L:" rg --type java 'expectedDuration\s*=\s*900L' src/test/java echo "\nSearching for 'expectedDuration' set to 1000L:" rg --type java 'expectedDuration\s*=\s*1000L' src/test/java echo "\nListing all 'expectedDuration' assignments:" rg --type java 'expectedDuration\s*=\s*\d+L' src/test/javaLength of output: 510
Script:
#!/bin/bash # Description: List all 'expectedDuration' assignments and usages in test files. echo "Listing all 'expectedDuration' assignments and usages:" rg --type java 'expectedDuration' src/test/javaLength of output: 607
Script:
#!/bin/bash # Description: Find all calls to assertVideoConsistency and display their expectedDuration arguments. echo "Listing all 'assertVideoConsistency' method calls with their arguments:" rg --type java 'assertVideoConsistency\s*\(' src/test/java | grep -oP 'assertVideoConsistency\s*\([^)]*\)'Length of output: 773
MartelliEnrico
force-pushed
the
fix-ffmpeg-7.1-regression
branch
from
fb0d7c1 to
f3dff0a
Compare
With the latest version of Ffmpeg the duration of transcoded videos has changed, breaking some tests.