[vlc-devel] [vlc-commits] macosx/playlist: ensure that changes to the model are dispatched to the main thread
David Fuhrmann
david.fuhrmann at gmail.com
Fri Feb 1 17:17:32 CET 2019
Hi Felix,
Thanks for the update. Some small questions below.
> Am 01.02.2019 um 12:07 schrieb Felix Paul Kühne <git at videolan.org>:
>
> vlc | branch: master | Felix Paul Kühne <felix at feepk.net> | Fri Feb 1 12:07:07 2019 +0100| [9b5902e281494bc89b6f0c3d5d2141dcaaad58e4] | committer: Felix Paul Kühne
>
> macosx/playlist: ensure that changes to the model are dispatched to the main thread
>
>> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=9b5902e281494bc89b6f0c3d5d2141dcaaad58e4
> ---
>
> modules/gui/macosx/UI/VLCPlaylistTableCellView.xib | 3 +
> modules/gui/macosx/VLCPlaylistController.m | 86 +++++++++++++---------
> modules/gui/macosx/VLCPlaylistItem.m | 6 ++
> modules/gui/macosx/VLCPlaylistModel.h | 4 +-
> modules/gui/macosx/VLCPlaylistModel.m | 10 +--
> 5 files changed, 68 insertions(+), 41 deletions(-)
>
> diff --git a/modules/gui/macosx/UI/VLCPlaylistTableCellView.xib b/modules/gui/macosx/UI/VLCPlaylistTableCellView.xib
> index eab65dcab2..c68cf25a5b 100644
> --- a/modules/gui/macosx/UI/VLCPlaylistTableCellView.xib
> +++ b/modules/gui/macosx/UI/VLCPlaylistTableCellView.xib
> @@ -29,6 +29,9 @@
> </imageView>
> <textField horizontalHuggingPriority="251" verticalHuggingPriority="750" translatesAutoresizingMaskIntoConstraints="NO" id="3Ha-ZH-fa9">
> <rect key="frame" x="353" y="24" width="37" height="17"/>
> + <constraints>
> + <constraint firstAttribute="width" constant="33" id="0cB-ga-0Uw"/>
> + </constraints>
> <textFieldCell key="cell" lineBreakMode="clipping" title="Label" id="N0j-xB-3t9">
> <font key="font" usesAppearanceFont="YES"/>
> <color key="textColor" name="labelColor" catalog="System" colorSpace="catalog"/>
> diff --git a/modules/gui/macosx/VLCPlaylistController.m b/modules/gui/macosx/VLCPlaylistController.m
> index 47f00d3c00..525cc09bfd 100644
> --- a/modules/gui/macosx/VLCPlaylistController.m
> +++ b/modules/gui/macosx/VLCPlaylistController.m
> @@ -22,6 +22,7 @@
>
> #import "VLCPlaylistController.h"
> #import "VLCPlaylistModel.h"
> +#import "VLCPlaylistItem.h"
> #import "VLCPlaylistDataSource.h"
> #import "VLCMain.h"
> #import <vlc_interface.h>
> @@ -37,8 +38,8 @@ NSString *VLCPlaybackHasNextChanged = @"VLCPlaybackHasNextChanged";
> vlc_playlist_listener_id *_playlistListenerID;
> }
>
> -- (void)playlistResetWithItems:(vlc_playlist_item_t *const *)items count:(size_t)numberOfItems;
> -- (void)playlistAdded:(vlc_playlist_item_t *const *)items atIndex:(size_t)insertionIndex count:(size_t)numberOfItems;
> +- (void)playlistResetWithItems:(NSArray *)items count:(size_t)numberOfItems;
> +- (void)playlistAdded:(NSArray *)items atIndex:(size_t)insertionIndex count:(size_t)numberOfItems;
> - (void)playlistRemovedItemsAtIndex:(size_t)index count:(size_t)numberOfItems;
> - (void)playlistUpdatedForIndex:(size_t)firstUpdatedIndex items:(vlc_playlist_item_t *const *)items count:(size_t)numberOfItems;
> - (void)playlistPlaybackRepeatUpdated:(enum vlc_playlist_playback_repeat)currentRepeatMode;
> @@ -58,8 +59,15 @@ cb_playlist_items_reset(vlc_playlist_t *playlist,
> size_t numberOfItems,
> void *p_data)
> {
> - VLCPlaylistController *playlistController = (__bridge VLCPlaylistController *)p_data;
> - [playlistController playlistResetWithItems:items count:numberOfItems];
> + NSMutableArray *array = [NSMutableArray arrayWithCapacity:numberOfItems];
> + for (size_t i = 0; i < numberOfItems; i++) {
> + VLCPlaylistItem *item = [[VLCPlaylistItem alloc] initWithPlaylistItem:items[i]];
> + [array addObject:item];
> + }
> + dispatch_async(dispatch_get_main_queue(), ^{
> + VLCPlaylistController *playlistController = (__bridge VLCPlaylistController *)p_data;
> + [playlistController playlistResetWithItems:items count:numberOfItems];
> + });
> }
Seems good, but why creating the array if you do not use it? :-)
We can also get rid of the count: part.
>
> static void
> @@ -69,8 +77,15 @@ cb_playlist_items_added(vlc_playlist_t *playlist,
> size_t numberOfAddedItems,
> void *p_data)
> {
> - VLCPlaylistController *playlistController = (__bridge VLCPlaylistController *)p_data;
> - [playlistController playlistAdded:items atIndex:insertionIndex count:numberOfAddedItems];
> + NSMutableArray *array = [NSMutableArray arrayWithCapacity:numberOfAddedItems];
> + for (size_t i = 0; i < numberOfAddedItems; i++) {
> + VLCPlaylistItem *item = [[VLCPlaylistItem alloc] initWithPlaylistItem:items[i]];
> + [array addObject:item];
> + }
> + dispatch_async(dispatch_get_main_queue(), ^{
> + VLCPlaylistController *playlistController = (__bridge VLCPlaylistController *)p_data;
> + [playlistController playlistAdded:array atIndex:insertionIndex count:numberOfAddedItems];
Lets get rid of the count part, no? The array already knows how big it is. Luckily we do not need to deal with plain old C array here anymore...
> + });
> }
>
> static void
> @@ -79,8 +94,10 @@ cb_playlist_items_removed(vlc_playlist_t *playlist,
> size_t count,
> void *p_data)
> {
> - VLCPlaylistController *playlistController = (__bridge VLCPlaylistController *)p_data;
> - [playlistController playlistRemovedItemsAtIndex:index count:count];
> + dispatch_async(dispatch_get_main_queue(), ^{
> + VLCPlaylistController *playlistController = (__bridge VLCPlaylistController *)p_data;
> + [playlistController playlistRemovedItemsAtIndex:index count:count];
> + });
> }
>
> static void
> @@ -90,8 +107,10 @@ cb_playlist_items_updated(vlc_playlist_t *playlist,
> size_t numberOfUpdatedItems,
> void *p_data)
> {
> - VLCPlaylistController *playlistController = (__bridge VLCPlaylistController *)p_data;
> - [playlistController playlistUpdatedForIndex:firstUpdatedIndex items:items count:numberOfUpdatedItems];
> + dispatch_async(dispatch_get_main_queue(), ^{
> + VLCPlaylistController *playlistController = (__bridge VLCPlaylistController *)p_data;
> + [playlistController playlistUpdatedForIndex:firstUpdatedIndex items:items count:numberOfUpdatedItems];
> + });
> }
>
> static void
> @@ -115,8 +134,10 @@ cb_playlist_current_item_changed(vlc_playlist_t *playlist,
> ssize_t index,
> void *p_data)
> {
> - VLCPlaylistController *playlistController = (__bridge VLCPlaylistController *)p_data;
> - [playlistController currentPlaylistItemChanged:index];
> + dispatch_async(dispatch_get_main_queue(), ^{
> + VLCPlaylistController *playlistController = (__bridge VLCPlaylistController *)p_data;
> + [playlistController currentPlaylistItemChanged:index];
> + });
> }
>
> static void
> @@ -124,8 +145,10 @@ cb_playlist_has_prev_changed(vlc_playlist_t *playlist,
> bool has_prev,
> void *p_data)
> {
> - VLCPlaylistController *playlistController = (__bridge VLCPlaylistController *)p_data;
> - [playlistController playlistHasPreviousItem:has_prev];
> + dispatch_async(dispatch_get_main_queue(), ^{
> + VLCPlaylistController *playlistController = (__bridge VLCPlaylistController *)p_data;
> + [playlistController playlistHasPreviousItem:has_prev];
> + });
> }
>
> static void
> @@ -133,8 +156,10 @@ cb_playlist_has_next_changed(vlc_playlist_t *playlist,
> bool has_next,
> void *p_data)
> {
> - VLCPlaylistController *playlistController = (__bridge VLCPlaylistController *)p_data;
> - [playlistController playlistHasNextItem:has_next];
> + dispatch_async(dispatch_get_main_queue(), ^{
> + VLCPlaylistController *playlistController = (__bridge VLCPlaylistController *)p_data;
> + [playlistController playlistHasNextItem:has_next];
> + });
> }
>
> static const struct vlc_playlist_callbacks playlist_callbacks = {
> @@ -183,23 +208,18 @@ static const struct vlc_playlist_callbacks playlist_callbacks = {
>
> #pragma mark - callback forwarders
>
> -- (void)playlistResetWithItems:(vlc_playlist_item_t *const *)items count:(size_t)numberOfItems
> +- (void)playlistResetWithItems:(NSArray *)items count:(size_t)numberOfItems
> {
> - for (size_t i = 0; i < numberOfItems; i++) {
> - [_playlistModel addItem:items[i]];
> - }
> + [_playlistModel addItems:items];
>
> - [_playlistDataSource performSelectorOnMainThread:@selector(playlistUpdated) withObject:nil waitUntilDone:NO];
> + [_playlistDataSource playlistUpdated];
> }
>
> -- (void)playlistAdded:(vlc_playlist_item_t *const *)items atIndex:(size_t)insertionIndex count:(size_t)numberOfItems
> +- (void)playlistAdded:(NSArray *)items atIndex:(size_t)insertionIndex count:(size_t)numberOfItems
> {
> - for (size_t i = 0; i < numberOfItems; i++) {
> - [_playlistModel addItem:items[i] atIndex:insertionIndex];
> - insertionIndex++;
> - }
> + [_playlistModel addItems:items atIndex:insertionIndex];
>
> - [_playlistDataSource performSelectorOnMainThread:@selector(playlistUpdated) withObject:nil waitUntilDone:NO];
> + [_playlistDataSource playlistUpdated];
> }
>
> - (void)playlistRemovedItemsAtIndex:(size_t)index count:(size_t)numberOfItems
> @@ -207,7 +227,7 @@ static const struct vlc_playlist_callbacks playlist_callbacks = {
> NSRange range = NSMakeRange(index, numberOfItems);
> [_playlistModel removeItemsInRange:range];
>
> - [_playlistDataSource performSelectorOnMainThread:@selector(playlistUpdated) withObject:nil waitUntilDone:NO];
> + [_playlistDataSource playlistUpdated];
> }
>
> - (void)playlistUpdatedForIndex:(size_t)firstUpdatedIndex items:(vlc_playlist_item_t *const *)items count:(size_t)numberOfItems
> @@ -216,7 +236,7 @@ static const struct vlc_playlist_callbacks playlist_callbacks = {
> for (size_t i = firstUpdatedIndex; i < firstUpdatedIndex + numberOfItems; i++) {
> [_playlistModel updateItemAtIndex:i];
> }
> - [_playlistDataSource performSelectorOnMainThread:@selector(playlistUpdated) withObject:nil waitUntilDone:NO];
> + [_playlistDataSource playlistUpdated];
> }
>
> - (void)playlistPlaybackRepeatUpdated:(enum vlc_playlist_playback_repeat)currentRepeatMode
> @@ -234,7 +254,7 @@ static const struct vlc_playlist_callbacks playlist_callbacks = {
> - (void)currentPlaylistItemChanged:(size_t)index
> {
> _currentPlaylistIndex = index;
> - [_playlistDataSource performSelectorOnMainThread:@selector(playlistUpdated) withObject:nil waitUntilDone:NO];
> + [_playlistDataSource playlistUpdated];
> }
>
> - (void)playlistHasPreviousItem:(BOOL)hasPrevious
> @@ -289,9 +309,9 @@ static const struct vlc_playlist_callbacks playlist_callbacks = {
> actualInsertionIndex = vlc_playlist_Count(_p_playlist);
> }
> ret = vlc_playlist_Insert(_p_playlist,
> - actualInsertionIndex,
> - &p_input,
> - 1);
> + actualInsertionIndex,
> + &p_input,
> + 1);
>
> if (ret != VLC_SUCCESS) {
> msg_Err(p_intf, "failed to insert input item at insertion index: %zu", insertionIndex);
> diff --git a/modules/gui/macosx/VLCPlaylistItem.m b/modules/gui/macosx/VLCPlaylistItem.m
> index c881709bef..8d5b3db5fe 100644
> --- a/modules/gui/macosx/VLCPlaylistItem.m
> +++ b/modules/gui/macosx/VLCPlaylistItem.m
> @@ -32,11 +32,17 @@
> self = [super init];
> if (self) {
> _playlistItem = p_item;
> + vlc_playlist_item_Hold(_playlistItem);
> [self updateRepresentation];
> }
> return self;
> }
>
> +- (void)dealloc
> +{
> + vlc_playlist_item_Release(_playlistItem);
> +}
> +
> - (NSString *)description
> {
> return [NSString stringWithFormat:@"item %p, title: %@ duration %lli", &_playlistItem, _title, _duration];
> diff --git a/modules/gui/macosx/VLCPlaylistModel.h b/modules/gui/macosx/VLCPlaylistModel.h
> index 601c5af44d..c6ce2c22dd 100644
> --- a/modules/gui/macosx/VLCPlaylistModel.h
> +++ b/modules/gui/macosx/VLCPlaylistModel.h
> @@ -35,8 +35,8 @@ NS_ASSUME_NONNULL_BEGIN
>
> - (void)dropExistingData;
> - (VLCPlaylistItem *)playlistItemAtIndex:(NSInteger)index;
> -- (void)addItem:(vlc_playlist_item_t *)item;
> -- (void)addItem:(vlc_playlist_item_t *)item atIndex:(size_t)index;
> +- (void)addItems:(NSArray *)array;
> +- (void)addItems:(NSArray *)array atIndex:(size_t)index;
> - (void)removeItemsInRange:(NSRange)range;
> - (void)updateItemAtIndex:(size_t)index;
>
> diff --git a/modules/gui/macosx/VLCPlaylistModel.m b/modules/gui/macosx/VLCPlaylistModel.m
> index 5b9246d869..26dce6403d 100644
> --- a/modules/gui/macosx/VLCPlaylistModel.m
> +++ b/modules/gui/macosx/VLCPlaylistModel.m
> @@ -63,16 +63,14 @@
> return _playlistArray[index];
> }
>
> -- (void)addItem:(vlc_playlist_item_t *)item
> +- (void)addItems:(NSArray *)array
> {
> - VLCPlaylistItem *playlistItem = [[VLCPlaylistItem alloc] initWithPlaylistItem:item];
> - [_playlistArray addObject:playlistItem];
> + [_playlistArray addObjectsFromArray:array];
> }
>
> -- (void)addItem:(vlc_playlist_item_t *)item atIndex:(size_t)index
> +- (void)addItems:(NSArray *)array atIndex:(size_t)index
> {
> - VLCPlaylistItem *playlistItem = [[VLCPlaylistItem alloc] initWithPlaylistItem:item];
> - [_playlistArray insertObject:playlistItem atIndex:index];
> + [_playlistArray insertObjects:array atIndexes:[NSIndexSet indexSetWithIndex:index]];
> }
>
> - (void)removeItemsInRange:(NSRange)range
>
> _______________________________________________
> vlc-commits mailing list
> vlc-commits at videolan.org
> https://mailman.videolan.org/listinfo/vlc-commits
More information about the vlc-devel
mailing list