-
Notifications
You must be signed in to change notification settings - Fork 69
Implement Video Reader #356
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements a new VideoReader feature to enable video playback functionality after recording from a webcam. It also updates the example code, documentation, and the underlying stream module to support video reading and playback controls.
- Implements VideoReader with methods to start, grab, seek, set playback speed, and restart the video.
- Updates the example application and README to demonstrate recording and reading video.
- Adds pipeline accessors in the stream capture module to support new functionality.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
examples/video_read/src/main.rs | Adds video recording and subsequent playback using VideoReader. |
examples/video_read/README.md | Updates usage instructions (with a noted typo) for video reading. |
examples/video_read/Cargo.toml | Introduces necessary workspace dependencies for video reading. |
crates/kornia-io/src/stream/video.rs | Implements VideoReader along with several playback control methods. |
crates/kornia-io/src/stream/capture.rs | Adds getters for the pipeline used in the video stream capture. |
@AS1100K @edgarriba With all my respect and love, I think it is disrespectful for @AS1100K to work on the issue that I already made a pull request for, especially after he asked me if I am working on it, and I answered "yes, I am working on it." and I did finish it a week ago. Correct me if I am wrong, but I think it is against the rules set by the community here, or at least disrespectful. |
- Introduced `AppState` enum to manage video player states. - Simplified error handling and state transitions. - Modularized UI rendering into separate functions. - Added `TextureStore` for efficient texture management. - Improved image resizing logic for better performance.
@edgarriba I split the example into three functions to make it more easy to understand what's going on and have implemented |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Just minor things.
Some else I notice is that when you pause the video it goes black and you cannot see the frame where you paused nor being able to seek.
If that's too much work to do, we can skip to not overcomplicate things
|
Done 😀 |
|
||
// Video is now finished | ||
if current_pos == duration { | ||
is_finished = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why don't you jsut set the state to finished?
} | ||
} | ||
|
||
if is_finished { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check the app state
Fixes #257
Fixes #113
Example
I have created a simple video player example with egui
Demo
demo.mp4
TODOs