search

Rafostar / clapper

Level up your video experience with a modern and user-friendly media player.
https://rafostar.github.io/clapper/
GNU General Public License v3.0
714 stars 34 forks source link

Changes for GNOME 44 #344

Closed DaPigGuy closed 2 months ago

DaPigGuy commented 1 year ago

Port the about dialog and resume dialog to nicer looking counterparts from libadwaita :upside_down_face:

Rafostar commented 1 year ago

Thanks for contributing. May I ask what minimal libadwaita version will be required with this change?

DaPigGuy commented 1 year ago

libadwaita 1.2

DaPigGuy commented 1 year ago

CI should no longer fail on aarch64

Rafostar commented 1 year ago

Yes, it passed recently. I will test this before merging.

DaPigGuy commented 1 year ago

Commit 4f7bae1e54ed8bd64a2a2542b5e8fcdcc7177d54 brings the minimum GTK version to 4.10

Docker image for Flatpak Github Action doesn't seem to be tagged for the GNOME 44 runtime at the moment (https://github.com/flatpak/flatpak-github-actions/pull/125)

Rafostar commented 1 year ago

Hi, since you are still pushing commits, should I mark and treat this as WIP (draft)?

DaPigGuy commented 1 year ago

I'll mark as Draft, but we are just waiting for CI for GNOME 44

DaPigGuy commented 1 year ago

CI should continue to pass while on the gnome-43 tagged Docker image, PR is ready for review

Rafostar commented 1 year ago

Well, this overall seems fine to me. Thanks.

My one remaining concern is that this bumps minimal libadwaita version to 1.2 and latest Ubuntu LTS (and thus all distros based on it) are frozen on libadwaita 1.1.

Once merged, it won't be possible to ship latest Clapper git and next version on these distros from package manager like we do now. I cannot say when the next Clapper release will be and I have some other big changes planned that may/may not require bumping some other deps anyway. Thus if that's ok with you, I would hold this off for a little bit longer until we know if there will be some other reasons to not deliver Ubuntu packaging then cosmetic changes.

DaPigGuy commented 1 year ago

Yes, that seems fine to me. However, it would be nice if we could cherry-pick the commits 613bf68c4fbf72b92f824df5bf67a064334ade87 and 240193fa49cfd8b0ffabeebdffc671996b338592 just to bump the GNOME runtime version, or perhaps reflect the changes in the Flathub repository

orowith2os commented 9 months ago

Can we land this now?

Rafostar commented 9 months ago

Can we land this now?

There were various reasons why I decided to skip 44 runtime update. Will try to go straight to 45 since its now available. The remaining changes from this MR will probably land as part of the new code after new playback API is merged.

morfyum commented 8 months ago

Sorry for duplicate! Can I help somehow? Next release come with 44 or 45?

Thanks!

Rafostar commented 7 months ago

I will post more info on current project state here as more people are asking about runtime update.

Here is the current state of things:

Currently (for quite some time now, even before this MR) there is an ongoing initiative to change internals of this player into custom GStreamer implementation in a form of universal library that better works/is easier to use with GTK. This requires a complete rewrite of the app on top of it. Which is done in pure C this time around.

Due to that I would rather not touch GJS code at this point to not break it accidentally and fix all this in new implementation instead. For that I will take into account all changes that were done here and apply/use them when doing player code rewrite. I am grateful to @DaPigGuy for contributing and time he spent on this and will make sure that his suggestions/changes will be part of new code where/when applicable (some already are).

As for runtime update itself, I tried updating Flatpak directly to 45, but run into issue with subtitles scaling with latest GTK that comes with it. This will probably gonna need bisecting GTK changes to find the cause. Since as I mentioned, I dedicate my free time into player rewrite to finish it ASAP, this will unfortunately gonna have to wait until I am done, unless either the issue goes away in future updates or someone beats me to fixing it.

morfyum commented 7 months ago

Rewriting in C not making more issue than ever? I don't think so the C lang improve the video player performance, because its depends from codecs and gstreamer. Right? Clapper is the most useful video player on gnome with "native" theme.

Btw I dont understand why so hard to open 1 video, and 1 subtitle file, +optionally change audio, but only this app and vlc can.

I trust the best! 🤝

Rafostar commented 7 months ago

I don't think so the C lang improve the video player performance, because its depends from codecs and gstreamer.

Rewrite in C is NOT done to improve performance. There are other reasons, but this MR is unrelated to that so lets not discuss it here :-) Anyone is free to join Clapper matrix channel to ask questions and see progress updates.

Rafostar commented 2 months ago

This MR is now obsolete. Most of the changes suggested here were done as part of recently merged code rewrite, so closing.