Added max_keyframes parameter #3
Conversation
…ames generated (most relevant)
WalkthroughThe pull request introduces a new parameter Changes
Sequence DiagramsequenceDiagram
participant User
participant CLI
participant KeyFrameDetector
participant VideoCapture
User->>CLI: Provide video source and parameters
CLI->>KeyFrameDetector: Call keyframeDetection()
KeyFrameDetector->>VideoCapture: Open video
VideoCapture-->>KeyFrameDetector: Return frames
KeyFrameDetector->>KeyFrameDetector: Detect keyframes
KeyFrameDetector->>KeyFrameDetector: Limit keyframes to max_keyframes
KeyFrameDetector-->>CLI: Return keyframe indices
CLI-->>User: Display or process results
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 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: 1
🧹 Nitpick comments (1)
cli.py (1)
15-15: Makemax_keyframesa configurable CLI argument.
Currently, themax_keyframes=5value is hard-coded, which might limit usability. Consider adding an optional CLI parameter (e.g.,--max-keyframes) for flexibility, allowing end users to set a custom limit without modifying the script.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
KeyFrameDetector/key_frame_detector.py(2 hunks)cli.py(1 hunks)
🔇 Additional comments (7)
KeyFrameDetector/key_frame_detector.py (7)
9-9: Validatemax_keyframesparameter.
Consider adding a quick validation to ensure thatmax_keyframesis a positive integer. This helps avoid unintended results if a negative or zero value is passed.
19-21: Pythonic check for opened video capture.
Replacingif (cap.isOpened() == False)withif not cap.isOpened():makes the code cleaner and more idiomatic.
33-35: Graceful loop termination.
Usingif not ret: breakensures a clean stop when frames are unavailable, reducing risk of empty reads.
57-60: Selective keyframe ranking is well-implemented.
Limiting keyframes to the topmax_keyframesby highest magnitude ensures that only the most significant keyframes are retained. This effectively balances thoroughness with performance.
61-61: Optional metric plotting is a nice touch.
Conditionally plotting metrics whenplotMetricsis enabled helps keep the tool lightweight for general usage.
66-67: Clear logging format.
Using f-strings for logging improves readability. The message “keyframe {cnt} happened at {timeSpans[x]} sec.” clearly communicates results.
73-73: Incrementing the counter after logging.
Updatingcntafter writing each CSV row keeps keyframe numbering in sync with the actual loop. This is correct and intuitive.
| print(log_message) | ||
| with open(path2file, 'w') as csvFile: | ||
| writer = csv.writer(csvFile) | ||
| writer.writerows(log_message) | ||
| csvFile.close() | ||
| writer.writerow([log_message]) |
There was a problem hiding this comment.
Avoid overwriting CSV output.
Opening the CSV file in 'w' mode inside the loop overwrites previous entries, and only the last keyframe log is preserved. To retain all keyframe logs in a single CSV, open the file in 'a' (append) mode.
-with open(path2file, 'w') as csvFile:
+with open(path2file, 'a') as csvFile:
writer = csv.writer(csvFile)
writer.writerow([log_message])Committable suggestion skipped: line range outside the PR's diff.
max_keyframes limits the amout of keyframes if needed, it sorts by the biggest variation
Summary by CodeRabbit
New Features
Bug Fixes
Refactor