Skip to content

Commit

Permalink
Fix #547 - Don't add album of same name if adding tracks while renami…
Browse files Browse the repository at this point in the history 
…ng the existing tracks' album title

As we did add new tracks before updating modified tracks, we'd first add new tracks with the new album title, resulting in a new album. When we then updated the existing tracks we didn't assign the existing tracks to that new album, but simply modified the existing album title. This resulted in two albums of the same name, one with the old, one with the new tracks.

By changing the scan order (delete, update, add, instead of delete, add, update) we hopefully fix this without introducing new side-effects.
  • Loading branch information
@mherger
mherger committed Feb 5, 2022
1 parent e939c0b commit 09f788d
Show file tree
Hide file tree
Showing 2 changed files with 170 additions and 158 deletions.
3 changes: 2 additions & 1 deletion Changelog8.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,12 @@ <h2><a name="v8.3.0" id="v8.3.0"></a>Version 8.3.0</h2>
<li>Improve Deezer metadata lookup when adding albums/playlists through the CLI.</li>
<li><a href="https://github.com/andygrundman/Audio-Scan/issues/9">(Audio::Scan) #9</a> - For some WavPack DSD file the song_length_ms is incorrect (thanks aeeq & ralphy!)</li>
<li><a href="https://github.com/andygrundman/Audio-Scan/pulls/12">(Audio::Scan) #12</a> - ID3: Fix v2.4 extended header handling (thanks mw9 & ralphy!)</li>
<li><a href="https://github.com/Logitech/slimserver/issues/547">#547</a> - duplicate albums after adding tracks while renaming album</li>
<li><a href="https://github.com/Logitech/slimserver/pull/668">#668</a> - Podcasts: Pre-caching image and more-info data can bring the server to a crawl #668 (thanks mw9!)</li>
<li>Fix image transformation if a cover requested using /current/cover is pointing to a local file.</li>
<li><a href="https://github.com/Logitech/slimserver/issues/700">#700</a>/<a href="https://github.com/Logitech/slimserver/pull/718">#718</a> - High CPU load during playback of certain radio streams (thanks philippe44!)</li>
<li><a href="https://github.com/Logitech/slimserver/issues/704">#704</a> - changed artist names remain in database after quick rescan</li>
<li><a href="https://github.com/Logitech/slimserver/issues/705">#705</a> - changing large and lower case in file name results in double entries</li>
<li><a href="https://github.com/Logitech/slimserver/issues/705">#705</a> - changing upper and lower case in file name results in double entries</li>
<li><a href="https://github.com/Logitech/slimserver/pull/749">#749</a> - fix mp4 streams where audio offset comes from STCO (thanks philippe44 && bpa!)</li>
<li><a href="https://github.com/Logitech/slimserver/pull/754">#754</a> - improve reliabilityof "what's new" podcast search (thanks philippe44!)</li>
</ul>
Expand Down
325 changes: 168 additions & 157 deletions Slim/Utils/Scanner/Local.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -298,164 +298,9 @@ sub rescan {
sql => $inDBOnlySQL
}, $args);

$log->error( "Scanning new audio files ($onDiskOnlyCount)" ) unless main::SCANNER && $main::progress;

if ( $onDiskOnlyCount && !Slim::Music::Import->hasAborted() ) {
my $onDiskOnlySth;
my $onDiskOnlyDone = 0;

$pending{$next} |= PENDING_NEW;

my $progress;
if ( $args->{progress} ) {
$progress = Slim::Utils::Progress->new( {
type => 'importer',
name => $next . '|' . ($args->{scanName} . '_new'),
bar => 1,
every => ($args->{scanName} && $args->{scanName} eq 'playlist'), # record all playists in the db
total => $onDiskOnlyCount,
} );
}

my $new;
my $handle_new = sub {
if ( hasAborted($progress, $args->{no_async}) ) {
$onDiskOnlySth && $onDiskOnlySth->finish;
return 0;
}

# Page through the files, this is to avoid a long-running read query which
# would prevent WAL checkpoints from occurring.
if ( !$onDiskOnlySth ) {
my $sql = $onDiskOnlySQL . " LIMIT $onDiskOnlyDone, " . CHUNK_SIZE;
$onDiskOnlySth = $dbh->prepare($sql);
$onDiskOnlySth->execute;
$onDiskOnlySth->bind_col(1, \$new);
}

if ( $onDiskOnlySth->fetch ) {
$progress && $progress->update( Slim::Utils::Misc::pathFromFileURL($new) );
$changes++;
$onDiskOnlyDone++;

new($new);

return 1;
}
else {
my $more = 1;

if ( !$onDiskOnlySth->rows ) {
$dbh->do('DROP TABLE IF EXISTS diskonly');

if ( !$args->{no_async} ) {
$args->{paths} = $paths;
markDone( $next => PENDING_NEW, $changes, $args );
}

$progress && $progress->final;
$onDiskOnlySth->finish;
$more = 0;
}
else {
# Continue paging with the next chunk
$onDiskOnlySth->finish;
undef $onDiskOnlySth;
}

# Commit for every chunk when using scanner.pl
main::SCANNER && Slim::Schema->forceCommit;

return $more;
}
};

if ( $args->{no_async} ) {
while ( $handle_new->() ) { }
}
else {
Slim::Utils::Scheduler::add_ordered_task( $handle_new );
}
}

