mp4

[aMarcireau]

After a somewhat lengthy process (it took 70 attempts), x264 finally compiles on all platforms!

Besides video generation, this PR also includes the addition of an optional terminal progress bar.

There is no cross-compilation at the moment (we compile macOS ARM wheels on an ARM runner, macOS intel wheels on an intel runner, and Windows and Linux wheels on intel runners). This means that we do not create wheels for Windows ARM or Linux ARM (for instance the Raspberry Pi) at the moment. There are two ways to address this:

• Cross-compile wheels (potentially difficult given the C + Rust underlying code with the x264 static build)
• Wait for Linux and Windows ARM GitHub runners (currently in beta, should be available by the end of the year).

[Jegp]

An absolutely incredible effort. I can really see how much time and effort you spent here. It shows, and the API that's emerging from this is gorgeous. Not to mention really, really useful.

My only puzzle is whether we can somehow avoid copying in all the x264 code. Isn't there some for of cross-platform library for this? I realize this will be an added dependency, but it is a dependency, and then we don't have to maintain it :-)

Apart from that, I have a few minor remarks about the code. (But in general, I think you made some great choices. Particularly around the naming for the parameters (we discussed frequency etc). There are so many changes that it's difficult to make it through all of them, I'm not well-versed in Rust, and I skipped the x264 code. But all in all, I think it's ready to go! I can't wait to get my hands on this :)

Oh, and regarding the cross-compilation, I'm definitely in favor of waiting for a one-size-fits-all solution. That is, wait for the ARM runners. Especially if it means simplifying our lives. Our time is limited, unfortunately.

[Jegp]

Isn't the state manager redundant here? Maybe we could consider a default argument

[aMarcireau]

I am not sure what you mean by redundant.

[Jegp]

You're passing in the on_progress as an empty lambda (lambda _: None). Doesn't that mean the state manager is redundant? Or that the state isn't being updated?

[aMarcireau]

Oh, I see! The state manager is indeed redundant if the user does not specify an on_progress function, but it is useful if they do. We could optimize out the state manager when on_progress does nothing (we would probably want to use None as a default to do that), but it (hopefully) has minimal impact on performance (I have not checked though).

[Jegp]

Why can't we use format strings for this? Like f"{number:.2f}". It seems like you're trying to avoid some floating point rounding nastiness

[aMarcireau]

I added this to show integers when possible.

The function is intended to format the "speed up" ratio that appears on videos (that is, the duration represented by one event frame divided by the time that the frame is shown on screen by the video player). I quite like the following display rules:

• Show the speed up directly when it is larger than one (for instance x 10 for a time-lapse where each event frame covers 1/6 seconds in a video where each frame is shown for 1/60 seconds).
• Show the speed up as an inverse when it is smaller than one (for instance x 1/10 for a slow-motion video where each event frame covers 1/600 seconds in a video where each frame is shown for 1/60 seconds). This is particularly useful when discussing very slow motion videos ("ten thousand times slower than real-time", represented by x 1/10000, is easier to parse than x 0.0001).
• Show integers when close to a round value (x 1/10 instead of x 1/10.00) but keep the fractional digits where needed (x 1/1.4, not x 1/1).

Python's formatting function gives us three options that all are all (sometimes) problematic:

• Let Python pick the number of digits (f"x {number}"), which can cause problems when the number is close to but not quite an integer.
• Always show an integer (f"x {number:.0f}"), which can be misleading when the number is a small non-integer.
• Always show digits (f"x {number:.2f}"), which often shows redundant fractional digits.

[Jegp]

There are also https://docs.python.org/3/library/stdtypes.html#float.as_integer_ratio and https://docs.python.org/3/library/fractions.html#fractions.Fraction. I agree that the use cases are great, but I'm just wondering whether some of the code could be simplified. It seems like a problem that someone in the Python ecosystem has faced before :)

[aMarcireau]

That's a fair point! I was not aware of as_integer_ratio. I'll try to refactor this to simplify the code.

[aMarcireau]

Hi @Jegp,

Thank you for the review, this is extremely helpful!

Regarding H.264 video compression, there are indeed a few ways to set it up.

• (a) Compilation and static linking within the repository (which is what I did here). This allows for a completely standalone Python package and (possible) performance gains from static linking + link-time optimization (LTO). The biggest drawback, as you correctly pointed out, is the added maintenance burden. A similar solution consists in using submodules rather than hosting a copy of the files, but the x264 code base is fairly old and very stable so having an actual copy saves us from the madness of submodules. In either case, this approach requires a fairly complex CI/CD pipeline to compile the C + assembly x264 code base before compiling the Rust module.

• (b) Dynamic linking to libx264 or FFmpeg. There are many Rust and Python libraries that work that way (for instance https://github.com/kkroening/ffmpeg-python, https://github.com/zmwangx/rust-ffmpeg, or https://github.com/quadrupleslap/x264). The biggest downside is that a specific version of libx264 or FFmpeg must be installed on the user's machine, typically using their package manager (nix, apt, Homebrew, winget...). In many cases, the right version is not directly available and can cause all sorts of user-side headaches. A workaround is to build the right library version ourselves and ship it with the Python wheels, but that would essentially boil down to (a) without static linking.

• (c) Pure-Rust implementation of H.264 (the algorithm that the x264 library implements). This would be the best of both worlds since we could use Rust throughout (no C + assembly horror in the CI/CD pipeline). This would make the CI/CD pipeline (and cross-compilation) dramatically simpler but we would still ship a standalone library. Unfortunately, I could not find a library that does this (https://github.com/alfg/mp4-rust only writes MP4 container headers but has no support for compression, https://github.com/oddity-ai/rave is incomplete and possibly abandoned, https://github.com/xiph/rav1e/ only supports AV1, which is better than H.264 but incredibly slow without hardware acceleration).

I would love to migrate to (c) when a mature library shows up. In the mean time, I am leaning towards (a) since debugging CI/CD issues, as frustrating as it is, is still better than debugging compatibility issues on a wide range of operating systems and configurations (https://xkcd.com/1987/).

[Jegp]

Thank you for your comments. I agree that (c) is by far the best option. But I also agree that we need the library to exist. I also agree that (a) is the best solution for now. I just don't like it :)

At some point, I would really like to leverage nix to sort out these kinds of dependencies. Because then we can link all we want. Statically and dynamically. Unfortunately, it only works on Windows subsystem at the moment (https://nix.dev/install-nix.html)

[Jegp]

🎉