[vlc-commits] [Git][videolan/vlc][master] 2 commits: VLCVideoUIView: use dedicated queue for event

[Jean-Baptiste Kempf (@jbk)]

Jean-Baptiste Kempf pushed to branch master at VideoLAN / VLC


Commits:
d9de65d1 by Alexandre Janniaux at 2021-07-11T08:53:12+00:00
VLCVideoUIView: use dedicated queue for event

Switch the event queue used to a dedicated event queue which will create
its own thread, instead of using QOS parallel queues, which prevents
mixing with non-VLC tasks and ensure tasks are ordered.

- - - - -
786e351a by Alexandre Janniaux at 2021-07-11T08:53:12+00:00
VLCVideoUIView: fix event reporting

The event reporting must happen one event at a time and never trigger
behaviour not expected by the underlying application. It uses a custom
runloop mode to ensure only the VLC aware code can use the runloop (and
thus usually main thread) when reporting an event. It was ensured by
calling CFRunLoopRunInMode until the runloop is stopped.

But there were multiple undetected flaws in this:
 - CFRunLoopRunInMode will terminate (with timeout or finish) whenever
   one event have been processed, so it needs to be called in loop with
   the correct invariant.
 - CFRunLoopRunInMode with timeout=0 will exit even sooner if there are
   not events available.
 - CFRunLoopStop will interrupt CFRunLoopInMode only if it's actually
   running so there's an asynchronous design flaw where we try to stop
   the event loop from another thread without syncing with this event
   loop.

This patch fixes those issues, though it adds some callback nightmares,
by adding a new level of dispatch to the main CFRunLoop, which will be
the one to call CFRunLoopStop on itself. Thus, the CFRunLoop must be in
a running state when CFRunLoopStop is called.

It also ensure that we don't leave the loop for another reason than
calling CFRunLoopStop, eg. Timeout or Finish result code.

- - - - -


1 changed file:

- modules/video_output/apple/VLCVideoUIView.m


Changes:

=====================================
modules/video_output/apple/VLCVideoUIView.m
=====================================
@@ -82,6 +82,8 @@
     /* Window state */
     BOOL _enabled;
     int _subviews;
+
+    dispatch_queue_t _eventq;
 }
 
 - (id)initWithWindow:(vout_window_t *)wnd;
@@ -107,6 +109,7 @@
     if (!self)
         return nil;
 
+    _eventq = dispatch_queue_create("vlc_eventq", DISPATCH_QUEUE_SERIAL);
     vlc_mutex_init(&_mutex);
 
     /* The window is controlled by the host application through the UIView
@@ -170,14 +173,46 @@
 {
     CFStringRef mode = CFSTR("org.videolan.vlccore.window");
     CFRunLoopRef runloop = CFRunLoopGetCurrent();
+
+    /* Callback hell right below, we need to execute the call
+     * to CFRunLoopStop inside the CFRunLoopRunInMode context
+     * since the CFRunLoopRunInMode might have already returned
+     * otherwise, which means more callback wrapping. */
     CFRunLoopPerformBlock(runloop, mode, ^{
-        dispatch_async(dispatch_get_global_queue(QOS_CLASS_UTILITY, 0), ^{
+        /* Execute the event in a different thread, we don't
+         * want to block the main CFRunLoop since the vout
+         * display module typically needs it to Open(). */
+        dispatch_async(_eventq, ^{
             (eventBlock)();
-            CFRunLoopStop(runloop);
+            CFRunLoopPerformBlock(runloop, mode, ^{
+                /* Signal that we can end the ReportEvent call */
+                CFRunLoopStop(runloop);
+            });
+            CFRunLoopWakeUp(runloop);
         });
     });
+    /* Above and here, the CFRunLoopWakeUp call is necessary to
+     * signal to the event loop that it will need to process the
+     * blocks. They don't act like CFRunLoopSource so they won't
+     * wake up the loop otherwise. */
     CFRunLoopWakeUp(runloop);
-    CFRunLoopRunInMode(mode, 0, NO);
+    for (;;)
+    {
+        /* We need a timeout here, otherwise the CFRunLoopInMode
+         * call will check the events (if woken up), and since
+         * we might have no event, it would return a timeout
+         * result code, and loop again, creating a busy loop.
+         * INFINITY is more than enough, and we'll interrupt
+         * anyway. */
+        CFRunLoopRunResult ret = CFRunLoopRunInMode(mode, INFINITY, YES);
+
+        /* Usual CFRunLoop are typically checking result code
+         * like kCFRunLoopRunFinished too, but we really want
+         * to receive the Stop signal from above to leave the
+         * loop in the correct state. */
+        if (ret == kCFRunLoopRunStopped)
+            break;
+    }
 }
 
 - (void)detachFromParent



View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/ac4cd934035e400b9aa468df74158e45fb9cce10...786e351ab9aa45431881a189caf673ae454d74e6

-- 
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/ac4cd934035e400b9aa468df74158e45fb9cce10...786e351ab9aa45431881a189caf673ae454d74e6
You're receiving this email because of your account on code.videolan.org.