Skip to content

Conversation

@JakobDev
Copy link

This tries to add a 6.5 branch. I have edited the patch files local with a text editor (I don't know how to properly update them), so they don't throw a error any more., but I have no Idea what they do and I can't tell, if that was done right. I can't even say if this this even builds. If this builds and works, this needs to be merged in a new branch.

@flathubbot
Copy link

Started test build 49226

@JakobDev JakobDev mentioned this pull request Jun 23, 2023
@flathubbot
Copy link

Build 49226 failed

@JakobDev
Copy link
Author

QtWebEngine will not be built: Unmodified ffmpeg >= 5.0 is not supported.

Does that mean I need to include a older ffmpeg version?

@flathubbot
Copy link

Started test build 49236

@flathubbot
Copy link

Build 49236 successful
To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/31835/io.qt.qtwebengine.BaseApp.flatpakref

"-DQT_FEATURE_webengine_kerberos=ON",
"-DQT_FEATURE_webengine_proprietary_codecs=ON",
"-DQT_FEATURE_webengine_system_ffmpeg=ON",
"-DQT_FEATURE_webengine_system_ffmpeg=OFF",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why off?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ffmpeg version of the runtime is not compatible with Chromium

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and it is in the 6.4 branch? That's odd, what's the error?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was explained here. There are 3 possible solutions:

  1. runtime patches ffmpeg
  2. baseapp patches webengine
  3. internal ffmpeg is used

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thing is that 6.4 and 6.5 use the same freedesktop runtime. So if it works in 6.4 then it works in 6.5

Which is either 6.4 is broken or this should just be the same as 6.4

You can't expect that change in freedesktop runtime as 6.5 will not use a more recent runtime.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May also be that 6.5 requires a newer ffmpeg.

Anyhow i think "internal ffmpeg is used" is good enough, if -DQT_FEATURE_webengine_system_ffmpeg=OFF does that no opposition from my side.

Copy link

@Erick555 Erick555 Jun 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if it works in 6.4 then it works in 6.5

The webengine commit which blocks ffmpeg >= 5 (the problem is runtime ffmpeg is too new not too old) was introduced in 6.5.x branch. I don't know if 6.4 is silently broken or not affected.

Using internal ffmpeg should be just fine.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As Erik555 said, it doesn't build with -DQT_FEATURE_webengine_system_ffmpeg=ON, so we need to disable that.

It is also highly possible, that this is also broken in the 6.4 branch. I assume they got a bug report, that it is not working, so they added this check.

@JakobDev
Copy link
Author

Is there anything still prevent this from being merged?

@tsdgeos
Copy link

tsdgeos commented Jun 29, 2023

Is there anything still prevent this from being merged?

Nobody has said yes to my "Anyhow i think "internal ffmpeg is used" is good enough, if -DQT_FEATURE_webengine_system_ffmpeg=OFF does that no opposition from my side." yet.

Or i missed it ^_^

@Erick555
Copy link

yes, -DQT_FEATURE_webengine_system_ffmpeg=OFF enables internal ffmpeg.

@tsdgeos
Copy link

tsdgeos commented Jul 3, 2023

"I can't even say if this this even builds"

"Is there anything still prevent this from being merged?"

This is giving me a bit of conflicting information. Have you confirmed this at least builds or have you not? If you have not, what's preventing you to do it?

@JakobDev
Copy link
Author

JakobDev commented Jul 4, 2023

This is giving me a bit of conflicting information. Have you confirmed this at least builds or have you not?

It works. The buildbot has made a successfully testbuild. Did you miss the comment?

@tsdgeos
Copy link

tsdgeos commented Jul 4, 2023

Ok, i was hoping for something more than "it compiles" but i guess we could start with that...

I just realized i can't commit here though :D

@hfiguiere
Copy link
Contributor

let me create the branch then.

@hfiguiere
Copy link
Contributor

Branch created.

@hfiguiere hfiguiere closed this Jul 4, 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.

5 participants