feat(mv): enable caching for clapper

[GeopJr]

I'm not 100% sure about what cleaning up process to choose, so I'll leave it at that for now

Vapi is being to strict on clapper version matching so I sed it to 0.8.0 for now

[GeopJr]

@Rafostar

I cannot reproduce it reliably, but sometimes it will crash when dismissing, here's a backtrace:

#0  0x00007ffff7d7f918 in g_type_check_instance_is_a () at /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#1  0x00007ffff6ab96f3 in gtk_widget_set_visible () at /usr/lib/x86_64-linux-gnu/libgtk-4.so.1
#2  0x00007ffff645c151 in _set_buffering_animation_enabled (enabled=0, self=0x555560860b40) at ../src/lib/clapper-gtk/clapper-gtk-video.c:950
#3  _set_buffering_animation_enabled (enabled=0, self=0x555560860b40) at ../src/lib/clapper-gtk/clapper-gtk-video.c:942
#4  _player_state_changed_cb (player=<optimized out>, pspec=<optimized out>, self=0x555560860b40) at ../src/lib/clapper-gtk/clapper-gtk-video.c:969
#5  0x00007ffff7d586fa in g_closure_invoke () at /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#6  0x00007ffff7d6e3bc in signal_emit_unlocked_R.isra.0 () at /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#7  0x00007ffff7d6fe41 in signal_emit_valist_unlocked () at /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#8  0x00007ffff7d75e11 in g_signal_emit_valist () at /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#9  0x00007ffff7d75ed3 in g_signal_emit () at /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#10 0x00007ffff7d5cd54 in g_object_dispatch_properties_changed () at /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#11 0x00007ffff64b3b29 in gst_object_dispatch_properties_changed (object=0x55556102a300, n_pspecs=1, pspecs=0x7fffffffdf30) at ../subprojects/gstreamer/gst/gstobject.c:457
#12 0x00007ffff7d611e3 in g_object_notify_by_pspec () at /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#13 0x00007ffff65e7e79 in _handle_prop_notify_msg (structure=0x7ffeac0333e0, msg=0x7ffeac033510) at ../src/lib/clapper/clapper-app-bus.c:129
#14 clapper_app_bus_message_func (bus=<optimized out>, user_data=<optimized out>, msg=0x7ffeac033510) at ../src/lib/clapper/clapper-app-bus.c:284
#15 clapper_app_bus_message_func (bus=<optimized out>, msg=0x7ffeac033510, user_data=<optimized out>) at ../src/lib/clapper/clapper-app-bus.c:277
#16 0x00007ffff64c9e07 in gst_bus_source_dispatch (source=0x555560c719b0, callback=0x7ffff65e7d00 <clapper_app_bus_message_func>, user_data=0x0) at ../subprojects/gstreamer/gst/gstbus.c:834
#17 0x00007ffff7ed1667 in g_main_dispatch () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#18 0x00007ffff7ed3787 in g_main_context_iterate_unlocked.isra () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#19 0x00007ffff7ed3e43 in g_main_context_iteration () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#20 0x00007ffff7b9f05d in g_application_run () at /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0
#21 0x00005555555aaca4 in tuba_application_main (args=0x7fffffffe488, args_length1=1) at ../../../../../../../../../Projects/Tuba/src/Application.vala:222
#22 0x00005555555aacea in main (argc=1, argv=0x7fffffffe488) at ../../../../../../../../../Projects/Tuba/src/Application.vala:163

edit:
Might have to do with tuba destroying the widget while clapper is still working with it

[Rafostar]

I cannot reproduce it reliably, but sometimes it will crash when dismissing, here's a backtrace

I will look into it when I can.

I'm not 100% sure about what cleaning up process to choose, so I'll leave it at that for now

Linking for reference, where I mentioned different possibilities and why this is needed: #931 (comment). How it's done is entirely up to application to decide.

But in the end cache dir or its content (except files you want to keep for next Tuba launch if any) should be deleted anyway since it might contain incomplete downloads, ideally we should detect them, but it would need work (creating PR) at GStreamer, since upstream doesn't expose that currently unfortunately.

[GeopJr]

How it's done is entirely up to application to decide.

