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

[tests] use "h264" instead of "libx264" for tests (fixes: #951) #962

Closed
wants to merge 2 commits into from

Conversation

jlaine
Copy link
Member

@jlaine jlaine commented Apr 18, 2022

This allows running the test suite against FFmpeg builds which do not
have libx264 enabled.

This allows running the test suite against FFmpeg builds which do not
have libx264 enabled.
@jlaine
Copy link
Member Author

jlaine commented Apr 18, 2022

@deepsworld this one is for you. Changing "libx264" to "h264" doesn't seem to break the test suite if libx264 is available. I also tried removing libx264 support from our FFmpeg, but then things do start to break

@jlaine
Copy link
Member Author

jlaine commented Apr 22, 2022

I think I'm most likely going to close this issue, it doesn't actually solve anything..

@hmaarrfk
Copy link
Contributor

I think you need a few more patches:
https://github.com/conda-forge/av-feedstock/blob/main/recipe/lgpl_tests.patch

@hmaarrfk
Copy link
Contributor

For more clarity, we at a conda-forge now build ffmpeg with (gpl) and without (lgpl) x264.

We then compile pyav with lgpl, and then test it with the gpl and the lgpl versions. Without the patch I linked to, the lgpl-only tests would fail.

@hmaarrfk
Copy link
Contributor

Nevermind, it seems that your patch is good. It actually includes the addition 5 line cleanup that I didn't include in mine due to not wanting to touch too many lines of code.

Did you build your FFMPEG builds with openh264? I'm happy to test the patch at conda-forge (where we have lgpl builds readily available).

@jlaine
Copy link
Member Author

jlaine commented Nov 1, 2023

As previously stated I think we should close this PR.

@jlaine jlaine closed this Nov 1, 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