Repository navigation
Media library: list albums with a grouped tag query when mtime isn't needed - #670
Open
Sciencentistguy wants to merge 1 commit into
Open
Sciencentistguy wants to merge 1 commit into
Sciencentistguy wants to merge 1 commit into
Conversation
…eded In the two column and album-only modes the whole database was fetched with listallinfo and reduced to (tag, album, date) tuples on the client. With ~48k songs this takes about 0.85s, and the response is large enough to overflow mpd's default max_output_buffer_size when the mode is toggled repeatedly. Ask mpd for the same tuples with `list album group <tag> [group date]` instead, which takes ~15ms. The per-song listing is still used when media_library_sort_by_mtime is enabled, as modification times aren't available from the grouped query. This requires libmpdclient >= 2.12 and MPD >= 0.19 for the group argument.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In the two column and album-only media library modes, building the album list takes about 0.8s on my library (~48k songs), as they build their album list by fetching the whole database with
listallinfoand reducing it on the client.This changes it to request the tuples directly instead:
What changed
Connection::GetListGrouped()that sendslist <tag> group <tag>....MediaLibrary::update()uses it for the two column and album-only modes. The old code is moved unchanged intolistAlbumsWithMTime().configure.acnow needs libmpdclient >= 2.12 (formpd_search_add_group_tag). Thegroupargument also needs MPD >= 0.19.Behaviour notes
media_library_sort_by_mtime = yesstill useslistallinfo, since mtimes aren't available from the grouped query. That mode isn't any faster.media_library_albums_split_by_dateis on. One difference is that a song with several album or date values now gives one entry per combination, where before only the first value was used.Testing
I built a copy that computed both lists on every load and compared them. The albums were identical for two column and album-only mode, with
media_library_albums_split_by_dateon and off. I haven't tried MPD older than 0.19, or a library with songs that lack the primary tag (mine has none).Timings
I benchmarked the two code paths with hyperfine, using a small standalone program that sends the same command and builds the same
std::map(default settings, both return 10,488 entries). Times include process start and connecting.listallinfo(before)list album group artist group date(after)MPD 0.24.15 over a local unix socket, libmpdclient 2.24, 47,645 songs. Inside ncmpcpp itself I measured about 15ms against 860ms.
I used Claude Code to help with this change.