$log->error( "Rescanning changed audio files ($changedOnlyCount)" ) unless main::SCANNER && $main::progress;

if ( $changedOnlyCount && !Slim::Music::Import->hasAborted() ) {
my $changedOnlySth;
my $changedOnlyDone = 0;

$pending{$next} |= PENDING_CHANGED;

my $progress;
if ( $args->{progress} ) {
$progress = Slim::Utils::Progress->new( {
type => 'importer',
name => $next . '|' . ($args->{scanName} . '_changed'),
bar => 1,
every => ($args->{scanName} && $args->{scanName} eq 'playlist'), # record all playists in the db
total => $changedOnlyCount,
} );
}

my $changed;
my $handle_changed = sub {
if ( hasAborted($progress, $args->{no_async}) ) {
$changedOnlySth && $changedOnlySth->finish;
return 0;
}

# Page through the files, this is to avoid a long-running read query which
# would prevent WAL checkpoints from occurring.
if ( !$changedOnlySth ) {
my $sql = $changedOnlySQL . " LIMIT $changedOnlyDone, " . CHUNK_SIZE;
$changedOnlySth = $dbh->prepare($sql);
$changedOnlySth->execute;
$changedOnlySth->bind_col(1, \$changed);
}

if ( $changedOnlySth->fetch ) {
$progress && $progress->update( Slim::Utils::Misc::pathFromFileURL($changed) );
$changes++;
$changedOnlyDone++;

changed($changed);

return 1;
}
else {
my $more = 1;

if ( !$changedOnlySth->rows ) {
if ( !$args->{no_async} ) {
$args->{paths} = $paths;
markDone( $next => PENDING_CHANGED, $changes, $args );
}

$progress && $progress->final;
$changedOnlySth->finish;
$more = 0;
}
else {
# Continue paging with the next chunk
$changedOnlySth->finish;
undef $changedOnlySth;
}

# Commit for every chunk when using scanner.pl
main::SCANNER && Slim::Schema->forceCommit;

return $more;
}
};

if ( $args->{no_async} ) {
while ( $handle_changed->() ) { }
}
else {
Slim::Utils::Scheduler::add_ordered_task( $handle_changed );
}
}
$class->updateTracks($dbh, \$changes, \$paths, \$next, $changedOnlyCount, $changedOnlySQL, $args);

$class->addTracks($dbh, \$changes, \$paths, \$next, $onDiskOnlyCount, $onDiskOnlySQL, $args);

if ( hasAborted() ) {
# nothing to do here - should be handled in hasAborted()
Expand Down Expand Up @@ -596,6 +441,172 @@ sub deleteTracks {
}
}

