Skip to content
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

Compile even if not all cases are handled #97

Closed
wants to merge 1 commit into from

Conversation

kornelski
Copy link
Collaborator

These enums are likely to get out of sync with ffmpeg_sys, so some flexibility may be useful.

@meh
Copy link
Owner

meh commented Oct 31, 2017

I don't know how I feel about this, I'd rather fix the missing cases when ffmpeg-sys upgrades, are any failing currently?

@retrry
Copy link
Contributor

retrry commented Oct 31, 2017

Maybe changing ffmpeg-sys dependency from ffmpeg-sys="3.3" to ffmpeg-sys="~3.3" would be enough? Then it wouldn't pull newer versions of ffmpeg-sys than 3.3.X. With current requirement it can pull every version <4.0.0

@kornelski
Copy link
Collaborator Author

kornelski commented Oct 31, 2017

Admittedly that's a bit lazy solution, but the Rust wrapper currently does fail to compile because ffmpeg-sys has expanded these enums.

I'm unable to find any version of ffmpeg-sys that compiles with the current master. I guess ffmpeg-sys is using bindgen, and in that case it's not possible go guarantee 1:1 match in these enums (if you add missing items to the enums, they will fail to compile with FFI from an older version of C headers).

Related #90

@kornelski
Copy link
Collaborator Author

Can you merge it, and perhaps remove the cases later if you find a solution to lock sys to a specific ffmpeg version?

@kornelski kornelski mentioned this pull request Nov 4, 2017
@ghost
Copy link

ghost commented Nov 26, 2017

Any progress on this?

@retrry
Copy link
Contributor

retrry commented Dec 19, 2017

Bindgen authors in 0.3 version changed how enums are generated (rust-lang/rust-bindgen#758). By default instead of enums bindgen now generated constants. rust-ffmpeg-sys overrides this here: https://github.com/meh/rust-ffmpeg-sys/blob/master/build.rs#L918 Maybe we should drop this and rewrite rust-ffmpeg to use constants, but that would loose compiler check for exhaustiveness.

@kornelski kornelski closed this Jan 22, 2019
tilpner pushed a commit to tilpner/rust-ffmpeg that referenced this pull request Apr 26, 2023
tilpner pushed a commit to tilpner/rust-ffmpeg that referenced this pull request Apr 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants