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

Add --disable-audio-cache startup parameter #204

Merged
merged 4 commits into from
Jun 30, 2017

Conversation

michaelherger
Copy link
Contributor

Disable caching of downloaded audio files at runtime. Comes in handy when running librespot on a small device with SD card or other small storage.

Disable caching of downloaded audio files at runtime. Comes in handy when running librespot on a small device with SD card or other small storage.
@awiouy
Copy link
Contributor

awiouy commented Jun 20, 2017

Thanks! I am using this now, and I hope it will make it upstream.

@sashahilton00
Copy link
Contributor

Using this now as well, +1 for upstream :)

@plietar
Copy link
Owner

plietar commented Jun 29, 2017

Thanks @michaelherger

Could you move the logic to the Cache::save_file function ?

@mherger
Copy link

mherger commented Jun 29, 2017

I put it in the player code as I don't want to disable all (potential) file caching. It's really about caching the audio data. There could be other code using the cache. (FWICT there's none right now, is there?)

@plietar
Copy link
Owner

plietar commented Jun 29, 2017

Yes save_file is only for audio data, so it is fine to move it there

@michaelherger
Copy link
Contributor Author

Ok, will update my change.

(argh... at some point I have to have a better strategy to separate my pro from my personal GitHub account...)

@michaelherger
Copy link
Contributor Author

Hmm... credentials is using the same cache object all over the place. Not initializing cache for files only, but use it for credentials looks like a much more complicated change, dealing with multiple cache instances etc. Am I totally wrong?

@michaelherger
Copy link
Contributor Author

BTW: is cache/default_cache even used anywhere?

@plietar
Copy link
Owner

plietar commented Jun 29, 2017

My point was to make use_audio_cache a field of Cache rather than Config, and move the if use_audio_cache test into the save_file method.

default_cache is just some leftover from a previous implementation, I forgot to delete it. You can do it if you want

@michaelherger
Copy link
Contributor Author

Take 2.

src/main.rs Outdated
@@ -135,7 +136,7 @@ fn setup(args: &[String]) -> Setup {
let device_id = librespot::session::device_id(&name);

let cache = matches.opt_str("c").map(|cache_location| {
Cache::new(PathBuf::from(cache_location))
Cache::new(PathBuf::from(cache_location), !matches.opt_present("disable-audio-cache"))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a nit, could you extract !matches.opt_present("disable-audio-cache") to a separate variable ?

@plietar plietar merged commit 67deb07 into plietar:master Jun 30, 2017
@plietar
Copy link
Owner

plietar commented Jun 30, 2017

Awesome thanks !

olvidalo pushed a commit to olvidalo/librespot that referenced this pull request Nov 24, 2017
Disable caching of downloaded audio files at runtime. Comes in handy when running librespot on a small device with SD card or other small storage.
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.

5 participants