feat(libxmf): expose libvpx,add rust examples with it #55
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
irvingoujAtDevolution
requested review from
CBenoit,
awakecoding and
thenextman
irvingoujAtDevolution
changed the title
thenextman
reviewed
Nov 13, 2024
thenextman
reviewed
Nov 13, 2024
thenextman
reviewed
Nov 13, 2024
thenextman
reviewed
Nov 13, 2024
|
I thought the C and bindings looks good to me I didn't really look at the example, and I can't comment on how normative the rust files are. It's hard to build a mental model of the C/rust interface but I didn't see anything that looked obviously bad or unsafe. Good job! |
CBenoit
reviewed
Nov 14, 2024
Co-authored-by: Benoît Cortier <[email protected]>
Co-authored-by: Benoît Cortier <[email protected]>
… github.com:Devolutions/cadeau into ARC-251-devolutions-gateway-video-session-shadowing
CBenoit
reviewed
Nov 19, 2024
rust/cadeau/src/xmf/recorder.rs
Outdated
Comment on lines
142
to
145
| if !(surface_step * height <= buffer.len()) { | ||
| if surface_step * height > buffer.len() { | ||
| return Err(RecorderError::BadArgument { name: "buffer" }); | ||
| } | ||
| if !(width * height * 4 <= buffer.len()) { | ||
| if width * height * 4 > buffer.len() { |
There was a problem hiding this comment.
This got reverted again. What’s happening?
There was a problem hiding this comment.
Oh, I know why, it is because of clippy!
There was a problem hiding this comment.
Indeed! Can you annotate the function with this?
#[expect(clippy::nonminimal_bool)]There was a problem hiding this comment.
looks like expect is an experiment feature, can I use allow instead?
There was a problem hiding this comment.
Actually, it was stabilized recently. You can update the Rust toolchain instead 🙂
There was a problem hiding this comment.
I have to update rust-toolchain.toml
CBenoit
reviewed
Nov 19, 2024
CBenoit
reviewed
Nov 19, 2024
CBenoit
reviewed
Nov 19, 2024
CBenoit
reviewed
Nov 19, 2024
CBenoit
reviewed
Nov 19, 2024
|
Can you also try to run the examples with the LLVM memory sanitizers? You’ll need to build the xmf C library with something like this (at the beginning of the CMakeLists.txt file) target_compile_options(-fsanitize=address)
target_link_options(-fsanitize=address -static-libasan)And Rust nightly + Someone recently wrote a walkthrough blog post about exactly that: https://geo-ant.github.io/blog/2024/rust-address-sanitizer-with-c/ Maybe you can use that as a reference. First, try to introduce a memory issue in the C code and verify that the sanitizer catches it. Then, remove it and try to run the examples, and see if you get any problem. |
CBenoit
reviewed
Nov 19, 2024
CBenoit
reviewed
Nov 19, 2024
CBenoit
reviewed
Nov 19, 2024
CBenoit
reviewed
Nov 19, 2024
I really have problem doing this, I'll try valgrind in wsl instead, |
|
Here's the Valgrind report |
CBenoit
approved these changes
Nov 20, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request exposes libvpx encoder/decoder alone with Images,Frames and Packets to the caller.
Updated rust xmf-sys and wrapped rust cadeau with a safe abstraction.
Added examples to cut the video at a block level precision.
The file timed-dvls.webm is a un-remuxed file copied directly from Gateway recording, which can be used to test the rust
cutexample.