sub addTracks {
my ($class, $dbh, $changes, $paths, $nextFolder, $onDiskOnlyCount, $onDiskOnlySQL, $args) = @_;

$log->error( "Scanning new audio files ($onDiskOnlyCount)" ) unless main::SCANNER && $main::progress;

if ( $onDiskOnlyCount && !Slim::Music::Import->hasAborted() ) {
my $onDiskOnlySth;
my $onDiskOnlyDone = 0;

$pending{$nextFolder} |= PENDING_NEW;

my $progress;
if ( $args->{progress} ) {
$progress = Slim::Utils::Progress->new( {
type => 'importer',
name => $nextFolder . '|' . ($args->{scanName} . '_new'),
bar => 1,
every => ($args->{scanName} && $args->{scanName} eq 'playlist'), # record all playists in the db
total => $onDiskOnlyCount,
} );
}

my $new;
my $handle_new = sub {
if ( hasAborted($progress, $args->{no_async}) ) {
$onDiskOnlySth && $onDiskOnlySth->finish;
return 0;
}

# Page through the files, this is to avoid a long-running read query which
# would prevent WAL checkpoints from occurring.
if ( !$onDiskOnlySth ) {
my $sql = $onDiskOnlySQL . " LIMIT $onDiskOnlyDone, " . CHUNK_SIZE;
$onDiskOnlySth = $dbh->prepare($sql);
$onDiskOnlySth->execute;
$onDiskOnlySth->bind_col(1, \$new);
}

if ( $onDiskOnlySth->fetch ) {
$progress && $progress->update( Slim::Utils::Misc::pathFromFileURL($new) );
$changes++;
$onDiskOnlyDone++;

new($new);

return 1;
}
else {
my $more = 1;

if ( !$onDiskOnlySth->rows ) {
$dbh->do('DROP TABLE IF EXISTS diskonly');

if ( !$args->{no_async} ) {
$args->{paths} = $paths;
markDone( $nextFolder => PENDING_NEW, $changes, $args );
}

$progress && $progress->final;
$onDiskOnlySth->finish;
$more = 0;
}
else {
# Continue paging with the next chunk
$onDiskOnlySth->finish;
undef $onDiskOnlySth;
}

# Commit for every chunk when using scanner.pl
main::SCANNER && Slim::Schema->forceCommit;

return $more;
}
};

if ( $args->{no_async} ) {
while ( $handle_new->() ) { }
}
else {
Slim::Utils::Scheduler::add_ordered_task( $handle_new );
}
}
}

sub updateTracks {
my ($class, $dbh, $changes, $paths, $nextFolder, $changedOnlyCount, $changedOnlySQL, $args) = @_;

$log->error( "Rescanning changed audio files ($changedOnlyCount)" ) unless main::SCANNER && $main::progress;

if ( $changedOnlyCount && !Slim::Music::Import->hasAborted() ) {
my $changedOnlySth;
my $changedOnlyDone = 0;

$pending{$nextFolder} |= PENDING_CHANGED;

my $progress;
if ( $args->{progress} ) {
$progress = Slim::Utils::Progress->new( {
type => 'importer',
name => $nextFolder . '|' . ($args->{scanName} . '_changed'),
bar => 1,
every => ($args->{scanName} && $args->{scanName} eq 'playlist'), # record all playists in the db
total => $changedOnlyCount,
} );
}

my $changed;
my $handle_changed = sub {
if ( hasAborted($progress, $args->{no_async}) ) {
$changedOnlySth && $changedOnlySth->finish;
return 0;
}

# Page through the files, this is to avoid a long-running read query which
# would prevent WAL checkpoints from occurring.
if ( !$changedOnlySth ) {
my $sql = $changedOnlySQL . " LIMIT $changedOnlyDone, " . CHUNK_SIZE;
$changedOnlySth = $dbh->prepare($sql);
$changedOnlySth->execute;
$changedOnlySth->bind_col(1, \$changed);
}

if ( $changedOnlySth->fetch ) {
$progress && $progress->update( Slim::Utils::Misc::pathFromFileURL($changed) );
$changes++;
$changedOnlyDone++;

changed($changed);

return 1;
}
else {
my $more = 1;

if ( !$changedOnlySth->rows ) {
if ( !$args->{no_async} ) {
$args->{paths} = $paths;
markDone( $nextFolder => PENDING_CHANGED, $changes, $args );
}

$progress && $progress->final;
$changedOnlySth->finish;
$more = 0;
}
else {
# Continue paging with the next chunk
$changedOnlySth->finish;
undef $changedOnlySth;
}

# Commit for every chunk when using scanner.pl
main::SCANNER && Slim::Schema->forceCommit;

return $more;
}
};

if ( $args->{no_async} ) {
while ( $handle_changed->() ) { }
}
else {
Slim::Utils::Scheduler::add_ordered_task( $handle_changed );
}
}
}

sub hasAborted {
my ($progress, $no_async) = @_;

Expand Down

0 comments on commit 09f788d

Please sign in to comment.