diff --git a/Audio/PluginController.mm b/Audio/PluginController.mm index 28b39c1c5..5935a2846 100644 --- a/Audio/PluginController.mm +++ b/Audio/PluginController.mm @@ -37,6 +37,12 @@ static void cache_run(); +static std::string cache_key(NSURL *url, bool skipCue) { + std::string key = [[url absoluteString] UTF8String]; + key.append(skipCue ? "\x1fskip-cue" : "\x1fwith-cue"); + return key; +} + static void cache_init() { id dataStoreClass = NSClassFromString(@"RedundantPlaylistDataStore"); // CogAudio Cache_Data_Store = [dataStoreClass new]; @@ -56,12 +62,12 @@ static void cache_deinit() { Cache_Data_Store = nil; } -static void cache_insert_properties(NSURL *url, NSDictionary *properties) { +static void cache_insert_properties(NSURL *url, BOOL skipCue, NSDictionary *properties) { if(properties == nil) return; std::lock_guard lock(*Cache_Lock); - std::string path = [[url absoluteString] UTF8String]; + std::string path = cache_key(url, skipCue); properties = [Cache_Data_Store coalesceEntryInfo:properties]; Cached_Metadata &entry = Cache_List[path]; @@ -70,12 +76,12 @@ static void cache_insert_properties(NSURL *url, NSDictionary *properties) { entry.time_accessed = std::chrono::steady_clock::now(); } -static void cache_insert_metadata(NSURL *url, NSDictionary *metadata) { +static void cache_insert_metadata(NSURL *url, BOOL skipCue, NSDictionary *metadata) { if(metadata == nil) return; std::lock_guard lock(*Cache_Lock); - std::string path = [[url absoluteString] UTF8String]; + std::string path = cache_key(url, skipCue); metadata = [Cache_Data_Store coalesceEntryInfo:metadata]; Cached_Metadata &entry = Cache_List[path]; @@ -84,10 +90,10 @@ static void cache_insert_metadata(NSURL *url, NSDictionary *metadata) { entry.time_accessed = std::chrono::steady_clock::now(); } -static NSDictionary *cache_access_properties(NSURL *url) { +static NSDictionary *cache_access_properties(NSURL *url, BOOL skipCue) { std::lock_guard lock(*Cache_Lock); - std::string path = [[url absoluteString] UTF8String]; + std::string path = cache_key(url, skipCue); Cached_Metadata &entry = Cache_List[path]; @@ -99,10 +105,10 @@ static void cache_insert_metadata(NSURL *url, NSDictionary *metadata) { return nil; } -static NSDictionary *cache_access_metadata(NSURL *url) { +static NSDictionary *cache_access_metadata(NSURL *url, BOOL skipCue) { std::lock_guard lock(*Cache_Lock); - std::string path = [[url absoluteString] UTF8String]; + std::string path = cache_key(url, skipCue); Cached_Metadata &entry = Cache_List[path]; @@ -682,7 +688,7 @@ - (NSDictionary *)metadataForURL:(NSURL *)url skipCue:(BOOL)skip { [urlScheme isEqualToString:@"https"]) return nil; - NSDictionary *cacheData = cache_access_metadata(url); + NSDictionary *cacheData = cache_access_metadata(url, skip); if(cacheData) return cacheData; do { @@ -755,7 +761,7 @@ - (NSDictionary *)metadataForURL:(NSURL *)url skipCue:(BOOL)skip { } } - cache_insert_metadata(url, cacheData); + cache_insert_metadata(url, skip, cacheData); return cacheData; } @@ -768,7 +774,7 @@ - (NSDictionary *)propertiesForURL:(NSURL *)url skipCue:(BOOL)skip { NSDictionary *properties = nil; - properties = cache_access_properties(url); + properties = cache_access_properties(url, skip); if(properties) return properties; NSString *ext = [url pathExtension]; @@ -783,7 +789,7 @@ - (NSDictionary *)propertiesForURL:(NSURL *)url skipCue:(BOOL)skip { if([readers count] > 1) { properties = [CogPropertiesReaderMulti propertiesForSource:source readers:readers]; if(properties != nil && [properties count]) { - cache_insert_properties(url, properties); + cache_insert_properties(url, skip, properties); return properties; } } else { @@ -795,7 +801,7 @@ - (NSDictionary *)propertiesForURL:(NSURL *)url skipCue:(BOOL)skip { if([readers count] > 1) { properties = [CogPropertiesReaderMulti propertiesForSource:source readers:readers]; if(properties != nil && [properties count]) { - cache_insert_properties(url, properties); + cache_insert_properties(url, skip, properties); return properties; } } else { @@ -809,7 +815,7 @@ - (NSDictionary *)propertiesForURL:(NSURL *)url skipCue:(BOOL)skip { properties = [propertiesReader propertiesForSource:source]; if(properties != nil && [properties count]) { - cache_insert_properties(url, properties); + cache_insert_properties(url, skip, properties); return properties; } } @@ -826,7 +832,7 @@ - (NSDictionary *)propertiesForURL:(NSURL *)url skipCue:(BOOL)skip { [decoder close]; NSDictionary *cacheData = [NSDictionary dictionaryByMerging:properties with:metadata]; - cache_insert_properties(url, cacheData); + cache_insert_properties(url, skip, cacheData); return cacheData; } } diff --git a/Playlist/PlaylistLoader.m b/Playlist/PlaylistLoader.m index e6a29fceb..7195554cb 100644 --- a/Playlist/PlaylistLoader.m +++ b/Playlist/PlaylistLoader.m @@ -334,6 +334,31 @@ static inline BOOL isCueSheetTrackURL(NSURL *url) { return cueSheetValueHasContent(properties[@"cuesheet"]); } +static NSDictionary *entryInfoForURL(NSURL *url) { + BOOL cueSheetTrack = isCueSheetTrackURL(url); + // Resolve logical CUE metadata before opening a decoder for properties. A + // properties lookup may need to inspect the shared audio file, whose tags do + // not describe an individual CUE fragment. + NSDictionary *metadata = cueSheetTrack ? [AudioMetadataReader metadataForURL:url] : nil; + NSDictionary *properties = [AudioPropertiesReader propertiesForURL:url]; + if(!properties) { + return nil; + } + + if(!metadata) { + metadata = [AudioMetadataReader metadataForURL:url] ?: @{}; + } + if(cueSheetTrack) { + // Decoder properties may include tags from the shared album file. Apply + // the logical CUE dictionary last so its title and track fields replace + // those values unconditionally. + NSMutableDictionary *entryInfo = [properties mutableCopy]; + [entryInfo addEntriesFromDictionary:metadata]; + return [entryInfo copy]; + } + return [NSDictionary dictionaryByMerging:properties with:metadata]; +} + - (void)beginProgress:(NSString *)localizedDescription { while(playbackController.progressOverall) { [[NSRunLoop currentRunLoop] runUntilDate:[NSDate dateWithTimeIntervalSinceNow:0.01]]; @@ -393,24 +418,11 @@ - (void)setProgressJobStatus:(double)status { } } -- (BOOL)seedInitialCueMetadataForEntry:(PlaylistEntry *)entry url:(NSURL *)url { +- (NSDictionary *)initialCueInfoForURL:(NSURL *)url { if(!isCueSheetTrackURL(url)) { - return NO; + return nil; } - - NSDictionary *properties = [AudioPropertiesReader propertiesForURL:url]; - if(!properties) { - return NO; - } - - NSDictionary *metadata = [AudioMetadataReader metadataForURL:url]; - NSDictionary *entryInfo = [NSDictionary dictionaryByMerging:properties with:metadata]; - - dispatch_sync_reentrant(dispatch_get_main_queue(), ^{ - [entry setMetadata:entryInfo]; - }); - - return YES; + return entryInfoForURL(url); } + (NSString *)keyForPath:(NSString *)path { @@ -765,7 +777,7 @@ - (NSArray *)insertURLs:(NSArray *)urls atIndex:(NSInteger)index sort:(BOOL)sort NSInteger i = 0; __block NSMutableArray *entries = [NSMutableArray arrayWithCapacity:count]; - NSMutableArray *preloadedEntries = [NSMutableArray new]; + NSMutableArray *preloadedEntryInfo = [NSMutableArray new]; for(NSURL *url in validURLs) { __block PlaylistEntry *pe; @@ -782,8 +794,9 @@ - (NSArray *)insertURLs:(NSArray *)urls atIndex:(NSInteger)index sort:(BOOL)sort [addItemTask finish]; }); - if([self seedInitialCueMetadataForEntry:pe url:url]) { - [preloadedEntries addObject:pe]; + NSDictionary *entryInfo = [self initialCueInfoForURL:url]; + if(entryInfo) { + [preloadedEntryInfo addObject:@[pe, entryInfo]]; } [entries addObject:pe]; @@ -824,7 +837,12 @@ - (NSArray *)insertURLs:(NSArray *)urls atIndex:(NSInteger)index sort:(BOOL)sort [self->playlistController setSelectsInsertedObjects:NO]; @try { [self->playlistController insertObjects:entries atArrangedObjectIndexes:is]; - for(PlaylistEntry *pe in preloadedEntries) { + // Apply cue metadata only after the array controller owns each managed + // object. Otherwise it can miss the per-entry KVO changes and reused row + // bindings may display the final cue title for every inserted track. + for(NSArray *preloadedInfo in preloadedEntryInfo) { + PlaylistEntry *pe = preloadedInfo[0]; + [pe setMetadata:preloadedInfo[1]]; [self->playlistController firstSawTrackWithoutReload:pe]; } } @@ -1003,14 +1021,10 @@ - (void)loadInfoForEntries:(NSArray *)entries { } @try { - NSDictionary *entryProperties = [AudioPropertiesReader propertiesForURL:url]; - if(entryProperties == nil) + NSDictionary *entryInfo = entryInfoForURL(url); + if(entryInfo == nil) return; - NSDictionary *entryMetadata = [AudioMetadataReader metadataForURL:url]; - - NSDictionary *entryInfo = [NSDictionary dictionaryByMerging:entryProperties with:entryMetadata]; - [weakLock lock]; @autoreleasepool { entryInfo = [weakDataStore coalesceEntryInfo:entryInfo]; @@ -1163,12 +1177,10 @@ - (void)syncLoadInfoForEntries:(NSArray *)entries { } @try { - NSDictionary *entryProperties = [AudioPropertiesReader propertiesForURL:pe.url]; - if(entryProperties == nil) + NSDictionary *entryInfo = entryInfoForURL(pe.url); + if(entryInfo == nil) return; - NSDictionary *entryInfo = [NSDictionary dictionaryByMerging:entryProperties with:[AudioMetadataReader metadataForURL:pe.url]]; - [pe setMetadata:entryInfo]; [playlistController firstSawTrack:pe]; diff --git a/Plugins/CueSheet/CueSheetDecoder.m b/Plugins/CueSheet/CueSheetDecoder.m index cafac9a45..6e02de0a0 100644 --- a/Plugins/CueSheet/CueSheetDecoder.m +++ b/Plugins/CueSheet/CueSheetDecoder.m @@ -12,8 +12,6 @@ #import "CueSheetContainer.h" #import "CueSheetMetadataReader.h" -#import "NSDictionary+Merge.h" - #import "Logging.h" @implementation CueSheetDecoder @@ -56,8 +54,11 @@ - (NSDictionary *)metadata { NSDictionary *decoderMetadata = [decoder metadata]; if(decoderMetadata != nil) { // Cue-sheet fields describe this logical track and take priority over - // the metadata shared by the underlying audio file. - return [metadata dictionaryByMergingWith:decoderMetadata]; + // the metadata shared by the underlying audio file. Merge explicitly + // to avoid Objective-C category selector collisions between plug-ins. + NSMutableDictionary *mergedMetadata = [decoderMetadata mutableCopy]; + [mergedMetadata addEntriesFromDictionary:metadata]; + return [mergedMetadata copy]; } } return metadata; diff --git a/Plugins/CueSheet/CueSheetMetadataReader.m b/Plugins/CueSheet/CueSheetMetadataReader.m index d1168f664..ac314580a 100644 --- a/Plugins/CueSheet/CueSheetMetadataReader.m +++ b/Plugins/CueSheet/CueSheetMetadataReader.m @@ -12,7 +12,6 @@ #import "CueSheet.h" #import "AudioMetadataReader.h" -#import "NSDictionary+Merge.h" #import "NSDictionary+Optional.h" @implementation CueSheetMetadataReader @@ -94,8 +93,13 @@ + (NSDictionary *)metadataForURL:(NSURL *)url { NSDictionary *cuesheetMetadata = [CueSheetMetadataReader processDataForTrack:track]; // Cue-sheet fields describe this logical track and take priority over - // the metadata shared by the underlying audio file. - return [cuesheetMetadata dictionaryByMergingWith:fileMetadata]; + // the metadata shared by the underlying audio file. Do not use the + // NSDictionary merge category here: plug-ins load their own category + // implementations into one Objective-C runtime, so selector collisions + // can silently change which dictionary wins based on bundle load order. + NSMutableDictionary *metadata = fileMetadata ? [fileMetadata mutableCopy] : [NSMutableDictionary new]; + [metadata addEntriesFromDictionary:cuesheetMetadata]; + return [metadata copy]; } }