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

Feature: Local music library #1479

Merged
merged 11 commits into from
May 23, 2024

Conversation

bleonard252
Copy link
Contributor

@bleonard252 bleonard252 commented May 5, 2024

This PR introduces a new setting that allows you to set zero or more "local libraries" from a directory. These do not get modified in any way, not even created if they don't exist (they're just not loaded, in that case). Closes #595 (which seems to already be supported, actually).

This is only used right now in the "Local tracks" feature. Playback, queuing, removing libraries, and album art seem to work automatically, but MPRIS metadata does not, and it uses fallback data if you attempt to look at the track, album, or artist pages for any local track, but this seems to be an issue with the existing Downloads folder too.

The PR also changes some dependencies, which I had to do to get it to build and run on my machine. Feel free to revert it, or have me revert it, if you feel it necessary.

Screenshots

Screenshot of Spotube on the Settings page, focusing on the new item under Downloads labeled Local libraries
Screenshot of Spotube on the Local libraries settings sub-page, listing two entries

@KRTirtho
Copy link
Owner

KRTirtho commented May 6, 2024

Wow, that's really great work. Thanks for the contribution.

But I'd like to suggest some changes.

Instead of having the Local Library section in Settings, we can just show the Local Folders as grid of cards/list in the "Library" > "Local Tracks" Tab. The tracks for each folder will be accessible only on that Folder's specific page.
We can have the "add local library" button there as well to add the folders users want to.

This will also need the "Local Tracks" tab to be renamed as just "Local" meaning local library.

@bleonard252
Copy link
Contributor Author

