search

catharsis / spotifile

FUSE file system for Spotify
BSD 3-Clause "New" or "Revised" License
149 stars 7 forks source link

Add new option for setting bitrate preset #28

Closed raoulh closed 9 years ago

raoulh commented 9 years ago

bitrate_preset=STRING One of the following value: (96kbps/160kbps/320kbps)

This is the correct pull request fir bitrate option. Sorry for the wrong previous one. I also changed the option type to be a string with the real bitrate values as you requested.

catharsis commented 9 years ago

Cool! I'll have a better look later today, but one thing I noticed is that you're calling libspotify session functions (sp_session_preferred_bitrate) without holding the session mutex (g_spotify_api_mutex). That's not allowed, since it might cause races & crashes and other fun things (more here https://developer.spotify.com/technologies/libspotify/faq/).

There's a bunch of (terribly poorly documented) macros defined towards the top of spfs_spotify.c to help deal with these intricacies - you might be able to use one of those to generate a wrapper for the function for you.

raoulh commented 9 years ago

I was thinking that it was not needed because the bitrate function is called just after the session init

catharsis commented 9 years ago

I think it'd be wiser to play it safe, regardless. It's such an easy bug to avoid (regardless if you use one of the macros or lock/unlock the mutex yourself) that even if it only triggers once every millionth execution it's worth it, imo.

raoulh commented 9 years ago

New PR with updated code as requested