-
Notifications
You must be signed in to change notification settings - Fork 102
An initial pass at synchronizing with zmwangx's work #142
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
base: master
Are you sure you want to change the base?
Conversation
av_get_colorspace_name could return NULL. Also, switch from av_get_colorspace_name (frame.c) to av_color_space_name (pixdesc.c), which is more comprehensive.
pixdesc "helpfully" returns "unknown", which renders our Option pointless.
C enums allow duplicate variants, but Rust enums don't. At some point bindgen switched to generating duplicate variants as associated constants instead of module-level constants, so use SomeEnum::* broke. Here we switch to associated constants in the native API as well.
to a hopefully more stable one that doesn't require constant attention.
Currently, this crate will fail to compile if an older version of the ffmpeg libraries are installed on the machine. This change causes this crate to support FFMpeg 3.0 and newer by default. Newer API features can be enabled by setting the appropriate feature flag e.g. `ffmpeg_3_4`, although this will cause the crate using this crate to fail to compile on older versions of ffmpeg.
BREAKING CHANGE
refactor: address requested changes
(modified to avoid significant white-space changes that make it harder to compare forks of the project)
Total interest in merging back changes, I thought I had done that already honestly, but I must have missed stuff. 🐼 |
Cool! Yeah I think at the moment the easiest way of getting a view in to what's unique in zhwangx's repo is via There are some additional examples which could be good to get as low hanging fruit, and also fairly big updates in Then the differences start getting a bit more detailed and probably need more careful review. One difference that I currently depend on in zhwangx's branch is the addition of send_packet, send_eof and receive_frame bindings that could be good to cherry pick (re: #141) |
I'll look into that, I'm in the process of starting my own company and this repo is going to become maintained by us (instead of just me), so hopefully things will start picking up soon. |
Sounds really good! best of luck getting started (am a solo founder here)! |
I was wondering what kinds of hurdles remain to get this synchronization effort back into place? |
Back in 2018 a fork of this repo was created by zmwangx at https://github.com/zmwangx/rust-ffmpeg where a non-trivial amount of further development was done while this repo was dormant for some time. Now more recently it looks like the tides are turning and zmwangx isn't actively developing that fork currently and there has been some recent development happening here.
Although it looks like https://github.com/kornelski is acting as a bridge between these two projects by contributing to both it seems like a shame that recent developments aren't building on top of the work that zmwangx has done.
In the last few days I've tried to conduct a review of the rust-ffmpeg-sys and rust-ffmpeg repos to first help me understand which one I should aim to use but also to understand the differences and ideally, hopefully, help find a way towards a canonical repo that incorporates all the work that's been done across both sets of repos.
rust-ffmpeg-sys status
First when it comes to the ffmpeg-sys repos I ended up making two branches where I could cherry pick in both directions until I could clearly see what the differences where. In that case it turns out that the latest repo maintained by meh now incorporates mostly everything that is zmwangx's repo.
Here are the two comparison branches I made for reference:
https://github.com/rib/rust-ffmpeg-sys/commits/rust-ffmpeg-sys-meh-zmwangx-sync (Based on this meh's ffmpeg-sys repo)
https://github.com/rib/rust-ffmpeg-sys-1/commits/rust-ffmpeg-sys-zmwangx-meh-sync (Based on zmwangx's fork)
The more notable things missing include:
rib/rust-ffmpeg-sys@621c8dc
rib/rust-ffmpeg-sys@c29a12a
rust-ffmpeg
When it comes to comparing the rust-ffmpeg repos things are a bit more complex.
A number of external patches have found their way into both projects (such as from https://github.com/kornelski); a few changes that are similar/equivalent have been made independently in each project; in general there's quite a bit that's currently only in zmwangx's version and notably there are significant formatting differences between the projects now that make it hard to compare them.
The point at which both repos diverge is commit 67b7f13
Overall there have been a lot more changes made to zmwangx's repo and not too many have been made to this repo since then and so it was possible to review each commit made here to figure out whether to rebase / cherry pick it or skip if it was already included in zmwangx's repo.
To help me do the comparisons one notable thing I did was use
git filter-branch
to simplify the formatting changes made in d6a8ed3, to avoid all the whitespace differences.By applying the same formatting rules to zmwangx's branch it was then a lot more practical to review what's different and use git blame to track down the details.
The branch at https://github.com/rib/rust-ffmpeg/tree/limited-reformat is the result of simplifying the formatting which I then compared with zmwangx's branch.
This branch that I'm submitting as a pull request for reference is basically a rebase of all the changes to this repo since 67b7f13, based on top of zmwangx's latest branch.
I feel there's still more to review for me to understand the differences between the repos but I wanted to share this rebase that I did and see if there might be some interest in trying to consolidate these projects? 🤞