[vlc-devel] [vlc-commits] macosx: Rework log message storage and fix filtering
Marvin Scholz
epirat07 at gmail.com
Thu Sep 7 23:22:56 CEST 2017
On 7 Sep 2017, at 21:03, David Fuhrmann wrote:
>> Am 07.09.2017 um 03:02 schrieb Marvin Scholz <git at videolan.org>:
>>
>> vlc | branch: master | Marvin Scholz <epirat07 at gmail.com> | Thu Sep
>> 7 03:00:15 2017 +0200| [660257430f41f106ecb1834af7ccaf2a37a281de] |
>> committer: Marvin Scholz
>>
>> macosx: Rework log message storage and fix filtering
>>
>> This is a quite large change to how the log message window data
>> storage
>> works. It completely removes the message buffer that was used.
>> Instead, the messages are added to the backing array of the array
>> controller directly. The array controller will not pick up these
>> changes,
>> so adding a lot of messages at once is not a concern.
>> The timer is now used to sync the array controller with its backing
>> array by calling NSArrayControllers `rearrangeObjects`.
>>
>> The array controller is changed to not clear the predicate on adding
>> new items, which was the initial reason why the other changes became
>> necessary, as the array controller would not allow adding objects
>> that did not match the filter predicate. Adding them to the backing
>> array and syncing with the array controller by calling
>> rearrangeObjects
>> seems to be no problem though.
>>
>> This means filtering is now possible while new messages arrive and
>> while the filter is set, it will only show new messages matching
>> the filter, until the filter is unset, at which point it will show
>> all messages again, as expected.
>>
>>> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=660257430f41f106ecb1834af7ccaf2a37a281de
>> ---
>>
>> modules/gui/macosx/UI/LogMessageWindow.xib | 2 +-
>> modules/gui/macosx/VLCLogWindowController.m | 86
>> ++++++++++++++---------------
>> 2 files changed, 41 insertions(+), 47 deletions(-)
>>
>> diff --git a/modules/gui/macosx/UI/LogMessageWindow.xib
>> b/modules/gui/macosx/UI/LogMessageWindow.xib
>> index 79d1c01763..e8f3b4422c 100644
>> --- a/modules/gui/macosx/UI/LogMessageWindow.xib
>> +++ b/modules/gui/macosx/UI/LogMessageWindow.xib
>> @@ -442,7 +442,7 @@ IA
>> </toolbar>
>> <point key="canvasLocation" x="139" y="192"/>
>> </window>
>> - <arrayController editable="NO" selectsInsertedObjects="NO"
>> id="0Or-bh-QJU">
>> + <arrayController editable="NO" selectsInsertedObjects="NO"
>> clearsFilterPredicateOnInsertion="NO" id="0Or-bh-QJU">
>> <declaredKeys>
>> <string>component</string>
>> <string>message</string>
>> diff --git a/modules/gui/macosx/VLCLogWindowController.m
>> b/modules/gui/macosx/VLCLogWindowController.m
>> index 22d5c54162..a09839031d 100644
>> --- a/modules/gui/macosx/VLCLogWindowController.m
>> +++ b/modules/gui/macosx/VLCLogWindowController.m
>> @@ -32,16 +32,19 @@
>> /* This array stores messages that are managed by the arrayController
>> */
>> @property (retain) NSMutableArray *messagesArray;
>>
>> -/* This array stores messages before they are added to the
>> messagesArray on refresh */
>> - at property (retain) NSMutableArray *messageBuffer;
>> -
>> /* We do not want to refresh the table for every message, as that
>> would be very frequent when
>> - * there are a lot of messages, therefore we use a timer to refresh
>> the table with new data
>> - * from the messageBuffer every now and then, which is much more
>> efficient and still fast
>> - * enough for a good user experience
>> + * there are a lot of messages, therefore we use a timer to refresh
>> the table every now and
>> + * then, which is much more efficient and still fast enough for a
>> good user experience.
>> */
>> @property (retain) NSTimer *refreshTimer;
>>
>> +/*
>> + * Indicates if an update is needed, which is the case when new
>> messages were added
>> + * after the last firing of the update timer. It is used to prevent
>> unnecessary
>> + * rearranging of the NSArrayController content.
>> + */
>> + at property (atomic) BOOL needsUpdate;
>> +
>> - (void)addMessage:(NSDictionary *)message;
>>
>> @end
>> @@ -87,8 +90,7 @@ static void MsgCallback(void *data, int type, const
>> vlc_log_t *item, const char
>> {
>> self = [super initWithWindowNibName:@"LogMessageWindow"];
>> if (self) {
>> - _messagesArray = [[NSMutableArray alloc]
>> initWithCapacity:500];
>> - _messageBuffer = [[NSMutableArray alloc]
>> initWithCapacity:100];
>> + _messagesArray = [[NSMutableArray alloc]
>> initWithCapacity:1000];
>> }
>> return self;
>> }
>> @@ -140,7 +142,7 @@ static void MsgCallback(void *data, int type,
>> const vlc_log_t *item, const char
>> vlc_LogSet(getIntf()->obj.libvlc, MsgCallback, (__bridge
>> void*)self);
>> _refreshTimer = [NSTimer scheduledTimerWithTimeInterval:0.3
>> target:self
>> -
>> selector:@selector(appendMessageBuffer)
>> +
>> selector:@selector(updateArrayController:)
>> userInfo:nil
>> repeats:YES];
>> return [super showWindow:sender];
>> @@ -152,14 +154,24 @@ static void MsgCallback(void *data, int type,
>> const vlc_log_t *item, const char
>> vlc_LogSet( getIntf()->obj.libvlc, NULL, NULL );
>>
>> // Remove all messages
>> - [self clearMessageBuffer];
>> - [self clearMessageTable];
>> + [self removeAllMessages];
>>
>> // Invalidate timer
>> [_refreshTimer invalidate];
>> _refreshTimer = nil;
>> }
>>
>> +/**
>> + Called by the timer to re-sync the array controller with the
>> backing array
>> + */
>> +- (void)updateArrayController:(NSTimer *)timer
>> +{
>> + if (_needsUpdate)
>> + [_arrayController rearrangeObjects];
>> +
>> + _needsUpdate = NO;
>> +}
>> +
>> #pragma mark -
>> #pragma mark Delegate methods
>>
>> @@ -236,8 +248,7 @@ static void MsgCallback(void *data, int type,
>> const vlc_log_t *item, const char
>> vlc_LogSet(getIntf()->obj.libvlc, NULL, NULL);
>>
>> // Remove all messages
>> - [self clearMessageBuffer];
>> - [self clearMessageTable];
>> + [self removeAllMessages];
>>
>> // Reregister handler, to write new header to log
>> vlc_LogSet(getIntf()->obj.libvlc, MsgCallback, (__bridge
>> void*)self);
>> @@ -247,7 +258,7 @@ static void MsgCallback(void *data, int type,
>> const vlc_log_t *item, const char
>> */
>> - (IBAction)refreshLog:(id)sender
>> {
>> - [self appendMessageBuffer];
>> + [_arrayController rearrangeObjects];
>> [_messageTable scrollToEndOfDocument:self];
>> }
>>
>> @@ -264,7 +275,7 @@ static void MsgCallback(void *data, int type,
>> const vlc_log_t *item, const char
>>
>> /* Called when the user hits CMD + C or copy is clicked in the edit
>> menu
>> */
>> -- (void) copy:(id)sender {
>> +- (void)copy:(id)sender {
>> NSPasteboard *pasteBoard = [NSPasteboard generalPasteboard];
>> [pasteBoard clearContents];
>> for (NSDictionary *line in [_arrayController selectedObjects]) {
>> @@ -301,47 +312,30 @@ static void MsgCallback(void *data, int type,
>> const vlc_log_t *item, const char
>> #pragma mark Data handling
>>
>> /**
>> - Adds a message to the messageBuffer, it does not has to be called
>> from the main thread, as
>> - items are only added to the messageArray on refresh.
>> + Adds a message, it is only added visibly to the table on the next
>> firing
>> + of the update timer.
>> */
>> - (void)addMessage:(NSDictionary *)messageDict
>> {
>> - @synchronized (_messageBuffer) {
>> - [_messageBuffer addObject:messageDict];
>> - }
>> -}
>> + @synchronized (_messagesArray) {
>> + if ([_messagesArray count] > 1000000) {
>> + [_messagesArray removeObjectsInRange:NSMakeRange(0, 2)];
>> + }
>> + [_messagesArray addObject:messageDict];
>>
>
> Hi Marvin,
>
> Here the array is updated in a possibly separate thread.
> Where do you make sure that this lock is also held once the array is
> read out by the ArrayController in main thread?
>
Thanks, indeed. Not only that, it has a fundamental flaw.
Reverted.
Will come up with a different solution.
> BR. David
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
More information about the vlc-devel
mailing list