MissingCore / Music

A Nothing inspired local music player.
GNU Affero General Public License v3.0
216 stars 13 forks source link

chore: Updated saving strategy #54

Closed cyanChill closed 3 months ago

cyanChill commented 3 months ago

Why

With the changing of the package used for getting the metadata of tracks, I wonder if we could reduce the time it takes for saving. The idea I've came up with was to do a "sparse save" of the tracks, saving only the minimum amount of data needed for a Track as getting the list of tracks on your device is pretty fast. We then get & save the metadata of the track in the background. Although it did cut down the time by a decent chunk, it led to the laggy UI issue seen in the past when we tried saving artwork in the background, but worse.

Since this logic will only happen when the app discovers new tracks, I decided it would be fine to have this process take a while as it's a "setup process" in a sense. With the new strategy, we implemented an idea of "batching", in which, we decided to integrate with the original logic. This actually fixes an issue with the original logic, in which, it was an all-or-nothing strategy in which we save all the tracks in one go. If the app crashed during this process, essentially, the user will never get in the app as the app might crash in the same place in successive runs.

To improved the experience while on the loading screen, I've added some progress indicator to help the user understand that the app is actually doing something. In addition, we also added a modal after we get pass the loading screen to tell the user how we save artwork for the tracks as some were thinking the app was broken due to the track artwork not being visible.

Some other things changed/fixed:

[!IMPORTANT]
It seems that the generation of the file tree for the "Folder" feature is becoming a bottleneck, so I'm probably going to fix that next.

How

With users with large amount of media, there's a chance where the app might crash as we overloaded the app with promises (when we do a Promise.all() with more than 500 promises, we get the following warning: WARN Excessive number of pending callbacks). To counter-act this, we implemented batching (ie: passing fewer promises into Promise.all(), but getting through all of them via a for-loop).

Test Plan

Mainly, we're making sure that we don't emit the WARN Excessive number of pending callbacks, display informative information about the current saving status on the loading screen, and render the explanation modal when we open the app for the first time

Checklist