iina · uiryuu · Apr 22, 2023 · Apr 16, 2023 · Apr 16, 2023
diff --git a/iina/MPVController.swift b/iina/MPVController.swift
@@ -862,7 +862,7 @@ class MPVController: NSObject {
     case MPV_EVENT_AUDIO_RECONFIG: break
 
     case MPV_EVENT_VIDEO_RECONFIG:
-      onVideoReconfig()
+      player.onVideoReconfig()
 
     case MPV_EVENT_START_FILE:
       player.info.isIdle = false
@@ -990,28 +990,6 @@ class MPVController: NSObject {
     player.syncUI(.playlist)
   }
 
-  private func onVideoReconfig() {
-    // If loading file, video reconfig can return 0 width and height
-    if player.info.fileLoading {
-      return
-    }
-    var dwidth = getInt(MPVProperty.dwidth)
-    var dheight = getInt(MPVProperty.dheight)
-    if player.info.rotation == 90 || player.info.rotation == 270 {
-      swap(&dwidth, &dheight)
-    }
-    if dwidth != player.info.displayWidth! || dheight != player.info.displayHeight! {
-      // filter the last video-reconfig event before quit
-      if dwidth == 0 && dheight == 0 && getFlag(MPVProperty.coreIdle) { return }
-      // video size changed
-      player.info.displayWidth = dwidth
-      player.info.displayHeight = dheight
-      DispatchQueue.main.sync {
-        player.notifyMainWindowVideoSizeChanged()
-      }
-    }
-  }
-
   // MARK: - Property listeners
 
   private func handlePropertyChange(_ name: String, _ property: mpv_event_property) {

diff --git a/iina/PlayerCore.swift b/iina/PlayerCore.swift
@@ -496,11 +496,6 @@ class PlayerCore: NSObject {
     isShuttingDown = true
     Logger.log("Shutting down", subsystem: subsystem)
     savePlayerState()
-    // Once mpv has been instructed to quit accessing the mpv core can result in a crash. Must not
-    // allow the display link thread or the mpvGLQueue dispatch queue to continue processing because
-    // these entities will call mpv.
-    mainWindow.videoView.videoLayer.suspend()
-    mainWindow.videoView.stopDisplayLink()
     mpv.mpvQuit()
   }
 
@@ -1582,6 +1577,26 @@ class PlayerCore: NSObject {
     }
   }
 
+  func onVideoReconfig() {
+    // If loading file, video reconfig can return 0 width and height
+    guard !info.fileLoading, !isShuttingDown, !isShutdown else { return }
+    var dwidth = mpv.getInt(MPVProperty.dwidth)
+    var dheight = mpv.getInt(MPVProperty.dheight)
+    if info.rotation == 90 || info.rotation == 270 {
+      swap(&dwidth, &dheight)
+    }
+    if dwidth != info.displayWidth! || dheight != info.displayHeight! {
+      // filter the last video-reconfig event before quit
+      if dwidth == 0 && dheight == 0 && mpv.getFlag(MPVProperty.coreIdle) { return }
+      // video size changed
+      info.displayWidth = dwidth
+      info.displayHeight = dheight
+      DispatchQueue.main.sync {
+        notifyMainWindowVideoSizeChanged()
+      }
+    }
+  }
+
   @available(macOS 10.15, *)
   func refreshEdrMode() {
     guard mainWindow.loaded else { return }

diff --git a/iina/VideoView.swift b/iina/VideoView.swift
@@ -22,7 +22,7 @@ class VideoView: NSView {
 
   var videoSize: NSSize?
 
-  var isUninited = false
+  @Atomic var isUninited = false
 
   var draggingTimer: Timer?
 
@@ -82,14 +82,19 @@ class VideoView: NSView {
     fatalError("init(coder:) has not been implemented")
   }
 
+  /// Uninitialize this view.
+  ///
+  /// This method will stop drawing and free the mpv render context. This is done before sending a quit command to mpv.
+  /// - Important: Once mpv has been instructed to quit accessing the mpv core can result in a crash, therefore locks must be
+  ///     used to coordinate uninitializing the view so that other threads do not attempt to use the mpv core while it is shutting down.
   func uninit() {
-    player.mpv.lockAndSetOpenGLContext()
-    defer { player.mpv.unlockOpenGLContext() }
-
-    guard !isUninited else { return }
+    $isUninited.withLock() { isUninited in
+      guard !isUninited else { return }
+      isUninited = true
 
-    player.mpv.mpvUninitRendering()
-    isUninited = true
+      videoLayer.suspend()
+      player.mpv.mpvUninitRendering()
+    }
   }
 
   deinit {
@@ -200,7 +205,7 @@ class VideoView: NSView {
     }
     guard !CVDisplayLinkIsRunning(link) else { return }
     updateDisplayLink()
-    CVDisplayLinkSetOutputCallback(link, displayLinkCallback, mutableRawPointerOf(obj: player.mpv))
+    CVDisplayLinkSetOutputCallback(link, displayLinkCallback, mutableRawPointerOf(obj: self))
     CVDisplayLinkStart(link)
   }
 
@@ -405,7 +410,10 @@ fileprivate func displayLinkCallback(
   _ flagsIn: CVOptionFlags,
   _ flagsOut: UnsafeMutablePointer<CVOptionFlags>,
   _ context: UnsafeMutableRawPointer?) -> CVReturn {
-  let mpv = unsafeBitCast(context, to: MPVController.self)
-  mpv.mpvReportSwap()
+  let videoView = unsafeBitCast(context, to: VideoView.self)
+  videoView.$isUninited.withLock() { isUninited in
+    guard !isUninited else { return }
+    videoView.player.mpv.mpvReportSwap()
+  }
   return kCVReturnSuccess
 }
diff --git a/iina/ViewLayer.swift b/iina/ViewLayer.swift
@@ -19,8 +19,8 @@ class ViewLayer: CAOpenGLLayer {
 
   private var fbo: GLint = 1
 
-  @Atomic private var needsMPVRender = false
-  @Atomic private var forceRender = false
+  private var needsMPVRender = false
+  private var forceRender = false
 
   override init() {
     super.init()
@@ -97,8 +97,6 @@ class ViewLayer: CAOpenGLLayer {
     let mpv = videoView.player.mpv!
     needsMPVRender = false
 
-    guard !videoView.isUninited else { return }
-
     glClear(GLbitfield(GL_COLOR_BUFFER_BIT))
 
     var i: GLint = 0
@@ -145,18 +143,30 @@ class ViewLayer: CAOpenGLLayer {
   }
 
   func draw(forced: Bool = false) {
-    needsMPVRender = true
-    if forced { forceRender = true }
-    display()
-    if forced {
-      forceRender = false
-      return
+    videoView.$isUninited.withLock() { isUninited in
+      // The properties forceRender and needsMPVRender are always accessed while holding isUninited's
+      // lock. This avoids the need for separate locks to avoid data races with these flags. No need
+      // to check isUninited at this point.
+      needsMPVRender = true
+      if forced { forceRender = true }
     }
-    if needsMPVRender {
+
+    // Must not call display while holding isUninited's lock as that method will attempt to acquire
+    // the lock and our locks do not support recursion.
+    display()
+
+    videoView.$isUninited.withLock() { isUninited in
+      guard !isUninited else { return }
+      if forced {
+        forceRender = false
+        return
+      }
+      guard needsMPVRender else { return }
+
       videoView.player.mpv.lockAndSetOpenGLContext()
       defer { videoView.player.mpv.unlockOpenGLContext() }
       // draw(inCGLContext:) is not called, needs a skip render
-      if !videoView.isUninited, let renderContext = videoView.player.mpv.mpvRenderContext {
+      if let renderContext = videoView.player.mpv.mpvRenderContext {
         var skip: CInt = 1
         withUnsafeMutablePointer(to: &skip) { skip in
           var params: [mpv_render_param] = [
@@ -171,7 +181,16 @@ class ViewLayer: CAOpenGLLayer {
   }
 
   override func display() {
-    super.display()
+    let needsFlush: Bool = videoView.$isUninited.withLock() { isUninited in
+      guard !isUninited else { return false }
+
+      super.display()
+      return true
+    }
+    guard needsFlush else { return }
+
+    // Must not call flush while holding isUninited's lock as that method may call display and our
+    // locks do not support recursion.
     CATransaction.flush()
   }
 
@@ -213,5 +232,4 @@ class ViewLayer: CAOpenGLLayer {
   func ignoreGLError() {
     glGetError()
   }
-
 }