From 5c8c55f41c86c1f41d3cb0ab73985cd44145b690 Mon Sep 17 00:00:00 2001 From: Arkadiusz Fal Date: Wed, 3 Jun 2026 22:00:28 +0200 Subject: [PATCH] Fix macOS black video after player close/reopen The macOS CAOpenGLLayer is pull-based: after the shared render view is re-parented into a reopened player window, a single forced repaint is issued. draw() consumed the forceDraw flag before validating the viewport, so when that repaint landed while the layer was still 0x0 (the expanded window builds its content a runloop late), the request was swallowed. With mpv paused during load, nothing repainted again, leaving the video permanently black with working audio. - Validate viewport/framebuffer in draw() before consuming flags; on failure keep the forced draw armed and retry (bounded, 5x50ms) - Repaint from setFrameSize on the first non-zero layout while a forced draw is still pending - Drop the stale fbo=1 fallback; render only into the validated live framebuffer binding - clearToBlack: take the CGL lock and skip glClear when no framebuffer is bound - clearing FBO 0 on this core-profile context latched GL_INVALID_FRAMEBUFFER_OPERATION, which libmpv reported on every load - Add diagnostics for dropped draws and the first-frame render --- Yattee/Services/Player/MPV/MPVOGLView.swift | 13 ++ .../Services/Player/MPV/MPVOpenGLLayer.swift | 112 ++++++++++++++---- 2 files changed, 104 insertions(+), 21 deletions(-) diff --git a/Yattee/Services/Player/MPV/MPVOGLView.swift b/Yattee/Services/Player/MPV/MPVOGLView.swift index 642e2be4..a6e605b0 100644 --- a/Yattee/Services/Player/MPV/MPVOGLView.swift +++ b/Yattee/Services/Player/MPV/MPVOGLView.swift @@ -191,6 +191,19 @@ final class MPVOGLView: NSView { updateDisplayRefreshRate() } + override func setFrameSize(_ newSize: NSSize) { + super.setFrameSize(newSize) + + // If a forced draw was requested while the layer had no size/drawable + // (shared view re-parented into the deferred expanded sheet), repaint + // on the first non-zero layout. Gated on hasPendingForcedDraw so + // ordinary resizes don't trigger redundant redraws. + guard newSize.width > 0, newSize.height > 0, + let videoLayer, videoLayer.hasPendingForcedDraw else { return } + LoggingService.shared.debug("MPVOGLView: retrying pending forced draw after resize to \(newSize)", category: .mpv) + videoLayer.update(force: true) + } + override func draw(_ dirtyRect: NSRect) { // No-op - the layer handles all drawing } diff --git a/Yattee/Services/Player/MPV/MPVOpenGLLayer.swift b/Yattee/Services/Player/MPV/MPVOpenGLLayer.swift index b383c466..36b43542 100644 --- a/Yattee/Services/Player/MPV/MPVOpenGLLayer.swift +++ b/Yattee/Services/Player/MPV/MPVOpenGLLayer.swift @@ -77,9 +77,6 @@ final class MPVOpenGLLayer: CAOpenGLLayer { /// Buffer depth (8 for standard, 16 for 10-bit). private var bufferDepth: GLint = 8 - /// Current framebuffer object ID. - private var fbo: GLint = 1 - /// When `true` the frame needs to be rendered. private var needsFlip = false private let needsFlipLock = NSLock() @@ -88,6 +85,15 @@ final class MPVOpenGLLayer: CAOpenGLLayer { private var forceDraw = false private let forceDrawLock = NSLock() + /// Retry bookkeeping for draws dropped due to a zero-sized viewport or + /// missing framebuffer (e.g. the shared view re-parented into a window + /// whose content hasn't been laid out yet). + private var droppedDrawRetries = 0 + private let droppedDrawRetryLock = NSLock() + + /// True while a forced draw has been requested but not yet executed. + var hasPendingForcedDraw: Bool { forceDrawLock.withLock { forceDraw } } + /// Whether the layer has been set up with an MPV client. private var isSetup = false @@ -317,14 +323,10 @@ final class MPVOpenGLLayer: CAOpenGLLayer { ) { guard !isUninited, isSetup, let mpvClient else { return } - // Reset flags - needsFlipLock.withLock { needsFlip = false } - forceDrawLock.withLock { forceDraw = false } - - // Clear the buffer - glClear(GLbitfield(GL_COLOR_BUFFER_BIT)) - - // Get current FBO binding and viewport dimensions + // Validate the render target FIRST — before consuming flags or + // touching GL state. A forced draw that lands while the re-attached + // layer has no size/drawable must stay armed so a retry can repaint; + // consuming it here used to leave the video permanently black. var currentFBO: GLint = 0 glGetIntegerv(GLenum(GL_DRAW_FRAMEBUFFER_BINDING), ¤tFBO) @@ -334,16 +336,30 @@ final class MPVOpenGLLayer: CAOpenGLLayer { let width = viewport[2] let height = viewport[3] - guard width > 0, height > 0 else { return } - - // Use the detected FBO (or fallback to cached) - if currentFBO != 0 { - fbo = currentFBO + guard width > 0, height > 0, currentFBO != 0 else { + let boundsSize = bounds.size + let droppedFBO = currentFBO + Task { @MainActor in + LoggingService.shared.warning( + "MPVOpenGLLayer: dropped draw - viewport \(width)x\(height), fbo=\(droppedFBO), bounds=\(boundsSize), re-arming forced draw", + category: .mpv + ) + } + scheduleDroppedDrawRetry() + return } + // Reset flags + needsFlipLock.withLock { needsFlip = false } + forceDrawLock.withLock { forceDraw = false } + droppedDrawRetryLock.withLock { droppedDrawRetries = 0 } + + // Clear the buffer + glClear(GLbitfield(GL_COLOR_BUFFER_BIT)) + // Render the frame mpvClient.renderWithDepth( - fbo: fbo, + fbo: currentFBO, width: width, height: height, depth: bufferDepth @@ -351,9 +367,20 @@ final class MPVOpenGLLayer: CAOpenGLLayer { glFlush() + if !hasRenderedFirstFrame { + let glError = glGetError() + let renderedFBO = currentFBO + Task { @MainActor in + LoggingService.shared.debug( + "MPVOpenGLLayer: first-frame draw fbo=\(renderedFBO) viewport=\(width)x\(height) glError=\(glError)", + category: .mpv + ) + } + } + // Capture frame for PiP if enabled if captureFramesForPiP { - captureFrameForPiP(viewWidth: width, viewHeight: height, mainFBO: fbo) + captureFrameForPiP(viewWidth: width, viewHeight: height, mainFBO: currentFBO) } // Mark that we've rendered a frame (for first-frame tracking) @@ -449,6 +476,30 @@ final class MPVOpenGLLayer: CAOpenGLLayer { } } + /// Schedule a bounded retry after a draw was dropped because the layer + /// had no valid viewport/framebuffer yet. Keeps retrying only while the + /// forced-draw request is still pending; `setFrameSize` on the hosting + /// view covers the case where the layer gains its size later than this. + private func scheduleDroppedDrawRetry() { + let attempt = droppedDrawRetryLock.withLock { () -> Int in + droppedDrawRetries += 1 + return droppedDrawRetries + } + guard attempt <= 5 else { return } + + renderQueue.asyncAfter(deadline: .now() + .milliseconds(50)) { [weak self] in + guard let self, !self.isUninited else { return } + guard self.hasPendingForcedDraw else { return } + Task { @MainActor in + LoggingService.shared.debug( + "MPVOpenGLLayer: retrying dropped forced draw (attempt \(attempt))", + category: .mpv + ) + } + self.display() + } + } + /// Render a frame specifically for PiP capture (when main view is hidden). private func renderForPiP() { guard !isUninited, isSetup, let mpvClient else { return } @@ -563,10 +614,29 @@ final class MPVOpenGLLayer: CAOpenGLLayer { renderQueue.async { [weak self] in guard let self else { return } + CGLLockContext(self.cglContext) CGLSetCurrentContext(self.cglContext) - glClearColor(0.0, 0.0, 0.0, 1.0) - glClear(GLbitfield(GL_COLOR_BUFFER_BIT)) - glFlush() + + // Only clear when a framebuffer is actually bound. Clearing FBO 0 + // on this core-profile context (no drawable attached outside of + // CAOpenGLLayer.draw) is framebuffer-incomplete and latches + // GL_INVALID_FRAMEBUFFER_OPERATION, which libmpv then reports as + // "after creating texture: OpenGL error INVALID_FRAMEBUFFER_OPERATION". + var boundFBO: GLint = 0 + glGetIntegerv(GLenum(GL_DRAW_FRAMEBUFFER_BINDING), &boundFBO) + if boundFBO != 0 { + glClearColor(0.0, 0.0, 0.0, 1.0) + glClear(GLbitfield(GL_COLOR_BUFFER_BIT)) + glFlush() + } else { + Task { @MainActor in + LoggingService.shared.debug( + "MPVOpenGLLayer: clearToBlack skipped glClear (no framebuffer bound)", + category: .mpv + ) + } + } + CGLUnlockContext(self.cglContext) // Force a display to show the cleared frame self.update(force: true)