[Buildstream SDK] x265 encoder support #15864
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
philn
added
the
Tools / Tests
|
EWS run on current version of this PR (hash a42b305) |
clopez
reviewed
Jul 17, 2023
Comment on lines
+254
to
+273
| +- if(ARM AND CROSS_COMPILE_ARM) | ||
| +- if(ARM64) | ||
| +- set(ARM_ARGS -fPIC) | ||
| +- else() | ||
| +- set(ARM_ARGS -march=armv6 -mfloat-abi=soft -mfpu=vfp -marm -fPIC) | ||
| +- endif() | ||
| +- message(STATUS "cross compile arm") | ||
| +- elseif(ARM) | ||
| +- if(ARM64) | ||
| +- set(ARM_ARGS -fPIC) | ||
| +- add_definitions(-DHAVE_NEON) | ||
| +- else() | ||
| +- find_package(Neon) | ||
| +- if(CPU_HAS_NEON) | ||
| +- set(ARM_ARGS -mcpu=native -mfloat-abi=hard -mfpu=neon -marm -fPIC) | ||
| +- add_definitions(-DHAVE_NEON) | ||
| +- else() | ||
| +- set(ARM_ARGS -mcpu=native -mfloat-abi=hard -mfpu=vfp -marm) | ||
| +- endif() | ||
| +- endif() |
There was a problem hiding this comment.
This logic doesn't make much sense to me.
- Why making a difference between the flags passed to the compiler when cross-compiling and when not?. We do care about redistributable binaries on the SDK. So we should always build with compatible flags to the minimum baseline supported at run-time and not use as baseline whatever the CPU that builds the SDK happens to have. So I suggest to avoid using flags like
-mcpu=native - Why use
march=armv6 -mfloat-abi=softfor ARMv7 when cross-compiling but end using-mcpu=native -mfloat-abi=hardwhen building natively for ARMv7?
There was a problem hiding this comment.
I copied those patches from https://github.com/rpmfusion/x265/
I can remove the ones related with armv6/7 if needed, we don't support those legacy arches anyway.
There was a problem hiding this comment.
ok, is fine I guess. If in the end we end supporting armv7 this needs an update but right now we don't support it.
Comment on lines
+295
to
+304
| +- if(ARM AND CROSS_COMPILE_ARM) | ||
| +- set(ARM_ARGS -march=armv6 -mfloat-abi=soft -mfpu=vfp -marm -fPIC) | ||
| +- elseif(ARM) | ||
| +- find_package(Neon) | ||
| +- if(CPU_HAS_NEON) | ||
| +- set(ARM_ARGS -mcpu=native -mfloat-abi=hard -mfpu=neon -marm -fPIC) | ||
| +- add_definitions(-DHAVE_NEON) | ||
| +- else() | ||
| +- set(ARM_ARGS -mcpu=native -mfloat-abi=hard -mfpu=vfp -marm) | ||
| +- endif() |
There was a problem hiding this comment.
same here than commented above.
clopez
approved these changes
Jul 17, 2023
philn
added
the
unsafe-merge-queue
https://bugs.webkit.org/show_bug.cgi?id=259242 Reviewed by Carlos Alberto Lopez Perez. For WPT WebCodec tests we need an HEVC software encoder, since the bots don't have GPU capabilities for hardware encoding. * Tools/buildstream/elements/freedesktop-sdk.bst: * Tools/buildstream/elements/sdk-platform.bst: * Tools/buildstream/patches/fdo-0009-gst-plugins-bad-Enable-x265-encoder.patch: Added. Canonical link: https://commits.webkit.org/266098@main
webkit-commit-queue
force-pushed
the
eng/259242
branch
from
a42b305
to
ca2221a
Compare
|
Committed 266098@main (ca2221a): https://commits.webkit.org/266098@main Reviewed commits have been landed. Closing PR #15864 and removing active labels. |
webkit-commit-queue
removed
the
unsafe-merge-queue
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ca2221a
a42b305