Igalia / meta-webkit

Yocto / OpenEmbedded layer for WebKit based engines and browsers
MIT License
125 stars 69 forks source link

Fix compiler issue with the libyuv library in libwebrtc #332

Closed miltechniks closed 2 years ago

miltechniks commented 2 years ago

When building the wpewebkit from the git-source together with libwebrtc for imx8 there is a compiling Issue within libyuv. This is already fixed, but not in the currently used version. I created a patch for it.

Please have a look here: https://chromium-review.googlesource.com/c/libyuv/libyuv/+/2922146 and here: https://chromium-review.googlesource.com/c/libyuv/libyuv/+/2922147

philn commented 2 years ago

@ceyusa rings a bell?

ceyusa commented 2 years ago

@ceyusa rings a bell?

I don't think so. @psaavedra and I found a problem with libyuv and NEON when compiling with rpi4 toolchain (without neon support).

philn commented 2 years ago

@ceyusa rings a bell?

I don't think so. @psaavedra and I found a problem with libyuv and NEON when compiling with rpi4 toolchain (without neon support).

so what you found was not related with this PR at all?

psaavedra commented 2 years ago

Not for rpi4 but building libwebrtc on arm/v7 for rpi3:

{standard input}: Assembler messages:
{standard input}:5042: Error: Neon quad precision register expected -- `vld1.8 q4,[r3]'
{standard input}:5150: Error: instruction does not accept this addressing mode -- `vld1.8 d8,[r3,#16]'

Changing -mfpu=neon-vfpv4 by -mfpu=vfpv4 the problem is not longer there but that seems not the right preset for the rpi3.

@ceyusa you mentioned this workaround too:

diff --git a/Source/ThirdParty/libwebrtc/CMakeLists.txt b/Source/ThirdParty/libwebrtc/CMakeLists.txt
index 4345be388119..fa4d4ae350ef 100644
--- a/Source/ThirdParty/libwebrtc/CMakeLists.txt
+++ b/Source/ThirdParty/libwebrtc/CMakeLists.txt
@@ -1577,7 +1577,7 @@ else ()
         set(WEBKIT_LIBWEBRTC_OPENH264_ENCODER 1)
     endif ()

-    add_definitions(-DWEBKIT_LIBWEBRTC_OPENH264_ENCODER=${WEBKIT_LIBWEBRTC_OPENH264_ENCODER})
+    add_definitions(-DWEBKIT_LIBWEBRTC_OPENH264_ENCODER=${WEBKIT_LIBWEBRTC_OPENH264_ENCODER} -DLIBYUV_DISABLE_NEON)
     configure_file(LibWebRTCWebKitMacros.h.in LibWebRTCWebKitMacros.h @ONLY)

     list(APPEND webrtc_SOURCES

But in any case, it doesn't look like the one that @miltechniks descrives.

psaavedra commented 2 years ago

I think this patch it could be also added in L8 for the stable package too, or not @miltechniks ?

github-actions[bot] commented 2 years ago

Pull Request is being checked for https://github.com/Igalia/meta-webkit/pull/332/commits

miltechniks commented 2 years ago

@psaavedra What do you mean with L8?

psaavedra commented 2 years ago

@psaavedra What do you mean with L8?

Sorry, I supposed the link could be expanded well but it wasn't. I was asking if it could be also useful for the SRC_URI definition.

SRC_URI = "\
    https://wpewebkit.org/releases/${BPN}-${PV}.tar.xz;name=tarball \
    file://0001-WPE-GTK-Multiple-build-issues-with-ENABLE_VIDEO-OFF.patch \
    <<<HERE>>>
"
miltechniks commented 2 years ago

Ah, ok. No, not really. The Archive in the SRC_URI is missing the libwebrtc and therefore also the libyuv.

psaavedra commented 2 years ago

Ah, ok. No, not really. The Archive in the SRC_URI is missing the libwebrtc and therefore also the libyuv.

OK.

The CI bot is failing. Would you mind to rebase your miltechniks:main branch against main and push it again?

miltechniks commented 2 years ago

It should be up to date.

github-actions[bot] commented 2 years ago

Pull Request is being checked for https://github.com/Igalia/meta-webkit/pull/332/commits

psaavedra commented 2 years ago

It should be up to date.

We don't want "merge" commits in the Git history (0bcc1a67fa8c4bbbe40d27ddd9c7dbd73f5108c5 e352e2f3ce04d6cc32ba4aefea76017d87e83a28)

I'd suggest to update your MR branch with something like this:

git checkout main  # Assuming you are using main as origin in your Pull Request.
git remote add upstream git@github.com:Igalia/meta-webkit.git
git fetch upstream
git rebase -i upstream/main
git push -f origin main
miltechniks commented 2 years ago

It should be up to date.

We don't want "merge" commits in the Git history (0bcc1a6 e352e2f)

I'd suggest to update your MR branch with something like this:

git checkout main  # Assuming you are using main as origin in your Pull Request.
git remote add upstream git@github.com:Igalia/meta-webkit.git
git fetch upstream
git rebase -i upstream/main
git push -f origin main

Sorry, I updated it now.

github-actions[bot] commented 2 years ago

Pull Request is being checked for https://github.com/Igalia/meta-webkit/pull/332/commits

psaavedra commented 2 years ago

It should be up to date.

We don't want "merge" commits in the Git history (0bcc1a6 e352e2f) I'd suggest to update your MR branch with something like this:

git checkout main  # Assuming you are using main as origin in your Pull Request.
git remote add upstream git@github.com:Igalia/meta-webkit.git
git fetch upstream
git rebase -i upstream/main
git push -f origin main

Sorry, I updated it now.

The devupstream include was borken, please can you rebase the commit again and update the Pull Request one more time. The CI should pass now and we could merge this change. Thanks

miltechniks commented 2 years ago

Give it another try.

github-actions[bot] commented 2 years ago

Pull Request is being checked for https://github.com/Igalia/meta-webkit/pull/332/commits

psaavedra commented 2 years ago

Pull Request is being checked for https://github.com/Igalia/meta-webkit/pull/332/commits

Everything looks good.

Bastian-Krause commented 2 years ago

Not for rpi4 but building libwebrtc on arm/v7 for rpi3:

{standard input}: Assembler messages:
{standard input}:5042: Error: Neon quad precision register expected -- `vld1.8 q4,[r3]'
{standard input}:5150: Error: instruction does not accept this addressing mode -- `vld1.8 d8,[r3,#16]'

Changing -mfpu=neon-vfpv4 by -mfpu=vfpv4 the problem is not longer there but that seems not the right preset for the rpi3.

@psaavedra For the sake of completeness: this should be fixed with libyuv's daf9778a ("Fix for failed compile with armv-7a neon gcc").