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 a more complex caching mechanism which loads assets concurrently #66

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

xxxserxxx
Copy link
Collaborator

@xxxserxxx xxxserxxx commented Oct 14, 2024

As discussed: this adds a new structure, Queue, which can load queue up loading assets in the background. It replaces the current album art queue, and has been tested against Navidrome and gonic. You can see it at work if you load up the queue with songs from different albums and hold down the scroll key; at times the "logo" will flash for a song, and if you return to the song it'll usually have the art.

You are seeing > second long image load times; I'm seeing sub-second times, even with Navidrome. I am therefore guessing that your server is on the WAN, whereas mine are on my LAN, and the latency differences are network related and not server software related.

I'll be interested to hear how this behaves for you, as I have difficulty triggering slow image loading times, even on albums with large (> 1k×1k) image art files.

In case we need to use the queue for other types of assets (lyrics are in my crosshairs), I made it a generic container.

@xxxserxxx
Copy link
Collaborator Author

The container for the markdownlint job is failing; I haven't changed it, so I'm not sure what's going on there. It's not a linting error (I didn't change the README in this commit), it's an actual failure setting up the workflow.

@spezifisch
Copy link
Owner

The container for the markdownlint job is failing; I haven't changed it, so I'm not sure what's going on there. It's not a linting error (I didn't change the README in this commit), it's an actual failure setting up the workflow.

Looks like Github is having hiccups again. I restarted the job and it ran through without a hitch.

@spezifisch
Copy link
Owner

What milestone do you think this should be?

@xxxserxxx
Copy link
Collaborator Author

What milestone do you think this should be?

I think v1.1.0, where you put it, is fine. It's certainly more complex than a simple in-memory cache; OTOH, it's one of the few files that has 100% code coverage in unit tests.

If we notice memory issues, we can just push 1.1.0 out earlier; this does address the potential ever-growing image queue issue.

xxxserxxx and others added 2 commits October 29, 2024 13:51
… to handle slow asset servers. Abstracted enough to be able to handle caching to disk without having to change users of the Cache.
* Changes program exit function to properly close the asset queue. Closing the queue is not currently _required_, but if we replace the queue with an on-disk queue we may need to clean up resources; this commit ensures that doesn't get missed in the future.

* feat: #67, user can disable song info panel

* feat: #67, updates readme for how to disable the panel
@xxxserxxx xxxserxxx force-pushed the concurrent-album-art branch from 41de24e to 90036e1 Compare October 29, 2024 18:54
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.

[Feature] Pre-fetch album art for items in the queue
2 participants