slimserver
slimserver copied to clipboard
LMS-Community
Import "WORK" tag into the database and provide browse by Work menu options
This is an initial prototype, adding functionality to allow users to display their music library by "Work". This is mostly for classical music, where the standard album->track hierarchy is inadequate: classical music requires album->work->track because very many classical releases contain multiple works, often by multiple composers.
At the moment, it is limited to displaying either works, or works sorted by composer, the results being simple links to the album(s) containing the works. I want to add search capabilities and playlist creation, but I wanted to provide a early view of what I am doing, to make sure I'm going about this the right way, following standards, etc.
Having imported the tag WORK into the database, I found that it became available automatically for use in the Title Format settings (interface tab), which will be very useful, whether browsing by work or not.
To stress, this is very much a prototype: for example, I've only added the import of the WORK tag for flac files, for now.
For anyone interested in trying it out, I have attached test data here (the tracks within have been cut to 15 seconds each to avoid copyright issues). There are 2 zip files in order to stay within github's 25Mb per file limit.
So I'm trying to understand... The "Works" work is supposed to do something similar to an album's tracks as did the "release type" work to albums: group items into different sub-sets than the simple album/tracks. Is this correct so far?
Would you also see a pure Work view where different recordings of the same work would be grouped under the work in a menu?
I see you're using a different approach than I did: I extended the albums query to return the necessary information, while you added a new works query. Therefore I probably implemented more of the logic in the browsing code. Couldn't you make the Work title an additional value in the response to the titles query, then do the grouping in the navigation code? One huge advantage would be that a frontend (Material) could decide based on the data returned whether it wanted or would be able to group by work, rather than have to figure out whether the LMS version did actually support the feature.
Some comments about the code itself: I created a new BrowseLibrary/Releases.pm package. BrowseLibrary.pm has become too big and hard to follow. The same probably applies to Queries.pm, but I'm not sure how to split up that one... But please move your Works code to a sub-module, too.
And the schema change. I'm torn. I know I didn't normalize the release types, I simply store the literals in a new column. Yet I believe this is wrong in your case... my (lame) excuse is that release types usually are short, single words, and there probably would only be a hand full of them. But for works I'd imagine those titles could be pretty long, and there could be hundreds of them, right? Dealing with an additional table obviously would come at a complexity cost.
So I'm trying to understand... The "Works" work is supposed to do something similar to an album's tracks as did the "release type" work to albums: group items into different sub-sets than the simple album/tracks. Is this correct so far?
Yes, it's quite similar. Slightly more complicated as WORK is only completely unambiguous when qualified by Composer.
Would you also see a pure Work view where different recordings of the same work would be grouped under the work in a menu?
Possibly sitting under Composer, because of the above.
I see you're using a different approach than I did: I extended the albums query to return the necessary information, while you added a new works query. Therefore I probably implemented more of the logic in the browsing code. Couldn't you make the Work title an additional value in the response to the titles query, then do the grouping in the navigation code?...
OK, I'll study what you did for Release Types.
Some comments about the code itself: I created a new BrowseLibrary/Releases.pm package. BrowseLibrary.pm has become too big and hard to follow. The same probably applies to Queries.pm, but I'm not sure how to split up that one... But please move your Works code to a sub-module, too.
I did see the new package. I saw the comment in there about not being able to use _generic. My approach does currently use _generic. Is this linked to the "different approach" you suggest above?
And the schema change. I'm torn. I know I didn't normalize the release types, I simply store the literals in a new column. Yet I believe this is wrong in your case... my (lame) excuse is that release types usually are short, single words, and there probably would only be a hand full of them. But for works I'd imagine those titles could be pretty long, and there could be hundreds of them, right? Dealing with an additional table obviously would come at a complexity cost.
Because Work is owned by Composer, the work table would need to be id,contributor,work and then we'd have a track_work cross-reference table. I'm happy (actually happier) with this approach. I only did it the way I did so I could first concentrate on proving the feasibility of the browse/query changes I wanted to introduce.
darrell-k: I really applaud your efforts on this. My own very lame approach at grouping works has been to simply prepend two non-breaking space characters to the titles of movements 2 through nn of multi-movement pieces when tagging. That serves as a visual guide when browsing, but is of limited utility when appending works to playlists, etc. What you're doing sounds very promising.
On Thu, Oct 26, 2023 at 6:39 AM darrell-k @.***> wrote:
So I'm trying to understand... The "Works" work is supposed to do something similar to an album's tracks as did the "release type" work to albums: group items into different sub-sets than the simple album/tracks. Is this correct so far?
Yes, it's quite similar. Slightly more complicated as WORK is only completely unambiguous when qualified by Composer.
Would you also see a pure Work view where different recordings of the same work would be grouped under the work in a menu?
Possibly sitting under Composer, because of the above.
I see you're using a different approach than I did: I extended the albums query to return the necessary information, while you added a new works query. Therefore I probably implemented more of the logic in the browsing code. Couldn't you make the Work title an additional value in the response to the titles query, then do the grouping in the navigation code?...
OK, I'll study what you did for Release Types.
Some comments about the code itself: I created a new BrowseLibrary/Releases.pm package. BrowseLibrary.pm has become too big and hard to follow. The same probably applies to Queries.pm, but I'm not sure how to split up that one... But please move your Works code to a sub-module, too.
I did see the new package. I saw the comment in there about not being able to use _generic. My approach does currently use _generic. Is this linked to the "different approach" you suggest above?
And the schema change. I'm torn. I know I didn't normalize the release types, I simply store the literals in a new column. Yet I believe this is wrong in your case... my (lame) excuse is that release types usually are short, single words, and there probably would only be a hand full of them. But for works I'd imagine those titles could be pretty long, and there could be hundreds of them, right? Dealing with an additional table obviously would come at a complexity cost.
Because Work is owned by Composer, the work table would need to be id,contributor,work and then we'd have a track_work cross-reference table. I'm happy (actually happier) with this approach. I only did it the way I did so I could first concentrate on proving the feasibility of the browse/query changes I wanted to introduce.
— Reply to this email directly, view it on GitHub https://github.com/Logitech/slimserver/pull/930#issuecomment-1781041714, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACQJ44Z24HI7K4EYZ4AKPM3YBJKX5AVCNFSM6AAAAAA6PZAGQSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOBRGA2DCNZRGQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>
I'm still trying to understand... the main way to get to works would be through a Composers menu only? Would the Works view replace the Albums view for Composers? And only for composers?
At present I've added two options to the top level, both leading to a list of all works, one sorted by work within composer, the other by composer within work.
I think the work within composer option should maybe be inside the composer menu (as enabled with Additional Browse Modes), but not replacing the albums view - perhaps an additional entry of "works" before the composer's album list?
But there should still be a Works option, to show all works, at the top level. I also want to integrate works into the search.
But this is all UI stuff, and I'm concentrating at present on getting the plumbing in place. Once that's done, we can play with the exact layout and hierarchy in the UI.
I actually believe it's important to understand the UX to define what plumbing is needed. Eg. I'm pretty sure there's no need for a worksbycomposer request (is probably an accidental commit). And should the Works work be integrated with the new release type based browsing menu? Otherwise we might end up with another level of submenu below the artist. And beneath that would be the releases. Artist/Albums/Singles and Artist/Works, rather than Artist/Singles and Artist/Works.
Integrating with release type is an interesting idea: let me think it through.
By plumbing, I meant rewriting the components according to your previous suggestions. Of course, where we then plumb these in is very much dependent on the UI choices.
On the worksbycomposer request, I added this because there seemed to be caching problems otherwise: subsequent calls to the same request took no notice of the changed sorting requested and didn't rerun the SQL query.
I would appreciate some help here: I'm converting my code to use the titles query as suggested by @michaelherger . This of course returns the titles_loop containing an entry for every track, so I identified the first entry for each work and added a grep to remove the rest. See code below.
This works fine with Material Skin, but Default Skin is using the position in the originally returned titles_loop in the SQL LIMIT clause in the _getTagDataForTracks query which is retrieving the work to be fed into the Albums query, which is now out of step with the array amended for display. So it therefore links to the wrong album(s).
Any ideas on how to proceed?
my $results = shift;
my $items = $results->{'titles_loop'};
$remote_library ||= $args->{'remote_library'};
my $lastWork = "";
my $lastComposerID = 0;
foreach (@$items) {
if ( $_->{'artist_id'} ne $lastComposerID || $_->{'work'} ne $lastWork ) {
$lastComposerID = $_->{'artist_id'};
$lastWork = $_->{'work'};
if ( $byComposer ) {
$_->{'name'} = $_->{'artist'}."\n".$_->{'work'};
} else {
$_->{'name'} = $_->{'work'}."\n".$_->{'artist'};
}
$_->{'name2'} = $_->{'composer'};
$_->{'type'} = 'playlist';
$_->{'playlist'} = \&_tracks;
$_->{'url'} = \&_albums;
$_->{'passthrough'} = [ { searchTags => [@searchTags, "work_id:" . $_->{'work'}, "composer_id:" . $_->{'artist_id'}], remote_library => $remote_library } ];
$_->{'favorites_url'} = 'db:tracks.work=' . ($_->{'work'} || 0 );
}
};
@$items = grep { exists $_->{name} } @$items;
How's the Work information currently stored? This must be an attribute related to each and every title. Therefore you should be able to filter this out in the titles SQL query, wouldn't you?
Yes, but I was trying to do the grouping in the navigation code, as you suggested, unless I misunderstood, which is quite likely!
I'm sorry for my probably confusing feedback... I'm still learning the use case 😖. While I'd agree that the grouping could be done "client side", I'd do the filtering in the titles query. This way you wouldn't have to skip items or filter them out, but only group what is there. A composer might have a lot of tracks in which they played a different role, or where the tracks didn't belong to a work?
I think I understand better now.
Is it really a good idea to complicate the titles query, rather than introduce a new works query, as I first did? This seemed to me to be in line with other grouping options, like genre and year, which have their own queries.
On the role question, you will see I'm limiting the query to the composer role (a composer can, of course be a conductor, performer, etc and I would not be including these in the works query unless they're also the composer).
It's probably worth stating explicitly that I'm using composer not only for the ORDER BY - it is also necessary because in theory there may be works with the same name but a different composer, which are, of course, distinct works.
A works query might indeed make sense. I was mostly agains the worksbycomposer query, which I believe is way too specific. The composer should be a parameter to the works query.
worksbycomposer does call the works query with a parameter. But I had to add a separate dispatch to avoid problems with caching which I tried to describe above. You will see both dispatches execute the same subroutine.
I'm sorry, had missed the comment about the caching. Would you know at what level the caching would happen? Web pages in the Default skin are being cached pretty aggressively, as their rendering is so slow. But that should take into account any parameter used to render the page.
After further experimentation, it turns out that all I needed was a different mode parameter - a corresponding dispatch was not required. I was confused over modes Vs dispatches as they have the same names.
I then went on to look at the parameters - I added my parameter byComposer to @topLevelArgs and now I no longer require a different mode.
Thanks for the pointers.
I've updated the PR with my changes to date. I have a problem which I hope someone can help with.
I've managed to add a working "Works" entry in the composer-albums list by adding it to the @$extra array in Slim::Menu::BrowseLibrary::_albums (see screenshot
). But I want it to appear above the list of albums (there could be loads in a real life sized library),
Here is the code (it's the fixedParams mode => "works" which makes this work but when added to the top of the @$items array instead, every item in the menu becomes a link to the Works list, rather than the album, the Qobuz search, etc.
What am I doing wrong?
unshift @$extra, {
name => "Works",
image => 'html/images/playlists.png',
type => 'link',
playlist => \&_works,
url => \&_works,
passthrough => [
{
orderBy => undef,
searchTags => \@searchTags,
menuStyle => 'menuStyle:allSongs',
}
],
itemActions => {
allAvailableActionsDefined => 0,
items => {
command => [BROWSELIBRARY, 'items'],
fixedParams => { %$params, mode => "works" },
},
},
};
TBH: that's nothing I would know. I'd have to do a lot of trial and error, too... unfortunately.
That said: it's always good to learn from existing code. Would you know of anything which would add extra items at the beginning of the list? That's probably simply not a planned feature...
From the look of your picture it seems as if you were working on the albums view - which I recently replaced with albums or releases, depending on user preferences. How do they play together? Could works be just yet another item in the "releases" menu?...
Wow... you've figured out some of the cruel stuff (@topLevelArgs!)! I applaud your dedication! Hopefully we'll be able to iron things out.
Please bear with me as I'm asking more question than provide help. But the better I understand the better I'll be able to give you answers. I'm so not a classical listener, I don't understand many of the pain points - nor would I have a collection to test this with...
I'd be happy to contribute the metadata from my classical collection, if this would be helpful. At one time, I was a music director for a classical public radio network and I think I'm pretty consistent with my tagging. Stats on my collection:
Total Tracks: 95,824 Total Albums: 4,730 Total Artists: 903 Total Genres: 29 Total Playlists: 41 Total Playing Time: 6469:24:48
I could bundle up the metadata as cuesheets in a zip file, with all the cuesheets just referencing an empty flac file.
On Wed, Nov 15, 2023, 1:04 PM Michael Herger @.***> wrote:
Wow... you've figured out some of the cruel stuff @.***!)! I applaud your dedication! Hopefully we'll be able to iron things out.
Please bear with me as I'm asking more question than provide help. But the better I understand the better I'll be able to give you answers. I'm so not a classical listener, I don't understand many of the pain points - nor would I have a collection to test this with...
— Reply to this email directly, view it on GitHub https://github.com/Logitech/slimserver/pull/930#issuecomment-1813184028, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACQJ44ZRGYEKK44NVWXMG7TYEUN3PAVCNFSM6AAAAAA6PZAGQSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJTGE4DIMBSHA . You are receiving this because you commented.Message ID: @.***>
@michaelherger Yes, trial and error is my method, just thought I'd ask in case there was something obvious I was missing. I am still wondering about including Works with albums or releases, which I may well do if I can't figure out how to get the "Works" entry at the top of the list.
@gharris999 That would be very useful, testing with a big data set will be essential before this merged. Thanks.
I'd be happy to contribute the metadata from my classical collection, if this would be helpful. At one time, I was a music director for a classical public radio network and I think I'm pretty consistent with my tagging. Stats on my collection: Total Tracks: 95,824 Total Albums: 4,730 Total Artists: 903 Total Genres: 29 Total Playlists: 41 Total Playing Time: 6469:24:48 I could bundle up the metadata as cuesheets in a zip file, with all the cuesheets just referencing an empty flac file.
That would be great! I have such a 100k tracks fake library, but it had been created with random data. Therefore it's little useful for real testing.
I'll work on pulling this together. It may take a week or more as I'll have to write a script to do this. 99% of my metadata is in whole-album flacs with embedded cuesheets.
On Wed, Nov 15, 2023 at 9:52 PM Michael Herger @.***> wrote:
I'd be happy to contribute the metadata from my classical collection, if this would be helpful. At one time, I was a music director for a classical public radio network and I think I'm pretty consistent with my tagging. Stats on my collection: Total Tracks: 95,824 Total Albums: 4,730 Total Artists: 903 Total Genres: 29 Total Playlists: 41 Total Playing Time: 6469:24:48 I could bundle up the metadata as cuesheets in a zip file, with all the cuesheets just referencing an empty flac file.
That would be great! I have such a 100k tracks fake library, but it had been created with random data. Therefore it's little useful for real testing.
— Reply to this email directly, view it on GitHub https://github.com/Logitech/slimserver/pull/930#issuecomment-1813787316, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACQJ44Z7QIZD7QPBZZAD6PDYEWLY7AVCNFSM6AAAAAA6PZAGQSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJTG44DOMZRGY . You are receiving this because you were mentioned.Message ID: @.***>
Latest snapshot just committed.
I've abandoned trying to add the Works entry to the albums list and incorporated it into the Releases menu instead. I think it works better there anyway.
So the Releases menu will now have an extra entry "Classical Works" (if there are any works for the artist/composer in the database). I have excluded tracks which exist within works from the "Compositions" list - it is very possible for a composer/song writer to have stand-alone compositions as well as classical works (e.g. Elvis Costello, and Joan Armatrading has just written a symphony...).
I've added COMPOSER to the _albumsOrReleases subroutine so that _releases gets called when navigating from the extended browse modes Composer menu.
Adding to the play queue now works for Works.
I'll soon add another post here (and perhaps on the forum) with screenshots showing the new flows, so as to get some feedback/suggestions/criticism.
Material Skin is going to need some enhancements to play nicely with these changes, if/when the time comes.
Obviously still a lot of work to do (paging of works for large libraries, search as discussed previously, adding Works to favourites, as well as much tidying up/string internationalization etc and geting the tagging and online library integration working for all file types.
But I hope this is progress!
Current state of the navigation:
Select a composer from the Artists or Extended Browse Modes Composers menu. This shows the Releases menu, with a new item "Classical Works":
Click on Classical Works to see a list of all the works you have by the chosen composer:
Choose a work: you can use the "add" or "play" buttons to directly add all versions of the work in your library to the playlist - in this case I have two performances of Beethoven's 5th Symphony:
Alternatively click on the Work to show the albums which contain the work - you can now either add a specific performance of the work to the playlist:
Or click on the album to be taken to the whole album:
Note the useful full information against each track: you can now use WORK as a data element in your title format, which avoids the need to include the work name in every track name in order for the displayed track title to make sense:
Comments/suggestions for changes are welcome.
Git hint: I'd never commit to the main branch (public/8.4) but would always use feature branches. Your 8.4 now is ahead of the upstream 8.4. And as you've merged your version rather than "ours" this "Works" branch is now a mix of different states.
See eg. https://docs.gitlab.com/ee/gitlab-basics/feature_branch_workflow.html
Thanks, that's good advice. I knew it was a mistake even as I started it. I'm learning, hopefully.
Will this situation resolve itself when my release grouping PR is merged? Or do I need to do something?
@michaelherger , could I resolve this by:
- create a new branch "main" based on slimserver/public/8.4
- git checkout Works
- git rebase main
- git push origin Works
?
First I'd create a new branch for what is in 8.4 (on your end) now:
git checkout public/8.4
git checkout -b roles
git push --set-upstream origin roles
This will create a new branch roles with your latest changes and push it up to your repository (origin - change if that's not what you're using). Close the current pull request and create a new one for this new branch. Unfortunately this will loose the comments and history in the current PR. But I believe we're in pretty good shape there.
Now comes the somewhat scary part 😁.
Once you have created the new PR and verified it's the code you wanted to have, you can continue to reset 8.4 to match the official repo. I assume you call it something like upstream. If that's not the case then use the actual repo name:
git checkout public/8.4
git fetch --all
git reset --hard upstream/public/8.4
git push --force
This first makes sure you have the latest info from the upstream repo. Second it resets your current state to upstream's. Third it overrides the current state on your repo with the "original" version. You have to use --force as otherwise git would complain that the origin was ahead of what you try to push.
Next up: the Works branch. That one was good before you merged your 8.4. That's reference https://github.com/darrell-k/slimserver/commit/43ca257ae9a5e804b92bd6b398e7764a5ebb8706. Therefore reset it to that commit, then merge your now clean 8.4 branch:
git checkout Works
git reset --hard 43ca257ae9a5e804b92bd6b398e7764a5ebb8706
git merge public/8.4
git push --force
From now on always get the latest upstream 8.4 before you merge it to your feature branches.
I didn't test any of that. You might encounter a small issue here or there. But I believe it should be good... Good luck!