GeopJr / Tuba

Browse the Fediverse
https://tuba.geopjr.dev/
GNU General Public License v3.0
506 stars 55 forks source link

experiment(mediaviewer): clapper support #931

Closed GeopJr closed 2 weeks ago

GeopJr commented 2 months ago

Mostly experimenting

Overall Clapper is awesome! Streaming just works and it comes with many features!

Obviously, this is just an initial implementation, everything will get ironed out as we move forward with this (like saving volume > 1, fullscreen etc...)

Screencast from 2024-04-30 05-31-40.webm

cc: @Rafostar

Rafostar commented 2 months ago

I did not expect a draft PR so soon, surprise. Also, I see that I am for some reason co-authoring this... but I didn't do anything yet :sweat_smile:

Flatpak manifest partially copied from https://github.com/Rafostar/clapper-vala-test/

I have updated the Clapper Vala test recently. It now simply builds latest GStreamer version without any patches. I also removed some extra flatpak permissions from it.

I mainly used that repo for testing Vala bindings (and Clapper GI bindings overall) during development (which took a while). At that time GStreamer 1.24 did not exist yet, so I was working around my issues with patches, now that most of stuff I needed is upstreamed or fixed there and 1.24 exists now, there is no need for them anymore.

Due to above, I kindly ask you to not spread these patches further. If you are going to build GStreamer anyway, just use latest version with all fixes/benefits from it without patching stuff please. Its my fault for not doing that before tagging a release of Clapper, sorry.

Rafostar commented 2 months ago

Hey, thanks for the update :+1:

I noticed that when dismissing the MV, Tuba freezes with gdk_gl_context_make_current() failed, I probably omitted something

I gave it a quick go, and this doesn't happen to me anymore after latest rebuild.

Obviously, this is just an initial implementation, everything will get ironed out as we move forward with this (like saving volume > 1, fullscreen etc...)

Yes, there is now UI look/logic to figure out left. Next runtime (Freedesktop SDK used for GNOME 47) will have GStreamer 1.24 so you will not have to build it. I started some work on download cache that you suggested would also be nice to have and will probably continue/finish it anyway since it might be useful for other apps too regardless if this PR will continue or get scrapped.

Now its entirely up to you whether you want to continue with this or not (and its totally fine if you do not :smile:). Cheers!

GeopJr commented 3 weeks ago

Hello again @Rafostar!

I'm still getting gdk_gl_context_make_current() or sometimes just crashing when dismissing the media viewer, any ideas? (Using flatpak)

Screencast from 2024-06-15 18-43-48.webm

