Merge pull request #1338 from darrell-k/fix-mb-tags-cue-v2

Also clean/validate cue tags

Author: michaelherger

Slim/Formats.pm

@@ -275,7 +275,13 @@ sub readTags {
 
 	main::DEBUGLOG && $isDebug && $log->debug("Report for $file:");
 
-	# XXX: can Audio::Scan make these regexes unnecessary?
+	sanitizeTagValues($tags, $file);
+
+	return $tags;
+}
+
+sub sanitizeTagValues {
+	my ($tags, $file) = @_;
 
 	# Bug: 2381 - FooBar2k seems to add UTF8 boms to their values.
 	# Bug: 3769 - Strip trailing nulls
@@ -286,7 +292,7 @@ sub readTags {
 			my $original = $value;
 
 			use bytes;
-			if ( my $cached = $tagCache{$value} ) {
+			if ( my $cached = $tagCache{$tag}{$value} ) {
 				$tags->{$tag} = $cached;
 				next;
 
@@ -329,17 +335,15 @@ sub readTags {
 				$value = $tags->{$tag} = \@mbIDs;
 			}
 
-			$tagCache{$original} = $value;
+			$tagCache{$tag}{$original} = $value;
 		}
 
-		main::DEBUGLOG && $isDebug && $value && $log->debug(". $tag : $value");
+		main::DEBUGLOG && $log->is_debug && $value && $log->debug(". $tag : $value");
 	}
 
 	if (scalar (keys %tagCache) > 50) {
 		%tagCache = ();
 	}
-
-	return $tags;
 }
 
 1;

Slim/Formats/FLAC.pm

@@ -133,6 +133,10 @@ sub getTag {
 	# suck in metadata for all these tags
 	my $items = $class->_getSubFileTags($s, $tracks);
 
+	foreach (values %$tracks) {
+		Slim::Formats::sanitizeTagValues($_, $file);
+	}
+
 	# fallback if we can't parse metadata
 	if ( $items < 1 ) {
 		logWarning("Unable to find metadata for tracks referenced by cuesheet: [$file]");
@@ -217,13 +221,13 @@ sub _getStandardTag {
 	my $tags = $s->{tags} || {};
 
 	$class->_addInfoTags($s, $tags);
-	$class->_doTagMapping($tags);
+	$class->doTagMapping($tags);
 	$class->_addArtworkTags($s, $tags);
 
 	return $tags;
 }
 
-sub _doTagMapping {
+sub doTagMapping {
 	my ($class, $tags) = @_;
 
 	# Map ID3 tags first, so FLAC tags win out
@@ -349,10 +353,6 @@ sub _getSubFileTags {
 	$items = $class->_getCUEinVCs($s, $tracks);
 	return $items if $items > 0;
 
-	# try parsing stacked vorbis comments
-	$items = $class->_getStackedVCs($s, $tracks);
-	return $items if $items > 0;
-
 	# This won't yield good results - but without it, we regress from 6.0.2
 	my $tags = $class->_getStandardTag($s);
 
@@ -532,7 +532,7 @@ sub _getXMLTags {
 
 			%{$tracks->{$cuesheetTrack}} = (%{$defaultTags}, %{$tracks->{$cuesheetTrack}});
 
-			$class->_doTagMapping($tracks->{$cuesheetTrack});
+			$class->doTagMapping($tracks->{$cuesheetTrack});
 		}
 	}
 
@@ -627,7 +627,7 @@ sub _getNumberedVCs {
 
 		%{$tracks->{$num}} = (%{$defaultTags}, %{$tracks->{$num}});
 
-		$class->_doTagMapping($tracks->{$num});
+		$class->doTagMapping($tracks->{$num});
 
 		$tracks->{$num}->{TRACKNUM} = $num unless exists $tracks->{$num}->{TRACKNUM};
 	}
@@ -726,7 +726,7 @@ sub _getCDDBTags {
 
 		%{$tracks->{$key}} = (%{$tags}, %{$tracks->{$key}});
 
-		$class->_doTagMapping($tracks->{$key});
+		$class->doTagMapping($tracks->{$key});
 	}
 
 	return $items;
@@ -783,115 +783,14 @@ sub _getCUEinVCs {
 			}
 		}
 
-		$class->_doTagMapping($tracks->{$key});
+		$class->doTagMapping($tracks->{$key});
 
 		$items++;
 	}
 
 	return $items;
 }
 
-sub _getStackedVCs {
-	my ($class, $s, $tracks) = @_;
-
-	my $items  = 0;
-
-	# XXX: can't support this using Audio::Scan, this is a stupid tag scheme anyway!
-	return 0;
-
-=pod
-	# parse "stacked" vorbis comments
-	# this is tricky when it comes to matching which groups belong together
-	# particularly for various artist, or multiple album compilations.
-	# this as also not terribly efficent, so it's not our first choice.
-
-	# here's a simple example of the sort of thing we're trying to work with
-	#
-	# ARTIST=foo
-	# ALBUM=bar
-	# TRACKNUMBER=1
-	# TITLE=baz
-	# TRACKNUMBER=2
-	# TITLE=something
-
-	# grab the raw comments for parsing
-	my $rawTags = $flac->{'rawTags'};
-
-	# grab the cuesheet for reference
-	my $cuesheet = $flac->cuesheet();
-
-	# validate number of TITLE tags against number of
-	# tracks in the cuesheet
-	my $titletags = 0;
-	my $cuetracks = 0;
-
-	for my $tag (@$rawTags) {
-		$titletags++ if $tag =~ /^\s*TITLE=/i;
-	}
-
-	for my $track (@$cuesheet) {
-		$cuetracks++ if $track =~ /^\s*TRACK/i;
-	}
-
-	return 0 unless $titletags == $cuetracks;
-
-
-	# ok, let's see which tags apply to which tracks
-
-	my $tempTags = {};
-	my $defaultTags = {};
-
-	$class->_addInfoTags($flac, $defaultTags);
-
-	for my $tag (@$rawTags) {
-
-		# Match the key and value
-		if ($tag =~ /^(.*?)=(.*?)[\r\n]*$/) {
-
-			# Make the key uppercase
-			my $tkey  = uc($1);
-			my $value = $2;
-
-			# use duplicate detection to find track boundries
-			# retain file wide values as defaults
-			if (defined $tempTags->{$tkey}) {
-				$items++;
-				my %merged = (%{$defaultTags}, %{$tempTags});
-				$defaultTags = \%merged;
-				$tempTags = {};
-
-				# set the tags on the track
-				%{$tracks->{$items}} = (%{$tracks->{$items}}, %{$defaultTags});
-
-				$class->_doTagMapping($tracks->{$items});
-
-				if (!exists $tracks->{$items}->{'TRACKNUM'}) {
-					$tracks->{$items}->{'TRACKNUM'} = $items;
-				}
-
-			}
-
-			$tempTags->{$tkey} = $value;
-
-			main::DEBUGLOG && logger('formats.playlists')->debug("    $tkey: $value");
-		}
-	}
-
-	# process the final track
-	$items++;
-
-	%{$tracks->{$items}} = (%{$tracks->{$items}}, %{$defaultTags}, %{$tempTags});
-
-	$class->_doTagMapping($tracks->{$items});
-
-	if (!exists $tracks->{$items}->{'TRACKNUM'}) {
-		$tracks->{$items}->{'TRACKNUM'} = $items;
-	}
-
-	return $items;
-=cut
-}
-
 =head2 findFrameBoundaries( $fh, $offset, $time )
 
 Returns offset to audio frame containing $time.

Slim/Formats/Playlists/CUE.pm

@@ -367,7 +367,7 @@ sub parse {
 			}
 		} else {
 
-			# handle remaning Commands as a list of keys and values.
+			# handle remaining Commands as a list of keys and values.
 			($cuesheet, $tracks) = _addCommand($cuesheet,
 											 $tracks,
 											 $inAlbum,
@@ -564,6 +564,20 @@ sub parse {
 
 		# Everything in a cue sheet should be marked as audio.
 		$track->{'AUDIO'} = 1;
+
+		# external cue sheet hasn't been through tag mapping, so do it now.
+		if ( !$embedded && $track->{'FILENAME'} ) {
+			if ( my $type = Slim::Music::Info::typeFromPath(Slim::Utils::Misc::pathFromFileURL($track->{'FILENAME'})) ) {
+				if ( my $tagReaderClass = Slim::Formats->classForFormat($type) ) {
+					Slim::Formats->loadTagFormatForType($type);
+					if ( $tagReaderClass->can('doTagMapping') ) {
+						$tagReaderClass->doTagMapping($track);
+					}
+				}
+			}
+		}
+
+		Slim::Formats::sanitizeTagValues($track, $filename);
 	}
 
 	# Bug 8443, if no tracks contain a URI element, it's an invalid cue

Slim/Schema.pm

@@ -912,7 +912,8 @@ sub _createOrUpdateAlbum {
 	my $disc      = $attributes->{DISC};
 	my $discc     = $attributes->{DISCC};
 	# Bug 10583 - Also check for MusicBrainz Album Id
-	my $brainzId  = $attributes->{MUSICBRAINZ_ALBUM_ID}[0]; # Surely there is only one MB album id!
+	# Surely there is only one MB album id!
+	my $brainzId  = ref $attributes->{MUSICBRAINZ_ALBUM_ID} ? $attributes->{MUSICBRAINZ_ALBUM_ID}[0] : $attributes->{MUSICBRAINZ_ALBUM_ID};
 	my $extId     = $attributes->{EXTID} || $attributes->{ALBUM_EXTID};
 
 	# if we have a disc number from an online service, default disc count to 1
@@ -2643,6 +2644,9 @@ sub _preCheckAttributes {
 
 	# Normalize attribute names
 	while ( my ($key, $val) = each %{ $args->{'attributes'} } ) {
+		if ( $key =~ /^MUSICBRAINZ.*ID$/ && $val && ref $val ne 'ARRAY' ) {
+			logWarning("$key ($val) in " . Slim::Utils::Misc::pathFromFileURL($url) . " has not been validated by Slim::Formats::sanitizeTagValues");
+		}
 		# don't overwrite mapped values
 		next if $mappedValues{$key};