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

Caching is disabled? #109

Open
AndreKR opened this issue Jan 15, 2024 · 6 comments
Open

Caching is disabled? #109

AndreKR opened this issue Jan 15, 2024 · 6 comments

Comments

@AndreKR
Copy link

AndreKR commented Jan 15, 2024

Before I switched to LMS I was using librespot directly and it maintained a large cache directory and since I'm usually just playing my favorite playlists over and over I pretty much never had any playback issues even when the internet was spotty (no pun intended).

With LMS and Spotty I regularly have issues where playback takes several seconds to start or stops in the middle of a song.

Now I have read through some issues here and I'm aware that such issues could be due to the synchronization of playback state between LMS and Spotty.

But I also noticed that Spotty is running with --disable-audio-cache and the cache directory only ever contains a single file. With no caching at all I'm not particularly surprised that I'm having playback issues.

Is there a reason it's disabled or could we enable it?

@michaelherger
Copy link
Owner

It's disabled because I didn't want to have to deal with cache management: there's no size limit, nothing. The cache would simply grow infinitely. If that doesn't bother you feel free to modify the custom-convert.conf file to remove the --disable-audio-cache statement. But please note that I haven't tested this. I also don't know where it would store the cached files... On some systems it might end up in /tmpfs - which might be too small to keep a cache.

@AndreKR
Copy link
Author

AndreKR commented Jan 22, 2024

I got the cache working. 🎉

there's no size limit, nothing. The cache would simply grow infinitely.

There is support for a size limit in librespot and the code for that does exist in your branch, it's just that the command line option is missing and instead it's initialized to unlimited. Anyway, for my use case I don't need it.

feel free to modify the custom-convert.conf file to remove the --disable-audio-cache statement. But please note that I haven't tested this.

I removed the --disable-audio-cache but that did nothing. Then I noticed that, unlike librespot, spotty has an --enable-audio-cache option so I tried that one. The plugin really doesn't want me to use that, so I commented that line. Upon closer examination I saw that --enable-audio-cache isn't actually used, the problem is something else: Even if I configure a cache (by omitting --disable-audio-cache) that cache is never used with spotty, because --single-track initializes its own Session, always with None as cache.

However, it is possible to simply pass cache into spotty::play_track(), use it to create the session and the cache works great. 👍 I would send a PR but I can't fork your fork because I have already forked librespot.

Unfortunately now I still have to build it for ARM (for my Pi).

@michaelherger
Copy link
Owner

michaelherger commented Jan 23, 2024

Oh wow... I'm really sorry for sending you down that rabbit hole! It wasn't my intention to make things that hard. I simply thought I won't care about dealing with the cache size, totally ignoring it had been added along the lines.

So THANK YOU very much for all the work you've done. It would be a pity if that was lost!

You should be able to add another remote, eg. called michaelherger instead of origin. After a git fetch --all you should be able to create a branch for your work, and create a PR (you can define the target repo and branch when submitting the PR)?

@AndreKR
Copy link
Author

AndreKR commented May 9, 2024

Here's a PR for the modifications of the spotty helper: michaelherger/librespot#19

I can try to implement a checkbox in the plugin settings of the Spotty, but I'm really not a Perl guy. Also I'm not sure what's going on here.

@michaelherger
Copy link
Owner

That line was basically to make sure the cache isn't initialized. But you'd want to change that in order to pick up whatever pref you define. Please also double-check the custom-convert file (https://github.com/michaelherger/Spotty-Plugin/blob/479afd1670b7ca0090a25a783b9d25887188af00/custom-convert.conf). It obviously would have to match your code.

@michaelherger
Copy link
Owner

FWIW: Here's the single change I had to apply to make Spotty use the cache

diff --git a/Plugin.pm b/Plugin.pm
index 1f623cd..b72a369 100644
--- a/Plugin.pm
+++ b/Plugin.pm
@@ -265,7 +265,8 @@ sub updateTranscodingTable {
                        $commandTable->{$_} =~ s/\[spotty.*?\]/\[$helper\]/g if $helper;
                        $commandTable->{$_} =~ s/\[spotty.*?\]/\[$helper\]/g if $helper && $canOggDirect;
                        $commandTable->{$_} =~ s/\[spotty.*?\]/\[spotty-ogg\]/g if $commandTable->{$_} =~ /--pass.?through/ && (!$canOggDirect || $canReplayGain);
-                       $commandTable->{$_} =~ s/enable-audio-cache/disable-audio-cache/g;
+                       # $commandTable->{$_} =~ s/enable-audio-cache/disable-audio-cache/g;
+                       $commandTable->{$_} =~ s/disable-audio-cache/enable-audio-cache -M 30M/g;
                        $commandTable->{$_} =~ s/ --enable-volume-normalisation //;
                        $commandTable->{$_} =~ s/( -n )/ --enable-volume-normalisation $1/ if $canReplayGain;
                        $commandTable->{$_} =~ s/( -n )/ --ap-port=12321 $1/ if $forceFallbackAP && $commandTable->{$_} !~ /--ap-port/;

That's obviously just a POC. But I was able to see how it cached files, managed the cache size etc. You'd obviously have to replace that with real code, cache size etc.

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

No branches or pull requests

2 participants