I'm split between setting a max limit (200mb) and checking every ~5 mins or when the media viewer closes vs just clearing on close or start, I'll make a choice later.

ideally we should detect them

I think I can do it in app, by saving all completed destinations in an array and deleting every file that's in the folder but not the array when the media viewer closes

[Rafostar]

I think I can do it in app, by saving all completed destinations in an array and deleting every file that's in the folder but not the array when the media viewer closes

Yeah, that could be done as workaround. Just shows that there is some logic/work here to decide and implement for you 😓. But hey, at least you do not have to care about streaming+downloading+seeking at the same time just managing files by paths in cache directory however you want. So I hope this is still useful for Tuba.

[GeopJr]

You've done more than enough! In fact I'd suggest not overthinking cache further. Tuba is one of the first apps to use Clapper, but as other apps also start using it they'll have different needs than Tuba, so it's better if you have a generic approach so other apps decide on their own how they want to deal with cache.

Libsoup's built in cache, for example, has an opinionated approach on loading, saving and flushing and when something doesn't work the way I want it to, I have to either disable it or attempt to bypass it by replicating what it does internally.

[Rafostar]

I compiled Tuba from source to test my stuff and download cache seems to be working nicely with Tuba. 👍

I cannot reproduce it reliably, but sometimes it will crash when dismissing, here's a backtrace:

I pushed a commit to Clapper master that I hope will fix this. I haven't encountered any crashes anymore afterwards, but would be great if you could rebuild Clapper from latest source to confirm too.

[GeopJr]

Awesome! Can confirm it got fixed!

---

I added my initial cleanup implementation. It saves the locations in another array, and if there are more than 10 (I set it to 3 for testing, I will change it to 10 before merging), it will remove from the start of the array until there are 10 items max. Then will delete everything not in that array.

That way there's at most 10 cached videos at a time. Also at boot, it will empty out the cache folder.

---

I noticed a new bug:

When opening this video https://mastodon.social/@anatudor/112432781314072465, the first attachment wont trigger the download-complete signal, but it does get saved in the cache folder as expected. My speculation is that it finishes downloading before the signal is setup(?)

Cached items:

Cache folder:

Notice how the 3rd video is missing from the cached items

[GeopJr]

I did some debugging, and this is the culprit:

if (G_UNLIKELY (player->played_item == NULL))
      return;

I'm mostly speculating again, but maybe it's because the video is short and finishes by the time it gets there? so the played_item is null?

[Rafostar]

I did some debugging, and this is the culprit:

if (G_UNLIKELY (player->played_item == NULL))
      return;

I'm mostly speculating again, but maybe it's because the video is short and finishes by the time it gets there? so the played_item is null?

Thanks for help. I simply did not have time yet to look into it deeply. But if what you found is the case, then yes. It would be just that video playback cannot start playing until there is enough content downloaded and because video is very short its fully downloaded before there is enough data buffered for the playback to start.

[Rafostar]

When opening this video https://mastodon.social/@anatudor/112432781314072465, the first attachment wont trigger the download-complete signal, but it does get saved in the cache folder as expected.

Should be fixed now.

BTW, latest clapper-git can now also be compiled and runs on MS Windows. Just mentioning, cause you might care as I see you do windows builds of Tuba too.

[Rafostar]

That way there's at most 10 cached videos at a time. Also at boot, it will empty out the cache folder.

@GeopJr

With current logic you did, it should be fairly easy to do a persistent cache across app launches. Do you think it would be plausible for Tuba?

I think about adding something like clapper_media_item_new_cached (uri, location) so app would not need to worry about cache file still existing on next launch (it would try playing from location and on failure fallback to URI and let it be recached internally).

Dunno if it is possible here that video at the exact same URI changes when post is updated (different video without URI change)...

[GeopJr]

Should be fixed now.

Thanks!

BTW, latest clapper-git can now also be compiled and runs on MS Windows. Just mentioning, cause you might care as I see you do windows builds of Tuba too.

Awesome, I'll ask the msys2 maintainers or package it myself for msys2 when it makes it into a release!

With current logic you did, it should be fairly easy to do a persistent cache across app launches. Do you think it would be plausible for Tuba?

