[vlc-devel] [vlc-commits] macosx: basic implementation of the playlist data model

[David Fuhrmann]

> Am 31.01.2019 um 19:05 schrieb Felix Paul Kühne <fkuehne at videolan.org>:
> 
> Hi David,
> 
> Thanks for the feedback!
> 
>> On 31. Jan 2019, at 19:00, David Fuhrmann <david.fuhrmann at gmail.com> wrote:
>> 
>>> +static void
>>> +cb_playlist_current_item_changed(vlc_playlist_t *playlist,
>>> +                                 ssize_t index,
>>> +                                 void *p_data)
>>> +{
>>> +    if (index == UINT64_MAX) {
>>> +        NSLog(@"%s: no current item", __func__);
>>> +    }
>>> +    VLCPlaylistController *playlistController = (__bridge VLCPlaylistController *)p_data;
>>> +    [playlistController currentPlaylistItemChanged:index];
>>> +
>>> +    NSLog(@"%s: index: %zu", __func__, index);
>>> +}
>> 
>> I believe you might need to post all of that into the main thread, no? If so, it should be directly done in the callback, not in any later stage,
> 
> This was changed in subsequent commits :)
> 

Hi,

Maybe I’m blind, but not really. You are still not passing it to the main thread directly.
Instead, you still access _playlistModel unprotected (in the callback thread), and then only later perform something on the main thread. This is too late AFAICS.

Best,
David

>>> 
>>> +#pragma mark - helper methods
>>> +
>>> +- (input_item_t *)createInputItemBasedOnMetadata:(NSDictionary *)itemToCreateDict
>>> +{
>>> +    intf_thread_t *p_intf = getIntf();
>>> +
>>> +    input_item_t *p_input;
>>> +    BOOL b_rem = FALSE, b_dir = FALSE, b_writable = FALSE;
>>> +    NSString *uri, *name, *path;
>>> +    NSURL * url;
>>> +    NSArray *optionsArray;
>>> +
>>> +    /* Get the item */
>>> +    uri = (NSString *)[itemToCreateDict objectForKey: @"ITEM_URL"];
>>> +    url = [NSURL URLWithString: uri];
>>> +    path = [url path];
>>> +    name = (NSString *)[itemToCreateDict objectForKey: @"ITEM_NAME“];
>> 
>> Could we get rid of that weird pattern, please? ;-)
>> If we need to pass more than one object, it should be better wrapped in a proper object, not in those directories with magic keys hardcoded here…
> 
> Absolutely. I just did not want to re-write the open window controller and the playlist at the same so there is a chance to actually follow the changes. This will disappear over the next couple of days.
> 
> 
>>> +    optionsArray = (NSArray *)[itemToCreateDict objectForKey: @"ITEM_OPTIONS"];
>>> +
>>> +    if ([[NSFileManager defaultManager] fileExistsAtPath:path isDirectory:&b_dir]
>>> +        && b_dir &&
>>> +        [[NSWorkspace sharedWorkspace] getFileSystemInfoForPath:path
>>> +                                                    isRemovable:&b_rem
>>> +                                                     isWritable:&b_writable
>>> +                                                  isUnmountable:NULL
>>> +                                                    description:NULL
>>> +                                                           type:NULL]
>>> +        && b_rem && !b_writable && [url isFileURL]) {
>>> +
>>> +        NSString *diskType = getVolumeTypeFromMountPath(path);
>>> +        msg_Dbg(p_intf, "detected optical media of type %s in the file input", [diskType UTF8String]);
>>> +
>>> +        if ([diskType isEqualToString: kVLCMediaDVD])
>>> +            uri = [NSString stringWithFormat: @"dvdnav://%@", getBSDNodeFromMountPath(path)];
>>> +        else if ([diskType isEqualToString: kVLCMediaVideoTSFolder])
>>> +            uri = [NSString stringWithFormat: @"dvdnav://%@", path];
>>> +        else if ([diskType isEqualToString: kVLCMediaAudioCD])
>>> +            uri = [NSString stringWithFormat: @"cdda://%@", getBSDNodeFromMountPath(path)];
>>> +        else if ([diskType isEqualToString: kVLCMediaVCD])
>>> +            uri = [NSString stringWithFormat: @"vcd://%@#0:0", getBSDNodeFromMountPath(path)];
>>> +        else if ([diskType isEqualToString: kVLCMediaSVCD])
>>> +            uri = [NSString stringWithFormat: @"vcd://%@@0:0", getBSDNodeFromMountPath(path)];
>>> +        else if ([diskType isEqualToString: kVLCMediaBD] || [diskType isEqualToString: kVLCMediaBDMVFolder])
>>> +            uri = [NSString stringWithFormat: @"bluray://%@", path];
>>> +        else
>>> +            msg_Warn(getIntf(), "unknown disk type, treating %s as regular input", [path UTF8String]);
>> 
>> this looks a lot like duplicated code.
> 
> No, the originating class will cease to exist.
> 
> Looking forward to further feedback from you!
> 
> Cheers,
> 
> Felix
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel