Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

experiment(mediaviewer): clapper support #931

Merged
merged 11 commits into from
Jun 16, 2024
Merged

experiment(mediaviewer): clapper support #931

merged 11 commits into from
Jun 16, 2024

Conversation

GeopJr
Copy link
Owner

@GeopJr GeopJr commented Apr 30, 2024

Mostly experimenting

  • Flatpak manifest partially copied from https://github.com/Rafostar/clapper-vala-test/
  • Meson will detect if clapper is installed and enable clapper support, otherwise Gtk.Video will be used
  • I noticed that when dismissing the MV, Tuba freezes with gdk_gl_context_make_current() failed, I probably omitted something

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

Co-authored-by: Rafał Dzięgiel <rafostar.github@gmail.com>
@Rafostar
Copy link
Contributor

Rafostar commented May 1, 2024

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 😅

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
Copy link
Contributor

Rafostar commented May 5, 2024

Hey, thanks for the update 👍

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 😄). Cheers!

@GeopJr
Copy link
Owner Author

GeopJr commented Jun 15, 2024

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
Copy link
Contributor

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
Copy link
Owner Author

GeopJr commented Jun 16, 2024

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

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

@Rafostar
Copy link
Contributor

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
Copy link
Owner Author

GeopJr commented Jun 16, 2024

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
Copy link
Contributor

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
Copy link
Owner Author

GeopJr commented Jun 16, 2024

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

@GeopJr GeopJr marked this pull request as ready for review June 16, 2024 15:02
GNOME Builder defaults to the first .Devel.json manifest it finds, let's just drop the suffix for the clapper version
@GeopJr GeopJr merged commit 7800bb2 into main Jun 16, 2024
5 checks passed
@GeopJr GeopJr deleted the experiment/mv/clapper branch June 16, 2024 15:09
@Rafostar
Copy link
Contributor

Thanks for the merge! 🎉

@GeopJr

One more related thing I wanted to mention before I forget 😄. 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
Copy link
Owner Author

GeopJr commented Jun 16, 2024

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
Copy link
Contributor

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
Copy link
Owner Author

GeopJr commented Jun 16, 2024

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
Copy link
Contributor

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

obraz

@Rafostar
Copy link
Contributor

@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
Copy link
Owner Author

GeopJr commented Jun 16, 2024

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
Copy link
Contributor

@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
Copy link
Contributor

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

@GeopJr
Copy link
Owner Author

GeopJr commented Jun 19, 2024

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Owner Author

GeopJr commented Jun 19, 2024

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
Copy link
Contributor

Rafostar commented Jun 19, 2024

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
Copy link
Contributor

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
Copy link
Contributor

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 Rafostar/clapper#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.

@GeopJr GeopJr mentioned this pull request Jun 23, 2024
13 tasks
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.

3 participants