I don't know if I want to keep the cache for Tuba between restarts to be honest (and might be better to just clear it on shutdown instead). I don't think people will like having huge files in cache that they are probably never going to play again. But I can probably see other apps needing it (?)

I think about adding something like clapper_media_item_new_cached (uri, location) so app would not need to worry about cache file still existing on next launch (it would try playing from location and on failure fallback to URI and let it be recached internally).

I believe that's close to what libsoup does for caching!

Dunno if it is possible here that video at the exact same URI changes when post is updated (different video without URI change)...

https://gitlab.gnome.org/GNOME/libsoup/-/blob/master/libsoup/cache/soup-cache.c

libsoup keeps track of the media's etag, max cache/no cache headers etc to determine what to do. So if the same url has different etags, then it invalidates the previous one and caches the new one

[Rafostar]

Awesome, I'll ask the msys2 maintainers or package it myself for msys2 when it makes it into a release!

Thanks, but I do not want to give you additional thing to maintain. Hopefully once its in a stable release and there are some apps that can be built on Win that depend on it, then hopefully someone interested will step up and volunteer to maintain msys2 package.

I don't know if I want to keep the cache for Tuba between restarts to be honest (and might be better to just clear it on shutdown instead). I don't think people will like having huge files in cache that they are probably never going to play again. But I can probably see other apps needing it (?)

👍

libsoup keeps track of the media's etag, max cache/no cache headers etc to determine what to do. So if the same url has different etags, then it invalidates the previous one and caches the new one

That would be more of a future thing to do (if needed). If Clapper were to do that checking internally, there would be few obstacles. First being that while app like Tuba uses only "https(s)" URIs that lead to video, it might not be the case for another app cause GStreamer can work with all different kind of sources.

[GeopJr]

hopefully someone interested will step up and volunteer to maintain msys2 package.

That's what I'm hoping for too, but I don't mind making the initial package for them!

That would be more of a future thing to do (if needed). If Clapper were to do that checking internally, there would be few obstacles. First being that while app like Tuba uses only "https(s)" URIs that lead to video, it might not be the case for another app cause GStreamer can work with all different kind of sources.

Oh yeah, totally! I'm just pointing out how libsoup deals with urls with variable content

[Rafostar]

@GeopJr

I think about adding something like clapper_media_item_new_cached (uri, location)

I did that and made a PR (see: #1044) with changes to be added here. I think it simplifies your code a bit, since it removes custom structure and does not require you to store whole media item objects in memory (only location strings).

[Rafostar]

Could you rebase this branch on top of main, pretty please? It will be easier to quickly checkout and test how this works together with recently merged GIFV looping.

[GeopJr]

Done, thanks!

---

well, didn't rebase, but merged main

[GeopJr]

Feature freeze is finally done and I can merge this, I think it's pretty much done though I want to do some more testing. Let me know if there's any changes upstream :)

[Rafostar]

Thanks for the rebase and update. I was a little busy this week, so haven't had much time to test this some more. I will try to do so, later today.

[GeopJr]

No rush! It can wait if needed, next release is planned for months from now anyway

[Rafostar]

I did some quick testing. Caching seems to work fine. For anything else I found out while testing this I did a fix PR: #1075

Mainly fixes problem with all GIFs being downloaded at once due to multiple players going "paused" and a problem created by using multiple MPRIS instances with the same name at once (seems we can simply disable MPRIS for GIFs as I think we should as all other posts have only 1 video max AFAIK - otherwise we would need to append instance number in MPRIS bus name).

[GeopJr]

I think we should as all other posts have only 1 video max AFAIK - otherwise we would need to append instance number in MPRIS bus name).

Looks like we are going to have to do that. While Mastodon only allows one video, other backends like Misskey don't have a limit

I think I'll just use a uuid for the instance number 🤷

[Rafostar]

other backends like Misskey don't have a limit

Oh, didn't know that. Thank you for clarifying. In this case instance numbers should be added if app might create more then 1 player with MPRIS feature at once.

[Rafostar]

FWIW, I tried this after latest main update and seems to work fine here for me :-)
Only thing missing is an actual Clapper 0.8 release that adds caching.

[GeopJr]

Awesome, merging! Thanks for everything :)