(if github cant render the video above, visit https://github.com/GeopJr/Tuba/assets/18014039/1d4c1069-5345-4053-a0a7-6789c299d2b8)

Rafostar commented 2 weeks ago

This is supposed to be fixed in Clapper git master, but wasn't yet backported into a patch version release. Maybe you are using stable Clapper locally for development?

I removed Flathub Tuba from my system and installed latest artifact from this PR and I no longer get this error (tried on 2 different PCs - Intel and AMD GPU to confirm).

GeopJr commented 2 weeks ago

Oops, seems that GNOME Builder had cached an older version of clapper :sweat:

Thanks for the reply! I'll merge this soon!

Rafostar commented 2 weeks ago

Well, it still needs clapper-git, so you might want to hold off until fix is in stable release maybe?

BTW, ideally there should not be 2 fullscreen buttons. Maybe either hide the top one or from the Clapper controls panel (depending which one you prefer) by setting its fullscreenable prop to false.

GeopJr commented 2 weeks ago

It will be merged behind a flag for now, in case anyone building Tuba from source wants to test it. I'll probably hide clapper's fullscreen button for now, thanks for the tip!

Rafostar commented 2 weeks ago

It will be merged behind a flag for now, in case anyone building Tuba from source wants to test it.

Thanks. Maybe in addition to flag, an env variable can help in case there is still some problem to let user to test and/or switch between playback backend?

GeopJr commented 2 weeks ago

Maybe, though, if someone wants to test the clapper backend, they already are comfortable enough with building Tuba from source, so changing the flag shouldn't be too hard for them or too time consuming on compiling

Rafostar commented 2 weeks ago

Thanks for the merge! :tada:

@GeopJr

One more related thing I wanted to mention before I forget :smile:. I did that video download caching that you asked for some time ago on matrix. It streams and downloads video (with seeking working) at the same time just like you wanted. See signal.Player.download-complete and property.Player.download-enabled documentation and a working example code that loops 2 cached videos is available here.

Its gonna need a little effort from the application side to implement depending how and when Tuba wishes exactly to recycle downloaded locally content through.

GeopJr commented 2 weeks ago

Thank you so much for this!

I don't think it's going to need any effort at all from Tuba's side apart from setting the download dir and enabling it, right? Like most things Clapper, you made everything so easy and simple to use :)

I'll get on it right away!

LukaszH77 commented 2 weeks ago

So, what do I have to do to test Clapper enabled Tuba? Is it just using dev.geopjr.Tuba.Clapper.json file as target for flatpak-builder or something else?

GeopJr commented 2 weeks ago

Either that, or through GNOME Builder:

image image image

Edit:

I'd probably suggest GNOME Builder as to avoid messing with your system as, for now, Clapper requires some patches

LukaszH77 commented 2 weeks ago

I went with GNOME Builder and got this. Am I missing something?

obraz

Rafostar commented 2 weeks ago

@GeopJr

I don't think it's going to need any effort at all from Tuba's side apart from setting the download dir and enabling it, right?

I do not mean playback related effort, cause for this I took care of it like you say, but as stated in documentation:

Please note that player will not delete nor manage downloaded content. It is up to application to cleanup data in created cache directory (e.g. before app exits), in order to remove any downloads that app is not going to use next time it is run and incomplete ones.

i.e. Clapper does not know if app is going to reuse downloaded file at some path in future (possibly with next Tuba MediaViewer instance) or on next application launch or for other purposes after video is destroyed, etc.

So currently I decided its best for application to manage and delete files on its own however and whenever it wants. This means cleanup at exit, or deleting files older then N last used ones, or eventually keeping a local HashTable/database of sorts to reause a file on next application launch (by e.g. using stored/remembered filepath instead of URI).

Currently a ClapperMediaItem object instance remembers its cached file, so reusing it makes it play from cache. This is another possibility to reuse downloads (by e.g. keeping last few ClapperMediaItem in some array, finding and reusing them if same URI is requested).

So yeah, multiple possibilities depending when/how you wish to reuse a download for you to decide, Clapper takes care of playback + streaming and downloading.

GeopJr commented 2 weeks ago

Sounds good! Not sure if keeping MediaItems in memory will cause a big impact, I'll do some testing. If it does, I'll probably just save url_hash => filename in memory and create a media item from file if it exists/is cached

@LukaszH77 no idea, from what I can find online it might be related to drivers(?). I don't know why you are missing the fullscreen icon too, not sure if it's related...

But, from a user perspective, the Clapper backend doesn't have that many changes visually, apart from the new controls

The big changes behavior wise are more features image

and streaming, which allows users to watch big lengthy high quality videos without needing to first download them and then play them!

Rafostar commented 2 weeks ago

@LukaszH77

Check if you have Mesa and org.freedesktop.Platform.GL installed. Alternatively you can install Clapper from Flathub to check if same issue happens. If it does feel free to open an issue on Clapper GitHub.

@GeopJr

Sounds good! Not sure if keeping MediaItems in memory will cause a big impact, I'll do some testing. If it does, I'll probably just save url_hash => filename in memory and create a media item from file if it exists/is cached

ClapperMediaItem does not store actual data in memory, only few strings like URI, title, etc. So there shouldn't be big/noticeable impact. Just remember to delete files that will not be used anymore and/or just whole dir on exit then.

LukaszH77 commented 2 weeks ago

IDK why, but it just works now, and I must say I'm really impressed how great it is. Good job!

GeopJr commented 2 weeks ago

You can also use Clapper as your standalone video player! https://flathub.org/apps/com.github.rafostar.Clapper

It's very powerful, it can even play videos from remote sources like youtube

image

Rafostar commented 2 weeks ago

IDK why, but it just works now, and I must say I'm really impressed how great it is. Good job!

Thanks! Player on its own (Flatpak Clapper) can also play from PeerTube using video webpage URI, but you have to replace URI scheme (https:// -> peertube://) to explain it that such website hosts it.

LukaszH77 commented 2 weeks ago

I tried Clapper when it first got on Flathub, it seems it got much better since then. I like it.

After using Clapper-enabled Tuba for almost a day, I can say that I really, really like not having to wait for the video to download :)

Two things: I wish there was a way to mute the video and change the volume without having to open the menu. And it would also be quite handy if the play button could replay the video after it finished.

GeopJr commented 2 weeks ago

If those feature requests are for Clapper standalone, they should probably be opened on https://github.com/Rafostar/clapper/issues so Rafostar can keep track of them!

Otherwise, I can probably add them to Tuba's Clapper controls!

edit: for the mute specifically, it can be toggled by clicking the volume icon on the popover image

Rafostar commented 2 weeks ago

I wish there was a way to mute the video and change the volume without having to open the menu.

@GeopJr

This one would be a feature request for Tuba, I decided to not implement shortcut controllers nor scroll as part of the video widget nor API in order to not clash or break other shortcuts that an app might want/reserve and allow to customize.

And application can do a custom controls panel if needed too. ClapperSimpleControls widget is a pre-made and as name states "simple" to use, but an app can design its own one, widgets that simple one consists of are available separately so its a matter of packing them differently (in e.g. GtkBox with "osd" class) in ui/blueprint file.

LukaszH77 commented 2 weeks ago

If those feature requests are for Clapper standalone, they should probably be opened on https://github.com/Rafostar/clapper/issues so Rafostar can keep track of them!

In standalone Clapper, I can change the volume with the mouse wheel and mute with the m key.

Rafostar commented 2 weeks ago

Two things: I wish there was a way to mute the video and change the volume without having to open the menu. And it would also be quite handy if the play button could replay the video after it finished.

For first question I answered above. For second one there is already https://github.com/Rafostar/clapper/issues/252

BTW, I do not think this closed PR is a good place for discussing things further. For questions related to Clapper and using its APIs, its Matrix channel might be a better place.