Skip to content

Browse albums by record label: initial version #1382

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

Open
wants to merge 4 commits into
base: public/9.1
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
147 changes: 147 additions & 0 deletions Slim/Control/Queries.pm
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

serverstatusQuery && infoTotalQuery might need an update? They're missing eg. works, too

Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,7 @@ sub albumsQuery {
my $work = $request->getParam('work_id');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove the tab after $work to fix the alignment. Now that's really important.

my $composerID = $request->getParam('composer_id');
my $fromSearch = $request->getParam('from_search');
my $label = $request->getParam('record_label');

my $ignoreNewAlbumsCache = $search || $compilation || $contributorID || $genreID || $trackID || $albumID || $year || Slim::Music::Import->stillScanning();

Expand Down Expand Up @@ -514,6 +515,11 @@ sub albumsQuery {
push @{$p}, $year;
}

if (defined $label) {
push @{$w}, 'albums.label = ?';
push @{$p}, $label;
}

if (defined $fromSearch && !defined $search) {
# If we've got here from a search, we don't want to show the album unless it matches all the user's search criteria.
# This matters for a Works search: we've shown the user a Work because it matches their criteria, but it is possible
Expand Down Expand Up @@ -1989,6 +1995,147 @@ sub irenableQuery {
$request->setStatusDone();
}

sub labelsQuery {
my $request = shift;

# check this is the correct query.
if ($request->isNotQuery([['labels']])) {
$request->setStatusBadDispatch();
return;
}

if (!Slim::Schema::hasLibrary()) {
$request->setStatusNotDispatchable();
return;
}

my $sqllog = main::DEBUGLOG && logger('database.sql');

# get our parameters
my $client = $request->client();
my $index = $request->getParam('_index');
my $quantity = $request->getParam('_quantity');
my $year = $request->getParam('year');
my $contributorID = $request->getParam('artist_id');
my $workID = $request->getParam('work_id');
my $libraryID = Slim::Music::VirtualLibraries->getRealId($request->getParam('library_id'));
my $tags = $request->getParam('tags') || '';

my $sql = 'SELECT distinct label FROM albums ';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have an index on label yet. SQLite is fast, in particular with the usually not too big albums table. Therefore I'd say let's cheat that index in without bumping the database version. That way we don't require a full wipe & rescan, but those doing so anyway will still take advantage of the new index.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the preferred method?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just add it to schema_23_up.sql, where you've added the column.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I didn't know 'old' schema files would be run again.

Copy link
Member

@michaelherger michaelherger Apr 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, you're actually right 😞: we only wipe the tables during a wipe & rescan. I thought we'd drop the database and build from scratch. So that won't work - unless someone went to manually delete the file. I fear we'll need another migration file. Shouldn't hurt those coming from 9.0, but only those already on 9.1 (@frank1969b).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, you're actually right 😞: we only wipe the tables during a wipe & rescan. I thought we'd drop the database and build from scratch. So that won't work - unless someone went to manually delete the file. I fear we'll need another migration file. Shouldn't hurt those coming from 9.0, but only those already on 9.1 (@frank1969b).

We could do something like a "create index if doesn't exist" in server startup. Maybe a bit hacky.

Copy link
Member

@michaelherger michaelherger Apr 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just noticed that we don't have indexes extid for albums or contributors either. Not sure whether we use them. But eg. counting all albums where extid is not null takes a fraction of a second on a Pi4 with about 200 out of 1700 albums having an extid. Nothing compared to Frank's collection, of course. Would you know off the top of your head whether we ever query for albums or artists with/without extid? You might add them to the .sql files while we're at it...

Anyway: we could put such a hack in place for a few weeks. Should just not forget to remove it after a while Slim::Schema::init() might be a candidate. Maybe log a warning to remind us of the hack?

my $w = [];
my $p = [];
push @{$w}, 'albums.label IS NOT NULL';

if (defined $contributorID) {

$sql .= 'JOIN contributor_album ON albums.id = contributor_album.contributor ';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please indent the first line by one more tab

# handle the case where we're asked for the VA id => return compilations
if ($contributorID == Slim::Schema->variousArtistsObject->id) {
push @{$w}, 'albums.compilation = ?';
push @{$p}, 1;
}
else {
push @{$w}, 'contributor_album.contributor = ?';
push @{$p}, $contributorID;
}
}

if ( $libraryID ) {
$sql .= 'JOIN library_album ON library_album.album = albums.id ';
push @{$w}, 'library_album.library = ?';
push @{$p}, $libraryID;
}

if (defined $year) {
push @{$w}, 'albums.year = ?';
push @{$p}, $year;
}
if (defined $workID) {
$sql .= "JOIN tracks on tracks.album = albums.id ";
if ( $workID eq "-1" ) {
push @{$w}, 'tracks.work IS NOT NULL';
} else {
push @{$w}, 'tracks.work = ?';
push @{$p}, $workID;
}
}

if ( @{$w} ) {
$sql .= 'WHERE ';
my $s = join( ' AND ', @{$w} );
$s =~ s/\%/\%\%/g;
$sql .= $s . ' ';
}

my $dbh = Slim::Schema->dbh;

my $stillScanning = Slim::Music::Import->stillScanning();

# Get count of all results, the count is cached until the next rescan done event
my $cacheKey = md5_hex($sql . join( '', @{$p} ) . Slim::Music::VirtualLibraries->getLibraryIdForClient($client));

my $count = $cache->{$cacheKey};
if ( !$count ) {
my $total_sth = $dbh->prepare_cached( qq{
SELECT COUNT(1) FROM ( $sql ) AS t1
} );

$total_sth->execute( @{$p} );
($count) = $total_sth->fetchrow_array();
$total_sth->finish;

if ( !$stillScanning ) {
$cache->{$cacheKey} = $count;
}
}

# now build the result

if ($stillScanning) {
$request->addResult('rescan', 1);
}

$count += 0;

my ($valid, $start, $end) = $request->normalize(scalar($index), scalar($quantity), $count);

if ($valid && $tags ne 'CC') {

$sql .= 'ORDER BY 1 ';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow... hadn't known this shortcut. But... please use the column name for easier reading.


my $loopname = 'labels_loop';
my $chunkCount = 0;

# Limit the real query
if ( $index =~ /^\d+$/ && $quantity =~ /^\d+$/ ) {
$sql .= "LIMIT $index, $quantity ";
}

if ( main::DEBUGLOG && $sqllog->is_debug ) {
$sqllog->debug( "Labels query: $sql / " . Data::Dump::dump($p) );
}

my $sth = $dbh->prepare_cached($sql);
$sth->execute( @{$p} );

my ($label);
$sth->bind_columns( \$label );

while ( $sth->fetch ) {

$request->addResultLoop($loopname, $chunkCount, 'record_label', $label);

$chunkCount++;

main::idleStreams() if !($chunkCount % 5);
}
}

$request->addResult('count', $count);

$request->setStatusDone();
}

sub librariesQuery {
my $request = shift;

Expand Down
1 change: 1 addition & 0 deletions Slim/Control/Request.pm
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,7 @@ sub init {
addDispatch(['libraries'], [0, 1, 0, \&Slim::Control::Queries::librariesQuery]);
addDispatch(['libraries', 'getid'], [1, 1, 0, \&Slim::Control::Queries::librariesQuery]);
addDispatch(['linesperscreen', '?'], [1, 1, 0, \&Slim::Control::Queries::linesperscreenQuery]);
addDispatch(['labels', '_index', '_quantity'], [0, 1, 1, \&Slim::Control::Queries::labelsQuery]);
addDispatch(['logging'], [0, 0, 1, \&Slim::Control::Commands::loggingCommand]);
addDispatch(['mixer', 'bass', '?'], [1, 1, 0, \&Slim::Control::Queries::mixerQuery]);
addDispatch(['mixer', 'bass', '_newvalue'], [1, 0, 1, \&Slim::Control::Commands::mixerCommand]);
Expand Down
20 changes: 19 additions & 1 deletion Slim/Menu/BrowseLibrary.pm
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ use JSON::XS::VersionOneAndTwo;

use Slim::Menu::BrowseLibrary::Releases;
use Slim::Menu::BrowseLibrary::Works;
use Slim::Menu::BrowseLibrary::Labels;
use Slim::Music::VirtualLibraries;
use Slim::Utils::Cache;
use Slim::Utils::Log;
Expand Down Expand Up @@ -596,6 +597,23 @@ sub _registerBaseNodes {
weight => 40,
cache => 1,
},
{
type => 'link',
name => 'BROWSE_BY_LABEL',
params => {mode => 'labels'},
feed => \&_labels,
icon => 'html/images/albums.png',
jiveIcon => 'html/images/albums.png',
homeMenuText => 'BROWSE_LABELS',
condition => sub {
return unless isEnabledNode(@_);
my $totals = Slim::Schema->totals($_[0]);
return $totals->{label} if $totals;
},
id => 'myMusicLabels',
weight => 45,
cache => 1,
},
{
type => 'link',
name => 'BROWSE_NEW_MUSIC',
Expand Down Expand Up @@ -735,7 +753,7 @@ sub setMode {
$client->modeParam( handledTransition => 1 );
}

our @topLevelArgs = qw(track_id artist_id genre_id album_id playlist_id year only_album_years folder_id role_id library_id remote_library release_type work_id composer_id from_search subtitle grouping performance);
our @topLevelArgs = qw(track_id artist_id genre_id album_id playlist_id year only_album_years folder_id role_id library_id remote_library release_type work_id composer_id from_search subtitle grouping performance record_label);

sub _topLevel {
my ($client, $callback, $args, $pt) = @_;
Expand Down
71 changes: 71 additions & 0 deletions Slim/Menu/BrowseLibrary/Labels.pm
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
package Slim::Menu::BrowseLibrary;

use strict;

use Slim::Utils::Log;
use Slim::Utils::Strings qw(cstring);

use constant BROWSELIBRARY => 'browselibrary';

my $log = logger('database.info');

sub _labels {
my ($client, $callback, $args, $pt) = @_;
my @searchTags = $pt->{'searchTags'} ? @{$pt->{'searchTags'}} : ();
my $library_id = $args->{'library_id'} || $pt->{'library_id'};
my $search = $args->{'search'} || $pt->{'search'};
my $remote_library = $args->{'remote_library'} ||= $pt->{'remote_library'};

if ($library_id && !grep /library_id/, @searchTags) {
push @searchTags, 'library_id:' . $library_id;
}

Slim::Menu::BrowseLibrary::_generic($client, $callback, $args, 'labels',
[ @searchTags, ($search ? 'search:' . $search : undef) ],
sub {
my $results = shift;
my $items = $results->{'labels_loop'};
$remote_library ||= $args->{'remote_library'};

foreach (@$items) {
$_->{'name'} = $_->{'record_label'};
$_->{'image'} = 'html/images/albums.png';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was surprised by the number of labels found in my (test) collection. Must be because many of those files come from users with extensive metadata 😁.

But looking at that list I was wondering... albums.png all over the place doesn't make sense IMHO. I'd prefer a text list only. Or did you consider adding support for label artwork?... Would require another schema change. Or some MAI work? I bet people caring about labels would recognise those logos.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought the same thing: a job for MAI if it's feasible. I just used album as a placeholder. For the top level menu icon, we could do with a label.png but I am certainly not the right person to be designing one. Maybe you could ask AI to come up with a record label icon in the style of LMS. If it does a good job, I might change my opinion of its usefulness 😂.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI has failed me (in the short time I tried).

Just in case you were considering a dedicated labels table, reserve a column for a logo ID...

$_->{'type'} = 'playlist';
$_->{'playlist'} = \&_tracks;
$_->{'url'} = \&_albums;
$_->{'passthrough'} = [ { searchTags => [@searchTags, "record_label:" . $_->{'record_label'}], remote_library => $remote_library } ];
};

my $params = _tagsToParams(\@searchTags);
my %actions = $remote_library ? (
commonVariables => [record_label => 'record_label']
) : (
allAvailableActionsDefined => 1,
commonVariables => [record_label => 'record_label'],
items => {
command => [BROWSELIBRARY, 'items'],
fixedParams => {
mode => 'albums',
%$params
},
},
play => {
command => ['playlistcontrol'],
fixedParams => {cmd => 'load', %$params},
},
add => {
command => ['playlistcontrol'],
fixedParams => {cmd => 'add', %$params},
},
insert => {
command => ['playlistcontrol'],
fixedParams => {cmd => 'insert', %$params},
},
);
$actions{'playall'} = $actions{'play'};
$actions{'addall'} = $actions{'add'};

return {items => $items, actions => \%actions, sorted => 1}, undef;
}, undef, 1,
);
}
5 changes: 5 additions & 0 deletions Slim/Schema.pm
Original file line number Diff line number Diff line change
Expand Up @@ -1248,6 +1248,8 @@ sub _createOrUpdateAlbum {
$albumHash->{release_type} = Slim::Utils::Text::ignoreCase( $releaseType || 'album' );
Slim::Schema::Album->addReleaseTypeMap($releaseType, $albumHash->{release_type});

$albumHash->{label} = $attributes->{LABEL};

# Bug 3255 - add album contributor which is either VA or the primary artist, used for sort by artist
$vaObjId ||= $self->variousArtistsObject->id;

Expand Down Expand Up @@ -2814,6 +2816,8 @@ sub _preCheckAttributes {
# We also need these in _postCheckAttributes, but they should be set during create()
$deferredAttributes->{'DISC'} = $attributes->{'DISC'} if $attributes->{'DISC'};

$deferredAttributes->{'LABEL'} = $attributes->{'LABEL'}||undef;

# thumb has gone away, since we have GD resizing.
delete $attributes->{'THUMB'};

Expand Down Expand Up @@ -3266,6 +3270,7 @@ sub totals {
track => ['titles', 0, 1, 'tags:CC'],
playlist => ['playlists', 0, 1, 'tags:CC'],
work => ['works', 0, 1, 'tags:CC'],
label => ['labels', 0, 1, 'tags:CC'],
);

while (my ($key, $query) = each %categories) {
Expand Down
1 change: 1 addition & 0 deletions Slim/Schema/Album.pm
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ my $log = logger('database.info');
musicbrainz_id
release_type
extid
label
), title => { accessor => undef() });

$class->set_primary_key('id');
Expand Down
19 changes: 19 additions & 0 deletions strings.txt
Original file line number Diff line number Diff line change
Expand Up @@ -14628,6 +14628,17 @@ BROWSE_GENRES
RU Просмотр жанров
SV Bläddra efter genre

BROWSE_LABELS
CS Procházet nahrávací společnosti
DA Gennemse pladeselskaber
DE Plattenlabels durchsuchen
EN Browse Labels
ES Explorar sellos discográficos
FR Parcourir les labels d'enregistrement
HU Lemezkiadók böngészése
NL Bladeren door platenlabels
PT Procurar etiquetas de discos

BROWSE_WORKS
CS Procházet díla
DA Gennemsøg værker
Expand Down Expand Up @@ -14789,6 +14800,14 @@ BROWSE_BY_SONG_DBL
FR Morceaux
PL Utwory

BROWSE_BY_LABEL
DA pladeselskab
DE Plattenlabel
EN Record Label
FR Label d'enregistrement
HU Lemezkiadó
NL Platenmaatschappij

BROWSE_BY_WORK
DA Værker
DE Werke
Expand Down