From f130f1658ccdf0d31637b4611aed5cd7243cd8a7 Mon Sep 17 00:00:00 2001 From: Rob Arnold Date: Wed, 15 Apr 2015 02:58:00 -0700 Subject: [PATCH] Tested and fixed playlist item removal code, updated and added UI elements related to playlists. Added taglib as framework Added genre/playlist mapping, aka genre hack, found existing bugs, added more, then fixed a bunch of them. All were related to unicode/accent chars in media and filenames, or were bad assumptions about what is an error and what isn't during playlist cleanup or copy/conversion, usually race condition related with directory creation or dir/file deletion. Found several taglib crashes caused by such issues, and fixed the original causes, but still would like to make the taglib code in particular fail more gracefully. Added build config for using taglib as a framework in the app bundle. Note had to fix the dir layout of the framework and do a 'install_name_tool -id @rpath/tag.framework/Versions/A/tag tag.framework/Versions/A/tag' to get it to work properly. --- TeslaTunes.xcodeproj/project.pbxproj | 21 ++- TeslaTunes/Base.lproj/Main.storyboard | 23 ++- TeslaTunes/CopyConvertDirs.h | 2 + TeslaTunes/CopyConvertDirs.mm | 237 ++++++++++++++++---------- TeslaTunes/ViewController.h | 6 +- TeslaTunes/ViewController.m | 1 + TeslaTunes/todo and notes.txt | 10 ++ 7 files changed, 202 insertions(+), 98 deletions(-) diff --git a/TeslaTunes.xcodeproj/project.pbxproj b/TeslaTunes.xcodeproj/project.pbxproj index f6e6c87..5eefd11 100644 --- a/TeslaTunes.xcodeproj/project.pbxproj +++ b/TeslaTunes.xcodeproj/project.pbxproj @@ -27,7 +27,6 @@ E502D0C81A739D1500BE86E2 /* ViewController.m in Sources */ = {isa = PBXBuildFile; fileRef = E502D0C71A739D1500BE86E2 /* ViewController.m */; }; E502D0CA1A739D1500BE86E2 /* Images.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = E502D0C91A739D1500BE86E2 /* Images.xcassets */; }; E502D0CD1A739D1500BE86E2 /* Main.storyboard in Resources */ = {isa = PBXBuildFile; fileRef = E502D0CB1A739D1500BE86E2 /* Main.storyboard */; }; - E52E87971ADD9AA900B08A74 /* tag.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = E52E87961ADD9AA900B08A74 /* tag.framework */; }; E52E87991ADD9F2E00B08A74 /* AVFoundation.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = E52E87981ADD9F2E00B08A74 /* AVFoundation.framework */; }; E52E879B1ADD9F8F00B08A74 /* AudioToolbox.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = E52E879A1ADD9F8F00B08A74 /* AudioToolbox.framework */; }; E5436D291AC3D44D0068E42E /* iTunesLibrary.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = E5436D281AC3D44D0068E42E /* iTunesLibrary.framework */; }; @@ -37,8 +36,9 @@ E59078A41AAAFE0E007A26D6 /* CopyConvertDirs.mm in Sources */ = {isa = PBXBuildFile; fileRef = E59078A31AAAFE0E007A26D6 /* CopyConvertDirs.mm */; }; E59078A71AAC1AD7007A26D6 /* flac_utils.mm in Sources */ = {isa = PBXBuildFile; fileRef = E59078A51AAC1AD7007A26D6 /* flac_utils.mm */; }; E59078AA1AAEC044007A26D6 /* Receptionist.m in Sources */ = {isa = PBXBuildFile; fileRef = E59078A91AAEC044007A26D6 /* Receptionist.m */; }; - E59078AC1AAF9741007A26D6 /* todo and notes.txt in Resources */ = {isa = PBXBuildFile; fileRef = E59078AB1AAF9740007A26D6 /* todo and notes.txt */; }; E5CC88BF1AB6514E00C8951F /* Credits.html in Resources */ = {isa = PBXBuildFile; fileRef = E5CC88BE1AB6514E00C8951F /* Credits.html */; }; + E5F8767F1ADE477000F9A362 /* tag.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = E52E87961ADD9AA900B08A74 /* tag.framework */; }; + E5F876801ADE49D800F9A362 /* tag.framework in CopyFiles */ = {isa = PBXBuildFile; fileRef = E52E87961ADD9AA900B08A74 /* tag.framework */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; }; /* End PBXBuildFile section */ /* Begin PBXContainerItemProxy section */ @@ -51,6 +51,19 @@ }; /* End PBXContainerItemProxy section */ +/* Begin PBXCopyFilesBuildPhase section */ + E5F8767A1ADE347300F9A362 /* CopyFiles */ = { + isa = PBXCopyFilesBuildPhase; + buildActionMask = 12; + dstPath = ""; + dstSubfolderSpec = 10; + files = ( + E5F876801ADE49D800F9A362 /* tag.framework in CopyFiles */, + ); + runOnlyForDeploymentPostprocessing = 0; + }; +/* End PBXCopyFilesBuildPhase section */ + /* Begin PBXFileReference section */ E502814C1ACBA133006D996C /* TableCellViewCheckmark.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = TableCellViewCheckmark.h; sourceTree = ""; }; E502814D1ACBA133006D996C /* TableCellViewCheckmark.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = TableCellViewCheckmark.m; sourceTree = ""; }; @@ -88,9 +101,9 @@ isa = PBXFrameworksBuildPhase; buildActionMask = 2147483647; files = ( + E5F8767F1ADE477000F9A362 /* tag.framework in Frameworks */, E52E879B1ADD9F8F00B08A74 /* AudioToolbox.framework in Frameworks */, E52E87991ADD9F2E00B08A74 /* AVFoundation.framework in Frameworks */, - E52E87971ADD9AA900B08A74 /* tag.framework in Frameworks */, E58A24B91AC9282300E8DDE4 /* AppKit.framework in Frameworks */, E5436D291AC3D44D0068E42E /* iTunesLibrary.framework in Frameworks */, ); @@ -176,6 +189,7 @@ E502D0B81A739D1500BE86E2 /* Sources */, E502D0B91A739D1500BE86E2 /* Frameworks */, E502D0BA1A739D1500BE86E2 /* Resources */, + E5F8767A1ADE347300F9A362 /* CopyFiles */, ); buildRules = ( ); @@ -237,7 +251,6 @@ E502D0CA1A739D1500BE86E2 /* Images.xcassets in Resources */, E5CC88BF1AB6514E00C8951F /* Credits.html in Resources */, E502D0CD1A739D1500BE86E2 /* Main.storyboard in Resources */, - E59078AC1AAF9741007A26D6 /* todo and notes.txt in Resources */, ); runOnlyForDeploymentPostprocessing = 0; }; diff --git a/TeslaTunes/Base.lproj/Main.storyboard b/TeslaTunes/Base.lproj/Main.storyboard index 7ae6540..cac867a 100644 --- a/TeslaTunes/Base.lproj/Main.storyboard +++ b/TeslaTunes/Base.lproj/Main.storyboard @@ -252,7 +252,6 @@ - @@ -260,6 +259,7 @@ + @@ -314,6 +314,22 @@ + + + + + + + + + + + + + + + + @@ -369,11 +385,15 @@ + + + + @@ -424,6 +444,7 @@ + diff --git a/TeslaTunes/CopyConvertDirs.h b/TeslaTunes/CopyConvertDirs.h index ac68032..9f888a9 100644 --- a/TeslaTunes/CopyConvertDirs.h +++ b/TeslaTunes/CopyConvertDirs.h @@ -24,6 +24,7 @@ typedef NS_ENUM(NSUInteger, DirOperation) { @public const NSURL *sourceURL; const NSURL *destinationURL; + const NSString *genre; } @end @@ -34,6 +35,7 @@ typedef NS_ENUM(NSUInteger, DirOperation) { @property (readonly) unsigned filesChecked; @property (readonly) unsigned filesToCopyConvert; @property (readonly) unsigned filesCopyConverted; +@property (readonly) unsigned filesMarkedForOrDeleted; @property (readonly) BOOL isProcessing; @property (readonly) BOOL scanReady; diff --git a/TeslaTunes/CopyConvertDirs.mm b/TeslaTunes/CopyConvertDirs.mm index 411d697..e665727 100644 --- a/TeslaTunes/CopyConvertDirs.mm +++ b/TeslaTunes/CopyConvertDirs.mm @@ -11,6 +11,10 @@ #import #import +#include +#include +#include + #include "flac_utils.h" @@ -25,6 +29,20 @@ return sanitizedString; } + + +void genreHack(const NSURL *url, const NSString *genre) { + //NSLog(@"genreHack: file \"%s\", genre \"%@\".",[url fileSystemRepresentation], genre); + TagLib::FileRef f([url fileSystemRepresentation], false); + TagLib::Tag *t = f.tag(); + t->setGenre([genre UTF8String]); + //NSLog(@"Set genre of %@ to %s", url, t->genre().toCString(true) ); + if (!f.save()){ + NSLog(@"warning: couldn't set genre of %@ to %s", url, t->genre().toCString(true)); + } +} + + // make the destination filename out of the base path of destination and the relative path of the new item NSURL* makeDestURL(const NSURL *dstBasePath, const NSURL *basePathToStrip, const NSURL* srcURL) { // Need to create the destination file/dir URL to check it's existance. Use dst and the relative path @@ -54,9 +72,9 @@ relPathRange.location = basePathComponents.count; // start the relative path at the end of the base path relPathRange.length = srcPathComponents.count - basePathComponents.count; } - NSURL *dstURL = [NSURL fileURLWithPathComponents:[[dstBasePath pathComponents] - arrayByAddingObjectsFromArray:[srcPathComponents - subarrayWithRange:relPathRange]]]; + NSURL *dstURL = [NSURL fileURLWithPathComponents: + [[dstBasePath pathComponents] arrayByAddingObjectsFromArray: + [srcPathComponents subarrayWithRange:relPathRange]]]; return dstURL; } @@ -105,12 +123,13 @@ - (void)addOperationWithClosure:(void (^)(NSBlockOperation *operation))block @implementation FileOp -- (instancetype)initWithSourceURL:(const NSURL *)s DestinationURL:(const NSURL*)d +- (instancetype)initWithSourceURL:(const NSURL *)s DestinationURL:(const NSURL*)d withGenre: (const NSString *) g { self = [super init]; if (self) { sourceURL=s; destinationURL=d; + genre=g; } return self; } @@ -121,6 +140,7 @@ @interface CopyConvertDirs () @property (readwrite) unsigned filesChecked; @property (readwrite) unsigned filesToCopyConvert; @property (readwrite) unsigned filesCopyConverted; +@property (readwrite) unsigned filesMarkedForOrDeleted; @property (readwrite) BOOL scanReady; @@ -188,6 +208,7 @@ - (void) startOperationOnDir: (DirOperation) opType withPlaylistSelections:(cons // if we had a scan ready from before... well we won't after we start whatever we are doing here self.scanReady = NO; self.filesCopyConverted = 0; + self.filesMarkedForOrDeleted = 0; _skippedExtensions = [[NSCountedSet alloc] init ]; _copiedExtensions = [[NSCountedSet alloc] init]; @@ -224,14 +245,14 @@ - (void) startOperationOnDir: (DirOperation) opType withPlaylistSelections:(cons } -- (void) storeScannedForCopyWithSource:(const NSURL*)s Destination:(const NSURL*)d { - FileOp *newOp = [[FileOp alloc] initWithSourceURL:s DestinationURL: d]; +- (void) storeScannedForCopyWithSource:(const NSURL*)s Destination:(const NSURL*)d withGenre: (const NSString *) genre { + FileOp *newOp = [[FileOp alloc] initWithSourceURL:s DestinationURL: d withGenre: genre]; [copyOps addObject:newOp]; [self.copiedExtensions addObject:s.pathExtension]; } -- (void) storeScannedForConvertWithSource:(const NSURL*)s Destination:(const NSURL*)d { - FileOp *newOp = [[FileOp alloc] initWithSourceURL:s DestinationURL: d]; +- (void) storeScannedForConvertWithSource:(const NSURL*)s Destination:(const NSURL*)d withGenre: (const NSString *) genre { + FileOp *newOp = [[FileOp alloc] initWithSourceURL:s DestinationURL: d withGenre: genre]; [convertOps addObject:newOp]; [self.copiedExtensions addObject:@"m4a->flac (Apple Lossless -> flac)"]; } @@ -248,28 +269,61 @@ -(void) chillTillQueueLengthIsReasonable:(NSOperationQueue*)q{ } } --(void) convertWithSource:(const NSURL*)s Destination:(const NSURL*)d { +BOOL makeDirsAsNeeded(const NSURL* d) { + NSError *theError; + NSFileManager *fileManager = [NSFileManager defaultManager]; + if (NO ==[fileManager createDirectoryAtURL:[d URLByDeletingLastPathComponent] + withIntermediateDirectories:YES attributes:nil error:&theError]) { + // createDirectory returns YES even if the dir already exists because + // the "withIntermediateDirectories" flag is set according to the documentation, + // so if it fails, it's a real issue... BUT turns out the documentation may be wrong. + // In testing, I sometimes got dir already exists errors presumably when multiple threads + // were trying this same operation with several songs on a new album. + // So not sure what I should really do -- added all the code below to try to figure out + // if it's a real error. + BOOL isDir; + if ([theError code] != NSFileWriteFileExistsError) { + // an error, so let's skip the conversion + NSLog(@"warning: couldn't make target directory; %@ code (%ld). Skipping handling of %@", + [theError localizedDescription], (long)[theError code], d); + return NO; + } + if ([fileManager fileExistsAtPath:[[d URLByDeletingLastPathComponent] path] + isDirectory:&isDir]) { + if (!isDir) { + NSLog(@"warning: when handling \"%@\", tried to make the directory to hold it, " + "but a file already existed by that name that was not a directory. Skipping " + "conversion.", d); + return NO; + } + } else { + NSLog(@"warning: beats me what's happening. Report this. Tried to create directory to " + "hold %@, but got a file exists error, yet I looked and the file doesn't exist. " + "Skipping conversion.)", d); + return NO; + } + } + + // if we're here either the directory was made, or it wasn't made this time, but it + // exists and is a directory. + return YES; +} + +-(void) convertWithSource:(const NSURL*)s Destination:(const NSURL*)d withGenre: (const NSString*) genre { [self chillTillQueueLengthIsReasonable:opSubQ]; // make the parent directory (and all other intermediate directories) if needed, then copy if (isCancelled) return; [opSubQ addOperationWithBlock:^(void){ - NSError *theError; - NSFileManager *fileManager = [NSFileManager defaultManager]; - if (NO ==[fileManager createDirectoryAtURL:[d URLByDeletingLastPathComponent] - withIntermediateDirectories:YES attributes:nil error:&theError]) { - // createDirectory returns YES even if the dir already exists because - // the "withIntermediateDirectories" flag is set, so if it fails, it's a real issue - NSLog(@"warning: couldn't make target directory, error was domain %@, desc %@ - fail reason %@, code (%ld)", - [theError domain], [theError localizedDescription], [theError localizedFailureReason], (long)[theError code]); - //return; - } - - //NSLog(@"\nConverting Apple Lossless file->flac, %s, %s", s.fileSystemRepresentation, d.fileSystemRepresentation); - if (ConvertAlacToFlac(s, d, &(isCancelled))) { - [self.copiedExtensions addObject:@"m4a->flac (Apple Lossless -> flac)"]; - [self willChangeValueForKey:@"filesCopyConverted"]; - ++_filesCopyConverted; - [self didChangeValueForKey:@"filesCopyConverted"]; + if (makeDirsAsNeeded(d)) { + //NSLog(@"\nConverting Apple Lossless file->flac, %s, %s", s.fileSystemRepresentation, d.fileSystemRepresentation); + if (ConvertAlacToFlac([s URLByStandardizingPath], [d URLByStandardizingPath], &(isCancelled))) { + [self.copiedExtensions addObject:@"m4a->flac (Apple Lossless -> flac)"]; + [self willChangeValueForKey:@"filesCopyConverted"]; + ++_filesCopyConverted; + [self didChangeValueForKey:@"filesCopyConverted"]; + if (genre) + genreHack(d, genre); + } } }]; //NSLog(@"convert queued, current opSubQ depth:%lu", (unsigned long)opSubQ.operationCount); @@ -277,34 +331,26 @@ -(void) convertWithSource:(const NSURL*)s Destination:(const NSURL*)d { } --(void) copyWithSource:(const NSURL*)s Destination:(const NSURL*)d { +-(void) copyWithSource:(const NSURL*)s Destination:(const NSURL*)d withGenre: (const NSString*) genre { [self chillTillQueueLengthIsReasonable:opSubQ]; // make the parent directory (and all other intermediate directories) if needed, then copy [opSubQ addOperationWithBlock:^(void){ - NSError *theError; - NSFileManager *fileManager = [NSFileManager defaultManager]; - if (NO ==[fileManager createDirectoryAtURL:[d URLByDeletingLastPathComponent] - withIntermediateDirectories:YES attributes:nil error:&theError]) { - // createDirectory returns YES even if the dir already exists because - // the "withIntermediateDirectories" flag is set, so if it fails, it's a real issue - // TODO: in one run early in development, the above statement was not true - got - // dir already exists errors presumably when multiple threads were trying this same operation - // with several songs on a new album. So not sure what I should really do -- for now trying to - // continue rather than return - the copy should fail too if it's an issue. - NSLog(@"warning: couldn't make target directory, error was domain %@, desc %@ - fail reason %@, code (%ld)", - [theError domain], [theError localizedDescription], [theError localizedFailureReason], (long)[theError code]); - // return; - } - - //NSLog(@"\nCopying %s to %s", s.fileSystemRepresentation, d.fileSystemRepresentation); - if (![fileManager copyItemAtURL:[s URLByStandardizingPath] toURL:[d URLByStandardizingPath] error:&theError]){ - NSLog(@"Couldn't copy file \"%@\" to \"%@\", %@ - %@, (%ld)", s, d, - [theError localizedDescription], [theError localizedFailureReason], (long)[theError code] ); - } else { - [self.copiedExtensions addObject:s.pathExtension]; - [self willChangeValueForKey:@"filesCopyConverted"]; - ++_filesCopyConverted; - [self didChangeValueForKey:@"filesCopyConverted"]; + if (makeDirsAsNeeded(d)) { + NSError *theError; + NSFileManager *fileManager = [NSFileManager defaultManager]; + + //NSLog(@"\nCopying %s to %s", s.fileSystemRepresentation, d.fileSystemRepresentation); + if (![fileManager copyItemAtURL:[s URLByStandardizingPath] toURL:[d URLByStandardizingPath] error:&theError]){ + NSLog(@"Couldn't copy file \"%@\" to \"%@\", %@ (%ld)", s, d, + [theError localizedDescription], (long)[theError code] ); + } else { + [self.copiedExtensions addObject:s.pathExtension]; + [self willChangeValueForKey:@"filesCopyConverted"]; + ++_filesCopyConverted; + [self didChangeValueForKey:@"filesCopyConverted"]; + if (genre) + genreHack(d, genre); + } } }]; //NSLog(@"copy queued, current opSubQ depth:%lu", (unsigned long)opSubQ.operationCount); @@ -315,40 +361,30 @@ - (void) processScannedItems { for (FileOp *f in copyOps) { @autoreleasepool { if (isCancelled) break; - [self copyWithSource:f->sourceURL Destination:f->destinationURL]; + [self copyWithSource:f->sourceURL + Destination:f->destinationURL withGenre: f->genre]; } } for (FileOp *f in convertOps) { @autoreleasepool { if (isCancelled) break; - [self convertWithSource:f->sourceURL Destination:f->destinationURL]; + [self convertWithSource:f->sourceURL + Destination:f->destinationURL withGenre: f->genre]; } } for (NSURL *f in delOps) { if (isCancelled) break; + ++self.filesMarkedForOrDeleted; NSError *e; - if (![[NSFileManager defaultManager] removeItemAtURL:[f standardizedURL] error:&e]) { + if (![[NSFileManager defaultManager] removeItemAtURL:f + error:&e]) { // check and report potential cleanup failure - note that if f is nil, the operation returns YES // TODO: see above - NSLog(@"Couldn't remove from playlist folder item URL \"%@\".", [f standardizedURL]); + NSLog(@"Couldn't remove from playlist folder item URL \"%@\". %@ (%ld)", f, [e localizedDescription], [e code]); } } } -#include -#include -#include - -void genreHack(const NSURL *url, const NSString *genre) { - TagLib::FileRef f([url fileSystemRepresentation], false); - TagLib::Tag *t = f.tag(); - t->setGenre([genre UTF8String]); - NSLog(@"Set genre of %@ to %s", url, t->genre().toCString(true) ); - if (!f.save()){ - NSLog(@"warning: could set genre of %@ to %s", url, t->genre().toCString(true)); - } -} - // Returns the URL of the processed file at the destination, or nil in the event of error/cancellation // or simply that the file is not a handled type. Note it will also return the filename in // the event it is a handled filetype and does not need to be copied because it is already there. @@ -382,9 +418,9 @@ - (NSURL *) processFileURL:(const NSURL *) file toDestination: destinationFile [self didChangeValueForKey:@"filesToCopyConvert"]; if (scanOnly){ - [self storeScannedForConvertWithSource:file Destination:outURL]; + [self storeScannedForConvertWithSource:file Destination:outURL withGenre: genre]; } else { - [self convertWithSource:file Destination:outURL]; + [self convertWithSource:file Destination:outURL withGenre: genre]; } } else if ([extensionsToCopy containsObject:[file.pathExtension lowercaseString]]) { // NSLog(@"checking to see if %@ exists at dest path %@", fileURL, destFileURL); @@ -397,9 +433,9 @@ - (NSURL *) processFileURL:(const NSURL *) file toDestination: destinationFile ++_filesToCopyConvert; [self didChangeValueForKey:@"filesToCopyConvert"]; if (scanOnly) { - [self storeScannedForCopyWithSource:file Destination:destinationFile]; + [self storeScannedForCopyWithSource:file Destination:destinationFile withGenre: genre]; } else { - [self copyWithSource:file Destination:destinationFile]; + [self copyWithSource:file Destination:destinationFile withGenre: genre]; } outURL = destinationFile; } else { @@ -408,10 +444,8 @@ - (NSURL *) processFileURL:(const NSURL *) file toDestination: destinationFile [self.skippedExtensions addObject:file.pathExtension]; return nil; } - - if (genre) genreHack(outURL, genre); - return destinationFile; + return outURL; } } @@ -427,13 +461,17 @@ - (BOOL) pruneFilesNotInSet: (const NSSet*) fileSet inDirectory: (const NSURL*) { // this could happen without it being an error in the event the // destination playlist doesn't exist yet, for example when doing - // a scan only for the first time on a playlist. If that's the case, + // a scan only for the first time on a playlist, or simply getting the operations to copy + // a playlist queued up behind other ops in the queue, then hitting the prune code before + // any of them had gotten to execute yet. Regardless, if that's the case, // it isn't an error, and there obviously won't be anything to remove. - if (error && !scanOnly) { - NSLog(@"Error when looking for destination playlist folder to prune [Error] %@ (%@)", error, url); + if (error && (error.code != NSFileReadNoSuchFileError)) { + NSLog(@"Error when looking for destination playlist folder \"%@\" to prune. %@ (%ld)%@", + dir, [error localizedDescription], + [error code], url? [NSString stringWithFormat:@" Happened when looking at \"%@\"",url] : @""); return NO; } - + // either no error?!? or error code is no such file which probably means the scenario in the above comment happened return YES; }]; @@ -442,17 +480,32 @@ - (BOOL) pruneFilesNotInSet: (const NSSet*) fileSet inDirectory: (const NSURL*) return NO; } - if (![fileSet containsObject:[fileURL lastPathComponent]]) { + if (![fileSet containsObject:[fileURL URLByStandardizingPath]]) { // if scan only, queue the delete, if not then do the delete + ++self.filesMarkedForOrDeleted; + + // If the URL is a directory then the delete will recursively remove any files/dirs in it, + // so tell the enumerator to skip descendents so we don't work harder than needed + // Note that under current design there should be no subdirectories, so this would be cruft + // left over from manual moving things around (or this comment needs updating and hope we aren't + // deleting something wrong! + NSNumber *isDirectory; + [fileURL getResourceValue:&isDirectory forKey:NSURLIsDirectoryKey error:nil]; + if ([isDirectory boolValue]) { + [enumerator skipDescendants]; + } + if (scanOnly) { + //NSLog(@"adding op to remove \"%@\"", fileURL); [delOps addObject:fileURL]; } else { - //NSLog(@"removing %@", [fileURL standardizedURL]); + //NSLog(@"removing %@", fileURL); NSError *e; - if (![[NSFileManager defaultManager] removeItemAtURL:[fileURL standardizedURL] error:&e]) { + if (![[NSFileManager defaultManager] removeItemAtURL:fileURL error:&e]) { // check and report potential cleanup failure - note that if f is nil, the operation returns YES // TODO: see above - NSLog(@"Couldn't remove from playlist folder item URL \"%@\".", [fileURL standardizedURL]); + NSLog(@"Couldn't remove from playlist folder item URL \"%@\". %@ (%ld)", + fileURL, [e localizedDescription], [e code]); } } @@ -471,8 +524,8 @@ - (BOOL) pruneFilesNotInSet: (const NSSet*) fileSet inDirectory: (const NSURL*) // Return NO if processing was interrupted, either by error, or by cancel flag being set, YES otherwise - (BOOL) processPlaylistNode:(PlaylistNode *) node toDestinationDirectoryURL: destinationDir performScanOnly: (BOOL) scanOnly { - NSURL *destinationFolderForPlaylist = [destinationDir - URLByAppendingPathComponent: sanitizeFilename(node.playlist.name)]; + NSURL *destinationFolderForPlaylist = [[destinationDir + URLByAppendingPathComponent: sanitizeFilename(node.playlist.name)] URLByStandardizingPath]; //NSLog(@"playlist %@ was selected and will be copied to %s", node.playlist.name, destinationFolderForPlaylist.fileSystemRepresentation); NSMutableSet *playlistFilenames = [[NSMutableSet alloc] init]; @@ -524,7 +577,7 @@ - (BOOL) processPlaylistNode:(PlaylistNode *) node toDestinationDirectoryURL: de idx, track.title, track.artist.name, track.album.title, [track.location pathExtension] ]); - NSURL *destFileURL = [destinationFolderForPlaylist URLByAppendingPathComponent: filename]; + NSURL *destFileURL = [[destinationFolderForPlaylist URLByAppendingPathComponent: filename] URLByStandardizingPath]; #if 0 NSLog(@"Copying track %@ from location type %lu, locations %s to %s", track.title, @@ -535,9 +588,10 @@ - (BOOL) processPlaylistNode:(PlaylistNode *) node toDestinationDirectoryURL: de if (track.location) { NSURL *result = [self processFileURL:track.location toDestination: destFileURL performScanOnly:scanOnly setGenre:self.hackGenre? node.playlist.name:nil]; - if (result) - [playlistFilenames addObject:[result lastPathComponent]]; - + if (result) { + [playlistFilenames addObject:result]; + //NSLog(@"adding to playlistFilename url \"%@\"", result); + } } } } @@ -584,7 +638,8 @@ - (void) processOpsWithPlaylistSelections: (const PlaylistSelections *)playlistS errorHandler:^BOOL(NSURL *url, NSError *error) { if (error) { - NSLog(@"[Error] %@ (%@)", error, url); + NSLog(@"warning: error trying to scan source directory %@. %@ (%@)", + sourceDir, error, url); return NO; } diff --git a/TeslaTunes/ViewController.h b/TeslaTunes/ViewController.h index 930dab3..de28752 100644 --- a/TeslaTunes/ViewController.h +++ b/TeslaTunes/ViewController.h @@ -19,16 +19,18 @@ @property (weak) IBOutlet NSPathControl *destinationPath; @property (weak) IBOutlet NSButton *doItButton; +@property (weak) IBOutlet NSProgressIndicator *progressIndicator; + @property (weak) IBOutlet NSPopUpButton *opTypeButton; @property (weak) IBOutlet NSMenuItem *CCScanResultsPopupItem; @property NSString *report; @property (weak) IBOutlet NSTextField *numberOfFilesScannedLabel; +@property (weak) IBOutlet NSTextField *numberOfFilesForDeletionLabel; + @property (weak) IBOutlet NSTextField *numberOfFilesToCopyOrConvertLabel; @property (weak) IBOutlet NSTextField *numberOfFilesCopiedOrConvertedLabel; -@property (weak) IBOutlet NSProgressIndicator *progressIndicator; - @property CopyConvertDirs *ccDirs; @end diff --git a/TeslaTunes/ViewController.m b/TeslaTunes/ViewController.m index 654f00d..de84533 100644 --- a/TeslaTunes/ViewController.m +++ b/TeslaTunes/ViewController.m @@ -88,6 +88,7 @@ -(void) updateProgress { [self.numberOfFilesScannedLabel setIntegerValue: self.ccDirs.filesChecked]; [self.numberOfFilesToCopyOrConvertLabel setIntegerValue: self.ccDirs.filesToCopyConvert]; [self.numberOfFilesCopiedOrConvertedLabel setIntegerValue: self.ccDirs.filesCopyConverted]; + [self.numberOfFilesForDeletionLabel setIntegerValue: self.ccDirs.filesMarkedForOrDeleted]; } -(void) updateProgressTimerFired:(NSTimer *)t { [self updateProgress]; diff --git a/TeslaTunes/todo and notes.txt b/TeslaTunes/todo and notes.txt index 6518159..324b2e7 100644 --- a/TeslaTunes/todo and notes.txt +++ b/TeslaTunes/todo and notes.txt @@ -48,3 +48,13 @@ the create directory with intermediates failed saying the directory already exis left running overnight, an open failed, assert fired for empty metadata - example of error path to be handled. file locations can time out, for example network drive flakiness, USB drive getting pulled out, etc. (for that matter, they can get full, have an error, etc.) + + + +notes: +Saw an issue with unicode(?) UTF-8 representations, see: +adding to playlistFilename url "file:///Users/rarnold/devel/TeslaTunesProjects/TeslaTunesCL/testdir/dest/Playlists/Redemption%20Songs%20A/02-Whatever%20It%20Takes...-Sine%CC%81ad%20Lohan-No%20Mermaid.flac" - from URL "file:///Users/rarnold/devel/TeslaTunesProjects/TeslaTunesCL/testdir/dest/Playlists/Redemption%20Songs%20A/02-Whatever%20It%20Takes...-Sin%C3%A9ad%20Lohan-No%20Mermaid.flac" + +The 2 urls came from the same url. The first is what you get with [url URLbyStandardizingPath] and the second is just a print of the url. The accent character on Sinead (above the e) gets represented two different ways. +I resolved by switching the writes and checks for playlist items to remove to all store the [url URLbyStandardizingPath] version. +