Add rav1e to contribs #3199
Add rav1e to contribs #3199
Conversation
|
Thanks, will review as time allows. |
|
Looks like Linux CI succeeded even though it failed in contribs: It fails like that for all platforms right now, as it does not fetch the openssl-src properly with cargo. Will look into that. |
|
On macOS I'm getting: No idea what that means, but I hope it helps. |
|
@Nomis101 Never seen that so far, what did you do that caused that? I guess maybe I need to override the whole target to make sure it does not add some arbitrary extra flags that are incorrect for direct use with cargo… |
|
@Nomis101 hmm could it be your rust toolchain is very old? Can you share the output of |
|
@ePirat: It's not very old, but yes, this seems to be the above issue. I have rustc 1.43.1 (8d69840ab 2020-05-04) installed, because I need to it build Mozilla Thunderbird and a newer versions fails to build esr78. |
|
Thanks for the additional info. And yes, it's expected to fail without a rust toolchain. |
|
Thanks for the clarification. Yes, if I install rustc 1.47, rav1e builds just fine. But ffmpeg seems to not find rav1e. Is this expected as well? |
|
Yeah thats the same CI fails for now, I will look into this when I have time, probably just a matter of setting the right |
|
@ePirat rav1e should "install" its pkg_config into HB's ./build/contrib/lib/pkgconfig |
|
@maximd33 That's already happening, which is why I expected ffmpeg to just find it, as I assumed the contrib pkgconfig directory would already be set as |
|
@ePirat I see
|
|
Figured out why for ffmpeg the PKG_CONFIG_PATH was not set properly. Although maybe there was a legitimate reason for it to override the whole default env, in which case PKG_CONFIG_PATH needs to be set manually there… |
|
So now it clearly sets the right |
|
@ePirat
ffmpeg - have you checked details at ffbuild/config.log ? |
|
@maximd33 As it fails on CI I can not easily. Will add some logging in CI in my fork, that prints this file, to figure out whats wrong. You need at least rustc 1.47 so the cargo version will be at least the one that cargo-c depends on. |
|
I need to say, I have no idea what this means, but in ffbuild/config.log I see: |
ePirat
force-pushed
the
epirat-add-rav1e
branch
6 times, most recently
from
0e51cba
to
dad5af7
Compare
ePirat
force-pushed
the
epirat-add-rav1e
branch
from
916bc8a
to
63c15e9
Compare
ePirat
force-pushed
the
epirat-add-rav1e
branch
from
63c15e9
to
7ecf6fb
Compare
The common contrib.defs already add the env.LOCAL_PATH to the final env, so this seems unnecessary and breaks adding the right PKG_CONFIG_PATH for contribs to ffmpeg.
This reverts commit 6e2c8a8.
ePirat
force-pushed
the
epirat-add-rav1e
branch
from
7ecf6fb
to
0cc6109
Compare
|
Got it building in macOS CI now, although it fails at the final link step: as rav1e is missing in the linker library list. What do I need to adjust to have it included in there? |
|
You need to add it to HandBrake.xcodeproj. You can use this as an example: |
|
Thanks for sticking with this. |
|
@Nomis101 Thanks for the example, thats very helpful! Although I wonder how I could make this work for rav1e given that at least currently it would be useful to have it as optional dependency (as it requires the rust toolchain, which not everyone might have installed)? Anyway I will probably proceed for now with just adding it so I can advance with the actual integration into Handbrake itself and solve this obstacle later. |
|
Circling back to this after quite some time. I'd like to get this in if we can separate out the dependencies by adding relevant commands to |
|
To conditionally link it in the Xcode project you can add something like these lines in x265 module.defs: HandBrake/contrib/x265/module.defs Line 25 in 61d7fc0 Just found out today we already had a system in place to resolve this 🙃 |
|
Since I'm curious: will this get merged before the end of this year? |
|
@galad87 If there is still interest in this MR I could give it a shot to rebase it and make the changes to get it working. It should be easier now that Handbrake already supports different AV1 encoders. |
|
I don't know about shipping it by default, even if we will probably need an AV1 encoder that isn't hopelessly slow on ARM cpu, but making it an optional encoder for now sounds good to me. Ping @bradleysepos @sr55 |
I rebased it locally, the only commits that I had to skip were the one related to mac stuff and two commits of libdovi. UPDATE: It fails, for now : Downloaded csv v1.1.3
: touch contrib/librav1e/.stamp.librav1e.configure
: /usr/bin/cargo clean --manifest-path=./contrib/librav1e/rav1e-0.3.4/Cargo.toml
: /usr/bin/rm -f ./contrib/librav1e/.stamp.librav1e.build
: ./contrib/bin/cargo-cbuild cbuild --manifest-path=./contrib/librav1e/rav1e-0.3.4/Cargo.toml --offline --release --library-type staticlib --prefix /home/gabriele/HandBrake/build/contrib/
: Error: CliError { error: Some(no matching package named `aom-sys` found
: location searched: registry `https://github.com/rust-lang/crates.io-index`
: required by package `rav1e v0.3.4 (/home/gabriele/HandBrake/build/contrib/librav1e/rav1e-0.3.4)`
: As a reminder, you're using offline mode (--offline) which can sometimes cause surprising resolution failures, if this error is too confusing you may wish to retry without the offline flag.), exit_code: 101 }
: gmake: *** [../contrib/librav1e/module.rules:3: contrib/librav1e/.stamp.librav1e.build] Error 1
-------------------------------------------------------------------------------
time end: Wed Mar 29 19:19:02 2023
duration: 7 minutes, 32 seconds (452.97s)
result: FAILURE (code 2)
-------------------------------------------------------------------------------
Build is finished!
You may now cd into ./build and examine the output.
UPDATE 2 The reason it fails now, after removing In fact, the compilation goes buttery smooth until it cannot find references into : /usr/bin/ld: /home/gabriele/HandBrake/build//contrib/lib/libavcodec.a(librav1e.o): in function `librav1e_receive_packet': : librav1e.c:(.text+0x7b): undefined reference to `rav1e_send_frame' : /usr/bin/ld: librav1e.c:(.text+0xb1): undefined reference to `rav1e_receive_packet' : /usr/bin/ld: librav1e.c:(.text+0xd4): undefined reference to `rav1e_frame_unref' : /usr/bin/ld: librav1e.c:(.text+0xec): undefined reference to `rav1e_status_to_str' : /usr/bin/ld: librav1e.c:(.text+0x13e): undefined reference to `rav1e_status_to_str' : /usr/bin/ld: librav1e.c:(.text+0x22e): undefined reference to `rav1e_frame_new' : /usr/bin/ld: librav1e.c:(.text+0x265): undefined reference to `rav1e_frame_fill_plane' : /usr/bin/ld: librav1e.c:(.text+0x2d6): undefined reference to `rav1e_twopass_in' : /usr/bin/ld: librav1e.c:(.text+0x313): undefined reference to `rav1e_twopass_out' : /usr/bin/ld: librav1e.c:(.text+0x38b): undefined reference to `rav1e_data_unref' : /usr/bin/ld: librav1e.c:(.text+0x512): undefined reference to `rav1e_frame_extract_plane' : /usr/bin/ld: librav1e.c:(.text+0x536): undefined reference to `rav1e_status_to_str' : /usr/bin/ld: librav1e.c:(.text+0x554): undefined reference to `rav1e_status_to_str' : /usr/bin/ld: librav1e.c:(.text+0x579): undefined reference to `rav1e_twopass_out' : /usr/bin/ld: librav1e.c:(.text+0x5ca): undefined reference to `rav1e_data_unref' : /usr/bin/ld: librav1e.c:(.text+0x5f0): undefined reference to `rav1e_frame_set_opaque' : /usr/bin/ld: librav1e.c:(.text+0x607): undefined reference to `rav1e_send_frame' : /usr/bin/ld: librav1e.c:(.text+0x637): undefined reference to `rav1e_packet_unref' : /usr/bin/ld: librav1e.c:(.text+0x666): undefined reference to `rav1e_packet_unref' : /usr/bin/ld: librav1e.c:(.text+0x678): undefined reference to `rav1e_packet_unref' : /usr/bin/ld: librav1e.c:(.text+0x68e): undefined reference to `rav1e_data_unref' : /usr/bin/ld: librav1e.c:(.text+0x6ff): undefined reference to `rav1e_data_unref' : /usr/bin/ld: /home/gabriele/HandBrake/build//contrib/lib/libavcodec.a(librav1e.o): in function `librav1e_encode_close': : librav1e.c:(.text.unlikely+0x13): undefined reference to `rav1e_context_unref' : /usr/bin/ld: librav1e.c:(.text.unlikely+0x29): undefined reference to `rav1e_frame_unref' : /usr/bin/ld: /home/gabriele/HandBrake/build//contrib/lib/libavcodec.a(librav1e.o): in function `librav1e_encode_init': : librav1e.c:(.text.unlikely+0x82): undefined reference to `rav1e_config_default' : /usr/bin/ld: librav1e.c:(.text.unlikely+0xd7): undefined reference to `rav1e_config_set_time_base' : /usr/bin/ld: librav1e.c:(.text.unlikely+0x1b7): undefined reference to `rav1e_config_parse' : /usr/bin/ld: librav1e.c:(.text.unlikely+0x1e8): undefined reference to `rav1e_config_parse_int' : /usr/bin/ld: librav1e.c:(.text.unlikely+0x21f): undefined reference to `rav1e_config_parse_int' : /usr/bin/ld: librav1e.c:(.text.unlikely+0x248): undefined reference to `rav1e_config_set_sample_aspect_ratio' : /usr/bin/ld: librav1e.c:(.text.unlikely+0x25d): undefined reference to `rav1e_config_parse_int' : /usr/bin/ld: librav1e.c:(.text.unlikely+0x28d): undefined reference to `rav1e_config_parse_int' : /usr/bin/ld: librav1e.c:(.text.unlikely+0x2b2): undefined reference to `rav1e_config_parse_int' : /usr/bin/ld: librav1e.c:(.text.unlikely+0x2d7): undefined reference to `rav1e_config_parse_int' : /usr/bin/ld: librav1e.c:(.text.unlikely+0x2fc): undefined reference to `rav1e_config_parse_int' : /usr/bin/ld: /home/gabriele/HandBrake/build//contrib/lib/libavcodec.a(librav1e.o):librav1e.c:(.text.unlikely+0x324): more undefined references to `rav1e_config_parse_int' follow : /usr/bin/ld: /home/gabriele/HandBrake/build//contrib/lib/libavcodec.a(librav1e.o): in function `librav1e_encode_init': : librav1e.c:(.text.unlikely+0x51b): undefined reference to `rav1e_config_set_pixel_format' : /usr/bin/ld: librav1e.c:(.text.unlikely+0x544): undefined reference to `rav1e_config_set_color_description' : /usr/bin/ld: librav1e.c:(.text.unlikely+0x550): undefined reference to `rav1e_context_new' : /usr/bin/ld: librav1e.c:(.text.unlikely+0x5bb): undefined reference to `rav1e_container_sequence_header' : /usr/bin/ld: librav1e.c:(.text.unlikely+0x5cb): undefined reference to `rav1e_data_unref' : /usr/bin/ld: librav1e.c:(.text.unlikely+0x62c): undefined reference to `rav1e_data_unref' : /usr/bin/ld: librav1e.c:(.text.unlikely+0x634): undefined reference to `rav1e_config_unref' : collect2: error: ld returned 1 exit status : gmake[3]: *** [Makefile:631: ghb] Error 1 : gmake[3]: Leaving directory '/home/gabriele/HandBrake/build/gtk/src' : gmake[2]: *** [Makefile:449: all-recursive] Error 1 : gmake[2]: Leaving directory '/home/gabriele/HandBrake/build/gtk' : gmake[1]: *** [Makefile:381: all] Error 2 : gmake[1]: Leaving directory '/home/gabriele/HandBrake/build/gtk' : gmake: *** [../gtk/module.rules:31: gtk.build] Error 2 : gmake: *** Waiting for unfinished jobs.... : /usr/bin/strip ./HandBrakeCLI ------------------------------------------------------------------------------- time end: Wed Mar 29 20:12:05 2023 duration: 20 seconds (20.41s) result: FAILURE (code 2) ------------------------------------------------------------------------------- Build is finished! You may now cd into ./build and examine the output. @ePirat Do you have any idea how to fix it? Perhaps an ffmpeg patch or something? |
|
I'm not the boss. Just one of 3 project owners. Personally, ra1ve is of zero interest to me. I'm of the opinion that we should stick to a single encoder, or a single encoder per platform if SVT is problematic. So I will yield to @galad87 on this one if we want to swap the macOS encoder over if this works better on M1/2 |
Description of Change:
As discusses on IRC, this adds the rav1e AV1 encoder to contribs, along with cargo-c, needed to build and install the C interface library for rav1e.
Some more work on this is needed regarding cross-compilation. But it would be best to discuss the general approach first
before those things are tackled.
Test on: