Skip to content

fix(cue): preserve per-track metadata and isolate cache variants #514

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.

Sign up for GitHub

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
markojurkovic wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(cue): preserve per-track metadata and isolate cache variants #514

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
36 changes: 21 additions & 15 deletions Audio/PluginController.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand All @@ -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<std::mutex> 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];
Expand All @@ -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<std::mutex> 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];
Expand All @@ -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<std::mutex> lock(*Cache_Lock);

std::string path = [[url absoluteString] UTF8String];
std::string path = cache_key(url, skipCue);

Cached_Metadata &entry = Cache_List[path];

Expand All @@ -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<std::mutex> lock(*Cache_Lock);

std::string path = [[url absoluteString] UTF8String];
std::string path = cache_key(url, skipCue);

Cached_Metadata &entry = Cache_List[path];

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -755,7 +761,7 @@ - (NSDictionary *)metadataForURL:(NSURL *)url skipCue:(BOOL)skip {
}
}

cache_insert_metadata(url, cacheData);
cache_insert_metadata(url, skip, cacheData);
return cacheData;
}

Expand All @@ -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];
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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;
}
}
Expand All @@ -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;
}
}
Expand Down
72 changes: 42 additions & 30 deletions Playlist/PlaylistLoader.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]];
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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;

Expand All @@ -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];
Expand Down Expand Up @@ -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];
}
}
Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -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];

Expand Down
9 changes: 5 additions & 4 deletions Plugins/CueSheet/CueSheetDecoder.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
#import "CueSheetContainer.h"
#import "CueSheetMetadataReader.h"

#import "NSDictionary+Merge.h"

#import "Logging.h"

@implementation CueSheetDecoder
Expand Down Expand Up @@ -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;
Expand Down
10 changes: 7 additions & 3 deletions Plugins/CueSheet/CueSheetMetadataReader.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
#import "CueSheet.h"

#import "AudioMetadataReader.h"
#import "NSDictionary+Merge.h"
#import "NSDictionary+Optional.h"

@implementation CueSheetMetadataReader
Expand Down Expand Up @@ -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];
}
}

Expand Down
Loading