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

[Redesign] Download orphan tracks with library #734

Merged
merged 5 commits into from
May 18, 2024

Conversation

Komodo5197
Copy link

Download orphan tracks which are not contained in any album when downloading library. Actual functionality not yet tested as I don't have any orphan tracks.

Also forces full metadata refresh during download repair, adds some comments, cleans up unneeded childCount references, fixes showing deleting instead of syncing on subitems on the downloads screen, update playlist image on sync, and properly remove syncFailed state.

@Chaphasilor
Copy link
Collaborator

Whew. That's a lot of improvements!
How much testing would you want me to do?

As for orphaned downloads, I just dumped a few tracks into the root of my library directory and rescanned, in case you want to give it a try yourself.

@Komodo5197
Copy link
Author

The things to verify are just generally verifying that downloading/syncing/deleting are still working normally, verifying that the orphans are downloading now, and checking if updating an image actually works right. I was previously under the impression that changing an image would replace the imageId, but that isn't the case at all, so some verification that they now properly change after a sync would be good. Playlists will update on a quick sync, all other items may take a full sync/downloads repair, depending on the exact dependencies.

@Chaphasilor
Copy link
Collaborator

Alright, I'll give it a test now!

@Chaphasilor
Copy link
Collaborator

Chaphasilor commented May 17, 2024

  • Downloading and syncing orphans works perfectly, no duplicates were created either
  • Playlist properly resync tracks after a restart in online mode
  • Playlists images are properly updated during a regular sync. It actually also worked for artists without forcing a sync or repair, just restarting (with prefer quick syncs enabled). For track image changes, a repair was needed, but that did work fine

I noticed that after deleting an album from the device in offline mode, the albums tab kept loading until I toggled the "fully downloaded" filter. Not sure if that's new or also was the case before. I had marked some tracks of that album to always stay on device, and deleted from the album screen, if it matters

Edit: will the cached album covers also properly update changed images? Given that those are also used in online mode, it would be good if we could make sure they always reflect the latest version and aren't "stale"

@Komodo5197
Copy link
Author

The reason the artist image refreshed was because we were requiring mediaSources/mediaStreams on all downloaded items, but those seem to always be null on collections. I've changed it to only require those on songs.

The music screen locked up because we called pagingController.refresh() on offline deletes instead of the correct _refresh() which updates the deduplication state. This was an existing issue.

The cached covers, and any other downloaded image, will be properly ignored due to the blurHash change of the item. If blurHashes are broken on the server they will still be used, but that doesn't seem avoidable and is not a new issue.

@Chaphasilor
Copy link
Collaborator

Okay, perfect! So the issue with the artists and other types was that we always tried to re-fetch the BaseItemDto because the existing ones were missing the media sources/streams, and we want to avoid that? Because refreshing the image wouldn't be so bad after all (but it's fine that it doesn't do that anymore now) ^^

Thanks for the PR!

@Chaphasilor Chaphasilor merged commit 8dd9d6a into jmshrv:desktop-beta May 18, 2024
4 checks passed
@Komodo5197
Copy link
Author

Yeah, refreshing the image would be nice, but the amount of extra requests can really pile up. And we can't do it for albums due to skipping them completely when quicksyncing, and I believe quicksyncing would also skip any info-only artist due to skipping the tracks that point to them, so trying to refresh things like artists every time would be very inconsistent and doesn't seem like a great idea.

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