I'm not completely sure what a grid view would look like for a local tracks list. If you are thinking about album/artist grouping, that's a taller task, and I think it would be better to integrate it into the local fields anyway, which is definitely outside the scope of this PR. (I'm thinking about setting up local track integration via Spotify ID in a future PR.)

I think I might try integrating the settings into the "local tracks" list, by grouping by source and putting the "add to library" button there, as you suggested.

@bleonard252
Copy link
Contributor Author

One interesting side effect of this new listing mechanism is that, now, if you click play on a track (with a clear queue, not sure if that matters), it plays all the tracks from that library source only.

A screenshot of Spotube showing some tracks grouped by library source.

@KRTirtho
Copy link
Owner

KRTirtho commented May 9, 2024

Great work. This is close to what I meant and imagined.
But, instead of showing all the tracks of all the libraries together,
we can show just the Library names in the "Local Tracks" tab. It can be a ListView or a GridView of them. Kinda similar to Playlist. But, we'll utilize the FileSystem to manage/store it.
We can move the playbuttons, sort button etc. playback related actions to specific Library view page. The Library view page will List all the Tracks contained by it

One interesting side effect of this new listing mechanism is that, now, if you click play on a track (with a clear queue, not sure if that matters), it plays all the tracks from that library source only.

Do you mean it doesn't play any other online tracks if we add them to the Queue later?

@KRTirtho KRTirtho changed the base branch from master to dev May 9, 2024 11:05
@KRTirtho
Copy link
Owner

KRTirtho commented May 9, 2024

We actually only merge PRs in to the dev branch. But, you were targetting master branch. So some conflicts arose after changing the target branch

Do debugging/testing/build etc then submit to us with PR against the development branch (dev) & we'll review your code

@bleonard252
Copy link
Contributor Author

bleonard252 commented May 9, 2024

Do you mean it doesn't play any other online tracks if we add them to the Queue later?

It does. It doesn't add the other listed sources to the queue, though. I guess if I rework it into multiple pages, this becomes intended behavior :)

We actually only merge PRs in to the dev branch

Oops I must have missed that. That makes sense. I'll rebase and solve the conflict, and then start trying to rework it into multiple pages, like you asked. I still don't think a grid view makes sense, since I'd have to figure out what image shows up on it, so it'll still be a list, but I'll revert it back to a simple ListView and just list the libraries there. (This will look silly to anyone with just Downloads set, by the way.)

This folder just doesn't get downloaded to.
I think I'm going to rework it so that it can be multiple folders,
but I'm going to commit my progress so far anyway.

Signed-off-by: Blake Leonard <me@blakes.dev>
I'm not sure if this breaks CI or something, but I couldn't build
it locally to test my changes, so I made these changes and it
builds again.

Signed-off-by: Blake Leonard <me@blakes.dev>
If you used a previous commit from this branch, this is a breaking
change, because it changes the type of a configuration field. but
since this is still in development, it should be fine.

Signed-off-by: Blake Leonard <me@blakes.dev>
This also refactors the list to use slivers instead. That's the
easiest way to have multiple scrolling lists here...

The console keeps getting spammed with some intermediate layout
error but I can't hold it long enough to figure out what's causing
it.

Signed-off-by: Blake Leonard <me@blakes.dev>
Signed-off-by: Blake Leonard <me@blakes.dev>
Signed-off-by: Blake Leonard <me@blakes.dev>
Not sure if this would be the recommended way to do it...

Signed-off-by: Blake Leonard <me@blakes.dev>
Signed-off-by: Blake Leonard <me@blakes.dev>
Signed-off-by: Blake Leonard <me@blakes.dev>
Signed-off-by: Blake Leonard <me@blakes.dev>
Signed-off-by: Blake Leonard <me@blakes.dev>
@bleonard252
Copy link
Contributor Author

The force-push was from rebasing the branch upon dev. Here's what it looks like now:
image
image

@KRTirtho
Copy link
Owner

This is close to perfect. Later, I guess we can use the Album art of the first 4 songs of the library as the image for the GridView's items.

For now I think is acceptable. I can always redo the UI for better UX later.

@KRTirtho
Copy link
Owner

Sorry for the delay, I thought I merged this PR.

@KRTirtho KRTirtho merged commit 22caa81 into KRTirtho:dev May 23, 2024
1 check failed
KRTirtho added a commit that referenced this pull request Jun 3, 2024
* chore: fix analyzer issues

* fix(updater): dead link (#1408)

* docs: broken link in README.md (fixes #1310) (#1311)

* docs: remove appimage link in readme #1082 (#1171)

* Updating Readme according to #1082

Updating Readme according to #1082

* Added explanation

The explanation is now given and the expression is more formal and explanatory, instead of just linking the issue.

* Update use_update_checker.dart

---------

Co-authored-by: MerkomassDev <70111455+MerkomassDev@users.noreply.github.com>
Co-authored-by: Karim <37943746+ksaadDE@users.noreply.github.com>
Co-authored-by: Kingkor Roy Tirtho <krtirtho@gmail.com>

* fix(linux): tray icon not showing #541

upgrade old packages

* fix(search): load more button not working #1417

* fix: spotify friends and user profile icon (mobile) showing when not authenticated #1410

* chore: add docker and m1 based linux arm build

* cd: fix sed failing us

* cd: use docker cask

* fix: windows SSL Certificate error breaking login #905 (#1474)

* fix: certificate error by using custom ssl certificate

* Cd/docker linux ar (#1468)

* cd: use docker buildx

* cd: use linux host for linux arm instead of macos m1

m1 doesn't support nested virtualization. (Apple truly sucks)

* cd: don't specify arch in Dockerfile

* cd: use custom Dockerfile from ubuntu instead of flutter image

* cd: add setup java for android

* cd: add flutter distributor pre-built docker image for arm

* cd: save me from this cursed arm build

* cd: ??

* cd: ??

* cd: use docker build

* fix: windows SSL Exception for Signing in

* refactor: extract update checker as a basic function instead of a hook

* cd: fix windows build error due to nightly version format

* cd: fix github versioning scheme

* chore:  remove assets/ca entry in pubspec.yaml

* fix(macos): Logs directory not created by default #1353

* refactor: Dart based Github Workflow CLI (#1490)

* feat: add build dart script for windows

* feat: add android build support

* feat: add linux build support

* feat: add macos build support

* feat: add ios build support

* feat: add deps install command and workflow file

* cd: what?

* cd: what?

* cd: what?

* cd: update workflow inputs

* cd: replace release binary

* cd: run flutter pub get

* cd: use dpkg zstd instead of xz, windows disable innoInstall, fix channel enum.name and reset pubspec after changing build no for nightly

* cd: fix tar copy path

* cd: fix copy linux command

* cd: fix windows inno depend and fix android aab path

* cd: idk

* cd: linux why???

* cd: windows choco copy failed

* cd: use dart tar archive for creating tar file

* cd: fix linux file copy error

* cd: use tar command directly

* feat: add linux_arm platform

* cd: add linux_arm platform

* cd: don't know what?

* feat: notification about nightly channel update

* chore: fix some errors parsing nightly version info

* refactor: move dart scripts as commands under CLI

* chore: add translated message command to command list

* feat(translations): add Basque translation (#1493)

* added Basque translation

* chore: fix country codes and language native name

---------

Co-authored-by: Kingkor Roy Tirtho <krtirtho@gmail.com>

* feat(translations): add georgian language (#1450)

* feat: add georgian language

* feat: translate more georgian words

* feat(translations): add Finnish translations (#1449)

* docs: broken link in README.md (fixes #1310) (#1311)

* docs: remove appimage link in readme #1082 (#1171)

* Updating Readme according to #1082

Updating Readme according to #1082

* Added explanation

The explanation is now given and the expression is more formal and explanatory, instead of just linking the issue.

* added finnish translation

* chore: fix arb syntax errors and language in l10n entries

---------

Co-authored-by: MerkomassDev <70111455+MerkomassDev@users.noreply.github.com>
Co-authored-by: Karim <37943746+ksaadDE@users.noreply.github.com>
Co-authored-by: Kingkor Roy Tirtho <krtirtho@gmail.com>
Co-authored-by: Onni Nevala <nevalaonni@gmail.com>

* feat(translations): add Indonesian translation (#1426)

* docs: broken link in README.md (fixes #1310) (#1311)

* docs: remove appimage link in readme #1082 (#1171)

* Updating Readme according to #1082

Updating Readme according to #1082

* Added explanation

The explanation is now given and the expression is more formal and explanatory, instead of just linking the issue.

* Add Indonesia translation

---------

Co-authored-by: MerkomassDev <70111455+MerkomassDev@users.noreply.github.com>
Co-authored-by: Karim <37943746+ksaadDE@users.noreply.github.com>
Co-authored-by: Kingkor Roy Tirtho <krtirtho@gmail.com>

* feat(translations): Improve tr locales (#1419)

* docs: broken link in README.md (fixes #1310) (#1311)

* docs: remove appimage link in readme #1082 (#1171)

* Updating Readme according to #1082

Updating Readme according to #1082

* Added explanation

The explanation is now given and the expression is more formal and explanatory, instead of just linking the issue.

* Improve tr locales

---------

Co-authored-by: MerkomassDev <70111455+MerkomassDev@users.noreply.github.com>
Co-authored-by: Karim <37943746+ksaadDE@users.noreply.github.com>
Co-authored-by: Kingkor Roy Tirtho <krtirtho@gmail.com>

* feat(player): add volume slider floating label showing percentage (#1445)

* docs: broken link in README.md (fixes #1310) (#1311)

* docs: remove appimage link in readme #1082 (#1171)

* Updating Readme according to #1082

Updating Readme according to #1082

* Added explanation

The explanation is now given and the expression is more formal and explanatory, instead of just linking the issue.

* add volume level tooltip in volume_slider

---------

Co-authored-by: MerkomassDev <70111455+MerkomassDev@users.noreply.github.com>
Co-authored-by: Karim <37943746+ksaadDE@users.noreply.github.com>
Co-authored-by: Kingkor Roy Tirtho <krtirtho@gmail.com>

* fix: fallback to LRCLIB when lyrics line less than 6 lines #1461

* feat: Local music library (#1479)

* feat: add one additional library folder

This folder just doesn't get downloaded to.
I think I'm going to rework it so that it can be multiple folders,
but I'm going to commit my progress so far anyway.

Signed-off-by: Blake Leonard <me@blakes.dev>

* chore: update dependencies so that it builds

I'm not sure if this breaks CI or something, but I couldn't build
it locally to test my changes, so I made these changes and it
builds again.

Signed-off-by: Blake Leonard <me@blakes.dev>

* feat: index multiple folders of local music

If you used a previous commit from this branch, this is a breaking
change, because it changes the type of a configuration field. but
since this is still in development, it should be fine.

Signed-off-by: Blake Leonard <me@blakes.dev>

* refactor: manage local library in local tracks tab

This also refactors the list to use slivers instead. That's the
easiest way to have multiple scrolling lists here...

The console keeps getting spammed with some intermediate layout
error but I can't hold it long enough to figure out what's causing
it.

Signed-off-by: Blake Leonard <me@blakes.dev>

* refactor: use folder add/remove icons in library

Signed-off-by: Blake Leonard <me@blakes.dev>

* refactor: remove redundant settings page

Signed-off-by: Blake Leonard <me@blakes.dev>

* refactor: rename "Local Tracks" to just "Local"

Not sure if this would be the recommended way to do it...

Signed-off-by: Blake Leonard <me@blakes.dev>

* fix: console spam about useless Expanded

Signed-off-by: Blake Leonard <me@blakes.dev>

* chore: remove completed TODO

Signed-off-by: Blake Leonard <me@blakes.dev>

* chore: use new Platform constants; regenerate plugins

Signed-off-by: Blake Leonard <me@blakes.dev>

* refactor: put local libraries on separate pages

Signed-off-by: Blake Leonard <me@blakes.dev>

---------

Signed-off-by: Blake Leonard <me@blakes.dev>

* fix: local track not showing up in queue

* feat: local library folder cards

* feat: personalized stats based on local music history (#1522)

* feat: add playback history provider

* feat: implement recently played section

* refactor: use route names

* feat: add stats summary and top tracks/artists/albums

* feat: add top date based filtering

* feat: add stream money calculation

* refactor: place search in mobile navbar and settings in home appbar

* feat: add individual minutes and streams page

* feat(stats): add individual minutes and streams page

* chore: default period to 1 month

* feat: add text to explain user how hypothetical fees are calculated

* chore: ensure usage of route names instead of direct paths

* cd: add cache key

* cd: remove media_kit_event_loop from git

* fix: some text are garbled in different parts of the app #1463 #1505

* refactor: use replace http with dio and use it as the default

* cd: use dio in cli as well

* chore: fix home feed not showing up

* chore: downloaded tracks folder not opening

* feat: play initially available tracks of playlist/album immediately and fetch rest in background #670

* feat: upgrade to Flutter 3.22.0

* refactor: migrate deprecated warnings

* fix(playback): skipping tracks with unplayable sources instead of falling back #1492

* chore: migrate android gradle to declarative config syntax

* chore: disable impeller for now

* fix(windows): installer tries to install in current directory

* chore: upgrade deps and appbar bg fix

* chore: podspec update

* chore: bump version and generate changelogs

---------

Signed-off-by: Blake Leonard <me@blakes.dev>
Co-authored-by: Kshamendra <github@ghoulcloud.slmail.me>
Co-authored-by: MerkomassDev <70111455+MerkomassDev@users.noreply.github.com>
Co-authored-by: Karim <37943746+ksaadDE@users.noreply.github.com>
Co-authored-by: Josu Igoa <josuigoa@ni.eus>
Co-authored-by: Omari Sopromadze <omari.sopromadze@gmail.com>
Co-authored-by: ctih <78687256+ctih1@users.noreply.github.com>
Co-authored-by: Onni Nevala <nevalaonni@gmail.com>
Co-authored-by: Yusril Rapsanjani <yusriltakeuchi@gmail.com>
Co-authored-by: W͏ I͏ N͏ Z͏ O͏ R͏ T͏ <75412448+mikropsoft@users.noreply.github.com>
Co-authored-by: Akash Pattnaik <akashjio66666@gmail.com>
Co-authored-by: Blake Leonard <blake@1024256.xyz>
@vico93
Copy link

vico93 commented Jun 3, 2024

Well, just saw this PR just now - after reinstalling 3.7.0 - don't know if this PR here (or another brand new issue) would be the proper place, but i just want to throw my two cents as the final user:

  • First of all, thanks for this effort to bring local playback as a feature. I'm in this cruzade to find a better alternative to the good ol'Windows Media Player (Legacy) since i migrated to Windows 10 and started seeing the growing issues with WMP from all those years (things like Discord integration, possibility to integrate with online libraries like Spotube does).
  • Well, the first thing i noticed was that, apparently, you guys decided to keep the local files separate of the online stuff in the UI; although i understand that comparing the metadata of each .mp3/.m4a/.flac/etc file locally to the online database to make them appear as one would be hellish, i was aiming to see really an "integrated" flow, where when i try to reproduce a song it would first check if a file on the disk has the same metadata and then play it instead of downloading from the online source - or appear as unavailable if the user is offline, that is the default behavior for the official Spotify app. From that i understood, this isn't the behavior implemented, resorting to us to search around the desired song in a linear list of songs (that isn't even separated by albums and such). This really sucks, to be honest.
  • Also, at least on my end (Windows 11 x64, 32GB of RAM), trying to play a local song freezes the entire app for a couple of time, and when the song starts to play the app gets very slow, a couple of times i tried i needed to kill the Spotube process on Task Manager.

if i was to suggest a different way to achieve that "seamless" local file seeking, would be actually adding a separate option on the audio source options (or even a checkbox immediately below) with something in the lines of "prioritize song files already existing in the local Music folder". When adding the local Music folder spotube should scan the folder (and its subdirectories, that was the missing thing from the already extant Downloads folder that made me to create #595) and create a internal "database" containing the song metadata and the file path on disk. And when the user tries to play a song, it would first seek if something with that metadata exists on the local database and, if positive, would play it instead of trying to resort to Youtube/Piped/Jiosaavn.

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.

Support for local library
3 participants