Skip to content

Conversation

AndreKR
Copy link

@AndreKR AndreKR commented May 9, 2024

This enables optional audio caching in Spotty.

Spotty has two command line options, --disable-audio-cache and --enable-audio-cache. --disable-audio-cache is a no-op, provided only for compatibility with librespot. --enable-audio-cache also did nothing until now. This PR changes that, it will now be honored and initializes the audio cache. The cache is then used in the same way as if in librespot the --disable-audio-cache option wasn't given.

This PR also restores the --cache-size-limit command line option that limits the total size of the cache by deleting older files whenever a new cache file is added.

@michaelherger
Copy link
Owner

Thanks for this PR! I understand this is mostly a restore/backport of the code upstream? Could you please also restore https://github.com/librespot-org/librespot/blob/dev/src/main.rs#L1063-L1068?

@AndreKR
Copy link
Author

AndreKR commented May 12, 2024

Thanks for this PR! I understand this is mostly a restore/backport of the code upstream? Could you please also restore https://github.com/librespot-org/librespot/blob/dev/src/main.rs#L1063-L1068?

Done.

@michaelherger
Copy link
Owner

Looking into this again I fear more work is required: spotty currently doesn't support the SYSTEM_CACHE. Therefore the librespot configuration data (credentials) are stored in the same place as the cached files. Which means that each configured account would have its own file cache alongside the config files. This wouldn't make sense in the popular family account setups, as files might become duplicated, and a single max size setting in the Spotty plugin wouldn't be good enough, as that limit would be available to each account individually. In my case that means I'd have to take care to only grant 25% of the available disk space, as we have four accounts.

Could you please look into restoring the SYSTEM_CACHE feature, too? Then use this on the plugin side for what we have today, and CACHE for the file cache? Thanks a ton!

@AndreKR
Copy link
Author

AndreKR commented May 15, 2024

Do you think a shared audio cache is actually useful/necessary or did you just ask because librespot has it?

Because personally I don't think a per-account cache is that much of an issue. If you use 1 GB of cache, like the official Spotify client, that would be 4 GB of cache. Not that much for a media server, even if it's a Pi. It becomes even less of an issue if you reduce the cache size. Currently we have no caching and it's mostly fine. I wanted the cache feature because I found it wasteful and annoying that it would stream the files again when I play the same playlist over and over again, 100 MB cache would already be enough to prevent that.

In any case, I'm happy to port the shared audio cache if you think it's a desirable feature.

@michaelherger
Copy link
Owner

michaelherger commented May 15, 2024

Let's think about the UX: are we going to allow the user to define the amount of disk space to use? Or will this be a hard-coded value and a user selectable toggle on/off? That's probably more important to understand whether the question about shared caches or not.

In any case we'd have to find a way to get the free disk space. I haven't found an OS agnostic way to get free disk space information in Perl. We might have to write something ourselves to make sure we don't use more than is available. People will be asking "how much are you using?". What will be the response? Even if we hard-code to a defensive 1GB, this could be too much for some installations (some people still believe formatting only a 2GB partition on their 64GB SD card will make things more stable).

Having a shared cache would simplify this calculation. Because we'd know 1GB is 1GB. But with individual caches it's might x times 1GB.

Edit: just found https://metacpan.org/pod/Filesys::DfPortable - but it would need to be compiled for each supported platform... Maybe shelling out to run df or using https://metacpan.org/pod/Win32::DriveInfo on Windows might get us there without compiling modules.

@AndreKR
Copy link
Author

AndreKR commented May 15, 2024

So the UI I had in mind would look something like this:

image

Informing the user about the size of their disk (or --storage-opt size of the Docker container) is something I would consider completely outside the scope of Spotty (or LMS for that matter).

I did include the path to the cache in my mockup because that would be super helpful to set up appropriate volumes.

That made me realize another reason to separate the credential cache from the audio cache though: You want the credential cache to be persistent, but you might want to put the audio cache on a ramdisk to reduce disk churn. So yeah, I'll go ahead and split up the paths.

@michaelherger
Copy link
Owner

michaelherger commented May 16, 2024

That setting actually looks amazingly simple and instructive! I almost don't know what I was thinking 😁.

Except that I don't like the line with the cache folder. Could this be wrapped in a "info" bubble popup?

Splitting the paths comes with a draw back: if you wanted to make this user configurable, there would be yet another potentially confusing preference on an already pretty busy and confusing screen...

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.

2 participants