search

mpv-player / mpv

🎥 Command line video player
https://mpv.io
Other
27.78k stars 2.86k forks source link

Video displacement and crash when toggling fullscreen #10964

Closed forthrin closed 9 months ago

forthrin commented 1 year ago

Important Information

Reproduction steps

$ defaults write com.apple.universalaccess reduceMotion 1 # seems to have no effect anymore
$ defaults write -g NSWindowResizeTime -float 0.001 # seems to have no effect anymore
$ mpv --config=no --native-fs=no --pause any-video.mp4
Press "f" successively.

Expected behavior

The video should go in and out of fullscreen instantly.

Actual behavior

The frames "..02" and "..03" should never happen. Display should go directly from "..01" to "..04".

Switching to native fullscreen introduces a transition effect, and sometimes even lags, so this is an undesirable option as it makes mpv feel sluggish (even though it may not be at fault).

Log file

mpv.log crash.log

Sample files

Happens with any video.

Screens

Displacement frames:

0103 0203 0303

See zip file for all frames.

screens.zip

Akemi commented 1 year ago

if this only happens with --no-native-fs this feature should finally be removed. i said it many times before when i had to fix shit Apple broke with this, if it does make any more problems it's not worth keeping and maintaining. Apple obviously doesn't want the none native fs and deliberately breaks it or changes its behaviour.

forthrin commented 1 year ago

As far as I can see, there are no glitches when using --native-fs, but the transition is slow, and lags frequently.

From your apparent frustration with the OS, throwing out the proprietary full screen code is understandable. (Unless someone else can come to the rescue with a viable solution.)

I will close this issue and test native fullscreen thoroughly, including issues in the two referenced reports, as I noticed that some of those things still happen (thus not related to the native fullscreen setting).

forthrin commented 1 year ago

On second thought, I'll leave it open until you make the final decision and commit the code removal.

low-batt commented 1 year ago

Apple's Human Interface Guidelines section Going full screen under macOS says:

Use the system-provided full-screen experience. Using the system’s full-screen support ensures that your full-screen window works well in all contexts. For developer guidance, see toggleFullScreen(_:).

If you use custom full-screen support, rely on system-defined areas to keep your full-screen window content unobscured.

Apple prefers macOS applications to use the full screen implementation built into AppKit, but applications are free to choose to implement their own custom full screen experience.

It doesn't feel like the Apple AppKit programmers are aware of this as changes in Big Sur added multiple defects in this area. The crash report in thie issue shows one of the problems:

2022-12-04 12:24:13.433 mpv[25768:478271] NSWindowStyleMaskFullScreen set on a window outside of a full screen transition. Called from (

The IINA preference Use legacy fullscreen enable's IINA's custom full screen feature. This feature was badly broken by changes in Big Sur. This code section from IINA's MainWindowController.legacyAnimateToFullscreen method shows the workaround added to avoid the crash reported in this issue:

    if #available(macOS 10.16, *) {
      window.styleMask.remove(.titled)
      (window as! MainWindow).forceKeyAndMain = true
      window.level = .floating
    } else {
      window.styleMask.insert(.fullScreen)
    }

Another workaround can be seen in MainWindowController.updateTitle to avoid a crash caused by calling NSWindow.setTitleWithRepresentedFilename.

And as mentioned in the HIG if you implement your own full screen experience then you must deal with the new Macs that have a camera housing intruding into the screen. Some of the code to do that can be seen in MainWindowController.setWindowFrameForLegacyFullScreen.

Quite a few changes were needed to get this IINA feature to work again with the changes made in Big Sur. I reported some of the problems to Apple. At least one was fixed in Monterey. Last I checked one was still outstanding.

So it is possible for an application to avoid using the AppKit supplied full screen code, but AppKit defects do make it challenging.

porsager commented 1 year ago

I don't think there's a new issue here is there? Isn't it just that the fix from #8490 never made it into any release?

forthrin commented 1 year ago

@porsager: ?? Why wasn't the patch included in the release? Why wouldn't all patches be included in official releases?

I think @Akemi meant to say not to revive old bug reports for previous releases, but rather create new ones with the latest release to re-test things, and also in case it's a different bug (which may be difficult for non-developers to know).

Akemi commented 1 year ago

the fix was committed, like i said on the other issue. the other issue is also closed and doesn't need any closing like suggested on the other issue.

it wasn't included in any release, at the time i made my comment, because we didn't have a new release at that time yet.

low-batt commented 1 year ago

Using the 0.35.0 build from stolendata.net I was able to reproduce the non-fatal NSWindowStyleMaskFullScreen exception. The following test was run on a MacBookPro18,2 (M1 Max chip) under macOS Ventura 13.0.1.

Terminal Output: ```bash low-batt@gag MacOS$ pwd /Users/low-batt/Downloads/sw/net/stolendata/mpv-0.35.0/mpv.app/Contents/MacOS low-batt@gag MacOS$ ./mpv --config=no --native-fs=no --pause --log-file=~/mpv.log ~/Movies/Airplaneski.mp4 (+) Video --vid=1 (*) (h264 682x438 25.000fps) (+) Audio --aid=1 (*) (aac 2ch 48000Hz) AO: [coreaudio] 48000Hz stereo 2ch floatp VO: [libmpv] 682x438 => 683x438 yuv420p 2022-12-04 14:20:47.868 mpv[77515:1578316] NSWindowStyleMaskFullScreen set on a window outside of a full screen transition. Called from ( 0 AppKit 0x00007ff808fb9298 __25-[NSWindow setStyleMask:]_block_invoke + 125 1 AppKit 0x00007ff808fb91c2 NSPerformVisuallyAtomicChange + 132 2 AppKit 0x00007ff808fb90a9 -[NSWindow setStyleMask:] + 172 3 mpv 0x00000001043ec0f3 $s11macOS_swift6WindowC9styleMaskSo013NSWindowStyleF0Vvs + 227 4 mpv 0x00000001043efb22 $s11macOS_swift6WindowC9styleMaskSo013NSWindowStyleF0VvsToTm + 34 5 mpv 0x00000001043ed804 $s11macOS_swift6WindowC15setToFullScreenyyF + 244 6 mpv 0x00000001043ec643 $s11macOS_swift6WindowC16toggleFullScreenyyypSgF + 1219 7 mpv 0x00000001043ec6b0 $s11macOS_swift6WindowC16toggleFullScreenyyypSgFTo + 96 8 mpv 0x0000000104407829 $s11macOS_swift6CommonC7control_6events7request4datas5Int32VSpySo2voVG_SpyAIGs6UInt32VSvSgtFyycfU0_TA + 9 9 mpv 0x00000001043e7a70 $sIeg_IeyB_TR + 32 10 libdispatch.dylib 0x00007ff805cb67fb _dispatch_call_block_and_release + 12 11 libdispatch.dylib 0x00007ff805cb7a44 _dispatch_client_callout + 8 12 libdispatch.dylib 0x00007ff805cc47b9 _dispatch_main_queue_drain + 952 13 libdispatch.dylib 0x00007ff805cc43f3 _dispatch_main_queue_callback_4CF + 31 14 CoreFoundation 0x00007ff805f5044d __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ + 9 15 CoreFoundation 0x00007ff805f10edb __CFRunLoopRun + 2498 16 CoreFoundation 0x00007ff805f0fe9f CFRunLoopRunSpecific + 560 17 HIToolbox 0x00007ff80fd37bd6 RunCurrentEventLoopInMode + 292 18 HIToolbox 0x00007ff80fd379e6 ReceiveNextEventCommon + 679 19 HIToolbox 0x00007ff80fd37723 _BlockUntilNextEventMatchingListInModeWithFilter + 70 20 AppKit 0x00007ff808f6db37 _DPSNextEvent + 909 21 AppKit 0x00007ff808f6c9b8 -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 1219 22 AppKit 0x00007ff808f5eff3 -[NSApplication run] + 586 23 mpv 0x00000001043da747 cocoa_main + 887 24 dyld 0x00000002048e9310 start + 2432 ) 2022-12-04 14:20:54.704 mpv[77515:1578316] NSWindowStyleMaskFullScreen set on a window outside of a full screen transition. Called from ( 0 AppKit 0x00007ff808fb9298 __25-[NSWindow setStyleMask:]_block_invoke + 125 1 AppKit 0x00007ff808fb91c2 NSPerformVisuallyAtomicChange + 132 2 AppKit 0x00007ff808fb90a9 -[NSWindow setStyleMask:] + 172 3 mpv 0x00000001043ec0f3 $s11macOS_swift6WindowC9styleMaskSo013NSWindowStyleF0Vvs + 227 4 mpv 0x00000001043efb22 $s11macOS_swift6WindowC9styleMaskSo013NSWindowStyleF0VvsToTm + 34 5 mpv 0x00000001043ed804 $s11macOS_swift6WindowC15setToFullScreenyyF + 244 6 mpv 0x00000001043ec643 $s11macOS_swift6WindowC16toggleFullScreenyyypSgF + 1219 7 mpv 0x00000001043ec6b0 $s11macOS_swift6WindowC16toggleFullScreenyyypSgFTo + 96 8 mpv 0x0000000104407829 $s11macOS_swift6CommonC7control_6events7request4datas5Int32VSpySo2voVG_SpyAIGs6UInt32VSvSgtFyycfU0_TA + 9 9 mpv 0x00000001043e7a70 $sIeg_IeyB_TR + 32 10 libdispatch.dylib 0x00007ff805cb67fb _dispatch_call_block_and_release + 12 11 libdispatch.dylib 0x00007ff805cb7a44 _dispatch_client_callout + 8 12 libdispatch.dylib 0x00007ff805cc47b9 _dispatch_main_queue_drain + 952 13 libdispatch.dylib 0x00007ff805cc43f3 _dispatch_main_queue_callback_4CF + 31 14 CoreFoundation 0x00007ff805f5044d __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ + 9 15 CoreFoundation 0x00007ff805f10edb __CFRunLoopRun + 2498 16 CoreFoundation 0x00007ff805f0fe9f CFRunLoopRunSpecific + 560 17 HIToolbox 0x00007ff80fd37bd6 RunCurrentEventLoopInMode + 292 18 HIToolbox 0x00007ff80fd379e6 ReceiveNextEventCommon + 679 19 HIToolbox 0x00007ff80fd37723 _BlockUntilNextEventMatchingListInModeWithFilter + 70 20 AppKit 0x00007ff808f6db37 _DPSNextEvent + 909 21 AppKit 0x00007ff808f6c9b8 -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 1219 22 AppKit 0x00007ff808f5eff3 -[NSApplication run] + 586 23 mpv 0x00000001043da747 cocoa_main + 887 24 dyld 0x00000002048e9310 start + 2432 ) 2022-12-04 14:20:56.436 mpv[77515:1578316] NSWindowStyleMaskFullScreen set on a window outside of a full screen transition. Called from ( 0 AppKit 0x00007ff808fb9298 __25-[NSWindow setStyleMask:]_block_invoke + 125 1 AppKit 0x00007ff808fb91c2 NSPerformVisuallyAtomicChange + 132 2 AppKit 0x00007ff808fb90a9 -[NSWindow setStyleMask:] + 172 3 mpv 0x00000001043ec0f3 $s11macOS_swift6WindowC9styleMaskSo013NSWindowStyleF0Vvs + 227 4 mpv 0x00000001043efb22 $s11macOS_swift6WindowC9styleMaskSo013NSWindowStyleF0VvsToTm + 34 5 mpv 0x00000001043ed804 $s11macOS_swift6WindowC15setToFullScreenyyF + 244 6 mpv 0x00000001043ec643 $s11macOS_swift6WindowC16toggleFullScreenyyypSgF + 1219 7 mpv 0x00000001043ec6b0 $s11macOS_swift6WindowC16toggleFullScreenyyypSgFTo + 96 8 mpv 0x0000000104407829 $s11macOS_swift6CommonC7control_6events7request4datas5Int32VSpySo2voVG_SpyAIGs6UInt32VSvSgtFyycfU0_TA + 9 9 mpv 0x00000001043e7a70 $sIeg_IeyB_TR + 32 10 libdispatch.dylib 0x00007ff805cb67fb _dispatch_call_block_and_release + 12 11 libdispatch.dylib 0x00007ff805cb7a44 _dispatch_client_callout + 8 12 libdispatch.dylib 0x00007ff805cc47b9 _dispatch_main_queue_drain + 952 13 libdispatch.dylib 0x00007ff805cc43f3 _dispatch_main_queue_callback_4CF + 31 14 CoreFoundation 0x00007ff805f5044d __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ + 9 15 CoreFoundation 0x00007ff805f10edb __CFRunLoopRun + 2498 16 CoreFoundation 0x00007ff805f0fe9f CFRunLoopRunSpecific + 560 17 HIToolbox 0x00007ff80fd37bd6 RunCurrentEventLoopInMode + 292 18 HIToolbox 0x00007ff80fd379e6 ReceiveNextEventCommon + 679 19 HIToolbox 0x00007ff80fd37723 _BlockUntilNextEventMatchingListInModeWithFilter + 70 20 AppKit 0x00007ff808f6db37 _DPSNextEvent + 909 21 AppKit 0x00007ff808f6c9b8 -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 1219 22 AppKit 0x00007ff808f5eff3 -[NSApplication run] + 586 23 mpv 0x00000001043da747 cocoa_main + 887 24 dyld 0x00000002048e9310 start + 2432 ) Exiting... (Quit) low-batt@gag MacOS$ ./mpv --version mpv 0.35.0 Copyright © 2000-2022 mpv/MPlayer/mplayer2 projects built on Sat Nov 12 21:24:01 CET 2022 FFmpeg library versions: libavutil 57.28.100 libavcodec 59.37.100 libavformat 59.27.100 libswscale 6.7.100 libavfilter 8.44.100 libswresample 4.7.100 FFmpeg version: 5.1.2 ```

I'm confused as to why so far I am not able to trigger that exception using mpv 0.35.0 installed by homebrew:

Homebrew: ```bash low-batt@gag ~$ which mpv /opt/homebrew/bin/mpv low-batt@gag ~$ mpv --config=no --native-fs=no --pause --log-file=mpv.log ~/Movies/Airplaneski.mp4 (+) Video --vid=1 (*) (h264 682x438 25.000fps) (+) Audio --aid=1 (*) (aac 2ch 48000Hz) AO: [coreaudio] 48000Hz stereo 2ch floatp VO: [libmpv] 682x438 => 683x438 yuv420p (Paused) AV: 00:00:00 / 01:11:53 (0%) A-V: 0.000 Exiting... (Quit) low-batt@gag ~$ mpv --version mpv 0.35.0 Copyright © 2000-2022 mpv/MPlayer/mplayer2 projects built on Sat Nov 12 16:25:05 UTC 2022 FFmpeg library versions: libavutil 57.28.100 libavcodec 59.37.100 libavformat 59.27.100 libswscale 6.7.100 libavfilter 8.44.100 libswresample 4.7.100 FFmpeg version: 5.1.2 ```
Akemi commented 1 year ago

maybe different SDK it was built with.

low-batt commented 1 year ago

That got me to remember encountering odd defective behavior with the Swift available attribute under Xcode 13.1. The Xcode 13 Release Notes mentioned problems in this area with iPhone and iPad apps:

Availability checks in iPhone and iPad apps on a Mac with Apple silicon always return true.

I concluded the problems with the available attribute extended way beyond what was documented in the release notes and was also not trustworthy for macOS applications under Xcode 13.1. I believe this was fixed in Xcode 13.2.

I was unable to determine what version of Xcode is being used for the stolendata.net build.

Another significant difference is that the 0.35.0 build from stolendata has to run under Rosetta:

low-batt@gag ~$ file Downloads/sw/net/stolendata/mpv-0.35.0/mpv.app/Contents/MacOS/mpv
Downloads/sw/net/stolendata/mpv-0.35.0/mpv.app/Contents/MacOS/mpv: Mach-O 64-bit executable x86_64

Whereas the homebrew installed mpv is native:

low-batt@gag ~$ file /opt/homebrew/bin/mpv
/opt/homebrew/bin/mpv: Mach-O 64-bit executable arm64
low-batt commented 1 year ago

Another question to consider is why the homebrew install of mpv 0.35.0 on my Mac is working for me and yet @forthrin is encountering the problem using a homebrew install.

@forthrin What kind of Mac, Intel or Apple Silicon? Mine is a MacBookPro18,2 with the M1 Mac chip.

The mpv Homebrew Formulae contains Bottles, so unless building from source was forced, or mpv is being installed on version of macOS for which there isn't a bottle, a binary package that was built by the Brew Test Bot is installed.

From the build date in the log @forthrin posted I'm thinking homebrew installed a bottle.

Looking at mpv.json shows there are separate bottles for Intel vs. Apple Silicon and for each major version of macOS.

The report from @forthrin lists macOS 12.6.1. I'm on macOS 13.0.1.

Seems likely we are using different builds from homebrew. What I was not able to figure out is what version of Xcode is being used by the Brew Test Bot for these builds. As there are different bottles for Monterey vs. Ventura a newer version of Xcode and SDK may be being used for the Ventura bottle.

Too much guessing. Need some more facts.

forthrin commented 1 year ago 
$ sw_vers 
ProductName:    macOS
ProductVersion: 12.6.1

$ system_profiler SPHardwareDataType
Model Identifier: MacBookAir7,2
Processor Name: Dual-Core Intel Core i5
Processor Speed: 1,8 GHz
Memory: 8 GB

I prefer bottled binaries. Did try to build from Git, but had some strange problems. Could try again if relevant for diagnostics.

low-batt commented 1 year ago

Up front I should mention that I am not one of the mpv developers. I chimed in because I had to deal with the macOS AppKit full screen related regressions introduced in Big Sur.

On this question:

Could try again if relevant for diagnostics.

That would give us another hard data point that might be very helpful in tracking down what is happening.

You would first want to make sure your copy of Xcode and the command line tools are up to date. Based on the "Minimum requirements and supported SDKs" table on the Xcode page you should be able to use Xcode 14.1.

Rather than try and build from a git clone you could reinstall using brew, forcing brew to build from source instead of installing the pre-built bottle. From the brew man page:

-s, --build-from-source: Compile formula from source even if a bottle is provided. Dependencies will still be installed from bottles if they are available.

If mpv built with Xcode 14.1 works then that would be further evidence pointing in the direction of some sort of problem with the build tools.

Here is what we know. This exception:

NSWindowStyleMaskFullScreen set on a window outside of a full screen transition.

Was introduced in macOS 11, Big Sur. Before Big Sur you could set a window's styleMask to fullScreen. This is what applications implementing their own custom full screen did. For reasons I can't explain, starting with macOS 11 AppKit is throwing the above exception if code uses this mask, effectively reserving use of it for the "system-provided full-screen experience".

Because of that mpv had to be changed to not use fullScreen starting with macOS 11. That change was committed and is present in mpv 0.35.0. From window.swift line 234:

        if #available(macOS 11.0, *) {
            styleMask = .borderless
            common.titleBar?.hide(0.0)
        } else {
            styleMask.insert(.fullScreen)
        }

That the exception is being thrown by 0.35.0 does not make sense. It is as if #available(macOS 11.0, *) in the above code is returning false causing styleMask.insert(.fullScreen) to be executed, thus triggering the exception.

How can that be happening? Worse yet, it happening for you with the homebrew install of 0.35.0 and but I am unable to reproduce it with the homebrew install of 0.35.0 on my machine.

I have pointed out that there definitely were problems with the available attribute in early versions of Xcode 13. The problems described in the Xcode 13 Release Notes do not indicate a problem with macOS:

Availability checks in iPhone and iPad apps on a Mac with Apple silicon always return true. This causes iOS apps running in macOS 11 Big Sur to see iOS 15 APIs as available, resulting in crashes.

But I experienced odd problems with available malfunctioning and concluded Xcode 13.1 was unreliable.

I have proposed a hypothesis that the problem at hand could be due to the problems with the available attribute in early versions of Xcode 13. I'm still looking into whether there is a way to figure out what version of Xcode was used to build these bottles.

If rebuilding with Xcode 14.1 corrects the problem for you then that would be another data point supporting the suspicion that this issue is due to problems with the build tools. If it still fails then we'd start looking elsewhere for a cause.

forthrin commented 1 year ago

Thanks for your engagement in the issue. I will try building from source both from Homebrew and a Git clone. (Note: I don't use XCode, but CLI Tools, and build with waf.) Will post my results back here.

low-batt commented 1 year ago

Inspecting the 0.35.0 mpv binary from stolendata.net with vtool shows:

low-batt@gag ~$ vtool -show-build /Applications/mpv.app/Contents/MacOS/mpv
/Applications/mpv.app/Contents/MacOS/mpv:
Load command 9
      cmd LC_BUILD_VERSION
  cmdsize 32
 platform MACOS
    minos 10.14
      sdk 10.14
   ntools 1
     tool LD
  version 450.3

I could not find official Apple documentation, but I did find the article Big Sur is both 10.16 and 11.0 – it’s official on The Electric Light Company web site which indicates:

For apps built with Xcode, the version returned depends on which version of its SDK they were built with. SDK 10.15 and earlier will consistently respond that Big Sur is major version 10 and minor version 16.

That seems to mean this statement:

        if #available(macOS 11.0, *) {

Would malfunction and return false if executed on Big Sur. What would the old SDK return under Ventura? If the 10.14 SDK is reporting Ventura as 10.x then this would explain why the stolendata build is exhibiting the problem for me.

Checking the homebrew bottle based installation on my machine shows:

low-batt@gag ~$ vtool -show-build /opt/homebrew/bin/mpv 
/opt/homebrew/bin/mpv:
Load command 11
      cmd LC_BUILD_VERSION
  cmdsize 32
 platform MACOS
    minos 13.0
      sdk 13.0
   ntools 1
     tool LD
  version 820.1
low-batt@gag ~$

@forthrin If you haven't already changed your homebrew installation, what does vtool -show-build output on your machine?

forthrin commented 1 year ago

Homebrew bottle:

$ vtool -show-build $(which mpv)
/usr/local/bin/mpv:
Load command 9
      cmd LC_BUILD_VERSION
  cmdsize 32
 platform MACOS
    minos 10.14
      sdk 10.14
   ntools 1
     tool LD
  version 450.3
Akemi commented 1 year ago

the @available check is a runtime check. what vtool or otool returns ist just the SDK the binary was built with and the minimum OS version the binary runs on. those are not related.

[edit] think i misunderstood your statement. yeah i think your conclusion is right in that regard. another problem with 11.0 and 10.16 numbering was fixed sometime with xcode 12(.2) (was also introduced with xcode 12, i believe, though). so this should be kept in mind too.

using an old SDK to build isn't really good practice imo, especially with what Apple did in the past years.

stolendata commented 1 year ago

@forthrin @low-batt I can't reproduce the crash itself on neither Monterey nor on the older Mojave setup where I built 0.35.0. When I enable reduceMotion and run mpv with your arguments I can still toggle between fullscreen and windowed instantaneously without crashes, glitches or misalignment. Only the non-fatal exception message shows up, which at least in my case isn't interfering with the playback.

The reason I've been building on older SDKs is to provide something for users still stuck with older setups. From the e-mail feedback I receive there are a lot of them. I won't "re-release" a new build of v0.35.0 as it is, for all I can tell, fully functional after all. However, my older MacBook running Mojave is no longer with me so future builds from me will be based on SDK 12 and require Monterey to run.

forthrin commented 1 year ago

@stolendata: Thanks for the update. I run Monterey myself, and presumably Ventura next year when I replace the Air 2017.

"Det måste ju vara definitionen på ironi. När en dansk ber dig prata tydligare." -- Johan Glans

Akemi commented 1 year ago

The reason I've been building on older SDKs is to provide something for users still stuck with older setups. From the e-mail feedback I receive there are a lot of them. I won't "re-release" a new build of v0.35.0 as it is, for all I can tell, fully functional after all. However, my older MacBook running Mojave is no longer with me so future builds from me will be based on SDK 12 and require Monterey to run.

i might say the obvious now though i want to mention it as a general info. the SDK isn't related to the minimum OS version an app can run on. one can still use a recent SDK and set the minimum (target version) lower (swift -target option, clfags -mmacosx-version-min, env variable MACOSX_DEPLOYMENT_TARGET).

stolendata commented 1 year ago

i might say the obvious now though i want to mention it as a general info. the SDK isn't related to the minimum OS version an app can run on. one can still use a recent SDK and set the minimum (target version) lower (swift -target option, clfags -mmacosx-version-min, env variable MACOSX_DEPLOYMENT_TARGET).

That's how I go about my build. But, the method has limited reach. For reasons I could never fully pin down I every now and then ended up with builds that didn't run on older versions of OS X, despite executable and shared libs' LC_VERSION_MIN_MACOSX implying compatiblity. It's possible that some frameworks (or dependencies) define different compile-time portions based on that environment variable. In the end I resorted to keeping an older version OS X/SDK around for building and testing.

stolendata commented 1 year ago

@forthrin I'm curious... What happens regarding glitching/misalignment if you revert or undefine NSWindowResizeTime in your global domain? I don't have this user default at all, only reduceMotion enabled, and toggling between fullscreen and windowed is still instant for me and it never ends up lopsided or otherwise misrendered.

forthrin commented 1 year ago

Well, I just used mpv-build and it does not crash. The glitches also seem to be better (or possibly non-existant). I want to test this properly before I give my final verdict. "I'll be back."

Having said this, most mpv users probably don't build from source, and I would hardly like to do so myself, unless I absolutely must. I like the convenience of centralized package systems that update all my software in one go. It would be nice looking into how packages can be updated more frequently, so users aren't consistently stuck with year-old versions.

Akemi commented 1 year ago

That's how I go about my build. But, the method has limited reach. For reasons I could never fully pin down I every now and then ended up with builds that didn't run on older versions of OS X, despite executable and shared libs' LC_VERSION_MIN_MACOSX implying compatiblity. It's possible that some frameworks (or dependencies) define different compile-time portions based on that environment variable. In the end I resorted to keeping an older version OS X/SDK around for building and testing.

yeah, completely understandable and i can relate. it's a lot easier, less work and less error prone.

low-batt commented 1 year ago

A lot of my comments on the root cause are speculation based on insufficient evidence. Please do question my statements about what might be happening. Normally I prefer to report definitive facts, but I do not have an environment with old build tools to run tests with.

A summary of the test results so far: User Build Arch macOS SDK Behavior
forthrin homebrew Intel 12.6.1 10.14 Exception
forthrin mpv-build Intel 12.6.1 ? Working
low-batt homebrew Apple 13.0.1 13 Working
low-batt stolendata Intel (Rosetta) 13.0.1 10.14 Exception
low-batt mpv-build Apple 13.0.1 13 Working

Assuming this statement from the article I mentioned above is correct:

For apps built with Xcode, the version returned depends on which version of its SDK they were built with. SDK 10.15 and earlier will consistently respond that Big Sur is major version 10 and minor version 16.

Then in order to properly support building with an old SDK these two statements:

video/out/mac/window.swift:234:        if #available(macOS 11.0, *) {
video/out/mac/window.swift:252:        if #available(macOS 11.0, *) {

Will need to be changed to use the original 10.16 version number for Big Sur.

On why building with the latest tools and setting MACOSX_DEPLOYMENT_TARGET to an older macOS version sometimes fails to work, I can provide a specific example. See this comment on IINA issue https://github.com/iina/iina/issues/3327. Short story, some of the techniques Apple uses can cause Autoconf templates to malfunction.

I still can't find official Apple documentation describing the behavior of the Swift available attribute with respects to Big Sur and SDKs. Apple probably should have mentioned the special issue with 10.16 vs 11 in Running code on a specific platform or OS version.

Akemi commented 1 year ago

Will need to be changed to use the original 10.16 version number for Big Sur.

out of the question. this would break other things or would need compile time checks too, which would be a big whole mess imo. as with Apple we only (actively) support the latest 3 macOS versions. if an old SDK is broken or incompatible then we just won't support it anymore.

[edit] we could probably add a configure check so it won't build anymore at all. if that is a desirable behaviour.

low-batt commented 1 year ago

If availablilty testing for 10.16 breaks something I'm very interested as IINA is still has checks that have not been updated. Just to be sure I quickly coded up this:

if #available(macOS 10.16, *) {
  print("macOS 10.16 available")
}
if #available(macOS 11, *) {
  print("macOS 11 available")
}

Using Xcode 14.1 and SDK 13 under macOS 13.0.1 the output is:

macOS 10.16 available
macOS 11 available
Akemi commented 1 year ago

yeah your example works, but in that case the code would be called twice. nesting the ifs wouldn't work either, or it would need some check that it was already called, or a compile time check für the SDK version as a workaround. maybe i am too tired and overlook the obvious here.

sry for being a bit vague, that specific check might not break anything. though if we take that a bit further. checks for macOS versions after 11, everything >=12 (might) break. my assumption was that 12 is 10.17 and so on (at least on sdk 10.14 or compat mode). if the available check is a simple bigger than check, in the case of #available(macOS 10.17, *) being true #available(macOS 11, *) would also be true. that's just my assumption. for 10.16/11 that seems okay because there are no more versions in-between those two versions, but between 10.17 and 12 there is 11 and so on.

a simple test shows that it seems to be a whole bigger mess. 12.6 for example would also identify as 10.16 in compat mode. so with compat mode version checks we can't properly check for versions after 10.16/11(?).

akemi ~ % cat /System/Library/CoreServices/SystemVersion.plist 
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
    <key>ProductBuildVersion</key>
    <string>21G217</string>
    <key>ProductCopyright</key>
    <string>1983-2022 Apple Inc.</string>
    <key>ProductName</key>
    <string>macOS</string>
    <key>ProductUserVisibleVersion</key>
    <string>12.6.1</string>
    <key>ProductVersion</key>
    <string>12.6.1</string>
    <key>iOSSupportVersion</key>
    <string>15.7</string>
</dict>
</plist>

akemi ~ % SYSTEM_VERSION_COMPAT=1 cat /System/Library/CoreServices/SystemVersion.plist
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
    <key>ProductBuildVersion</key>
    <string>21G217</string>
    <key>ProductCopyright</key>
    <string>1983-2022 Apple Inc.</string>
    <key>ProductName</key>
    <string>Mac OS X</string>
    <key>ProductUserVisibleVersion</key>
    <string>10.16</string>
    <key>ProductVersion</key>
    <string>10.16</string>
    <key>iOSSupportVersion</key>
    <string>15.7</string>
</dict>
</plist>

akemi ~ % SYSTEM_VERSION_COMPAT=0 cat /System/Library/CoreServices/SystemVersion.plist
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
    <key>ProductBuildVersion</key>
    <string>21G217</string>
    <key>ProductCopyright</key>
    <string>1983-2022 Apple Inc.</string>
    <key>ProductName</key>
    <string>macOS</string>
    <key>ProductUserVisibleVersion</key>
    <string>12.6.1</string>
    <key>ProductVersion</key>
    <string>12.6.1</string>
    <key>iOSSupportVersion</key>
    <string>15.7</string>
</dict>
</plist>

i am still of the opinion to just scrap SDK 10.14 support and use none compat mode version numbers.

low-batt commented 1 year ago

Yes, I agree. Once there is a need to check availability beyond Big Sur, it seems like the old SDK simply can't do that.

I'm not pushing for changing those statements to use 10.16. I was just pointing out that seems to be what is needed for the code to work as intended if built with SDK 10.14.

To build IINA the latest version of Xcode must be used. IINA sets MACOSX_DEPLOYMENT_TARGET to maintain compatibility with older versions of macOS. This way of supporting old macOS versions does suffer from the problems @stolendata mentioned. As an example of the "limited reach" @stolendata referred to, the latest Xcode/SDK can no longer build for deployment to OS releases older than macOS 10.13.

On this from @stolendata:

I can still toggle between fullscreen and windowed instantaneously without crashes, glitches or misalignment.

I am experiencing two glitches.

When in full screen there is a distracting light colored line across the very top of the screen (I have macOS appearance set to Light). This is especially obvious with wide aspect videos.

If while in full screen I move the cursor to the top of the screen to cause the menu bar to appear to say check the time, when the menu bar disappears the top left of the screen now displays the red, yellow and green buttons. Leaving full screen and going back into full screen sometimes clears the problem, sometimes not.

I do not experience either of these problems with a build that does not report the NSWindowStyleMaskFullScreen exception.

What I'm not experiencing is the lagging @forthrin reports.

low-batt commented 1 year ago

Based on this from @stolendata:

However, my older MacBook running Mojave is no longer with me so future builds from me will be based on SDK 12 and require Monterey to run

Being able to build with an old SDK may be a mute point.

But we still have to answer the question: Why is the homebrew install broken for @forthrin?

The mpv installation page contains:

Homebrew (without macOS application bundles) https://github.com/homebrew/homebrew-core/blob/master/Formula/mpv.rb

That formula contains:

  bottle do
    sha256 arm64_ventura:  "68bf624f6b6225aff7a18e7ebf133f6a2f0d81227a1a9122e327c8df980f1186"
    sha256 arm64_monterey: "796101aafdfcab4e4e583b106913f2a6eaa9ec457b59231d3474f44564930d77"
    sha256 arm64_big_sur:  "d3c50d2df9634d918459aac2b48203f203efdc1e2b7ec385db54186c7923074f"
    sha256 ventura:        "84bd369996a53ea59179b0388074b89acdacf8b67ecc9858d4d67e9bb8677872"
    sha256 monterey:       "4bf17644be73ef2e0b5d9029a73ccbab14e46f8af6f331ae5ef5329b33be4ab5"
    sha256 big_sur:        "a7ede2a10b6bc0b59d980f5d731a221fdd0dc450fd909e594e8c877031e72a7f"
    sha256 catalina:       "9b4b79cfceb6ed515080e7094fcb3361dab288ba4541f7d9f0ea9176d34a6445"
    sha256 x86_64_linux:   "9123423529705479772f97524aff7c8a6536733ca47d3c823c7c0b7cbdb1db94"
  end

That seems to indicate there are separate bottles for each major macOS version. Given that I was expecting the binaries would be built with build tool versions associated with that particular version of macOS. The use of SDK 13 for the arm64_ventura bottle as vtool reported on my machine lines up with that idea. But the output of vtool from @forthrin suggests the use of SDK 10.14 for the monterey bottle. Confusion!

From Bottles:

Bottles for homebrew/core formulae are created by Brew Test Bot when a pull request is submitted. If the formula builds successfully on each supported platform and a maintainer approves the change, Brew Test Bot updates its bottle do block and uploads each bottle to GitHub Packages

Searching GitHub Packages I found core/mpv . Clicking on the OS / Arch tab shows 7 0.35.0 packages for macOS, all for arm64. Where are the Intel packages? More confusion!

Poking around the net I found recommendations to use the mpv cask when installing via homebrew. Looking at the Cask Code it is accessing: https://laboratory.stolendata.net/~djinn/mpv_osx/mpv-#{version}.tar.gz

My homebrew installation of mpv did not use the cask. @forthrin I assume your homebrew installation used the mpv cask and thus installed the build from stolendata?

Akemi commented 1 year ago

maybe mpv installed before an OS update.

forthrin commented 1 year ago

Using brew install homebrew/cask/mpv which by all acounts uses the stolendata binary, yes.

Can't use the regular brew brew install mpv as it compiles FFmpeg plus a ton of dependencies I don't need, plus I've modified FFmpeg to feature a compact progress bar that doesn't flood the terminal, and a sorely missed ETA indicator.

PS! The lagging happens with --native-fs, so assumedly not related to mpv. This might be simply be too weak CPU/GPU AKA the regular old intentional software bloat to make people upgrade their hardware.

PPS! After cloning mpv, how do I to make waf point to a custom FFmpeg? waf --help has no options for this.

low-batt commented 1 year ago

I think we now have a pretty full picture of what is going on.

Let me start off by saying:

My attempt at a summary:

Anyone know why homebrew is lacking bottles for Intel-based Macs?

EDIT: Corrected summary. @makigumo Confirmed homebrew install of mpv 0.35.0 on Intel Macs is using bottles.

makigumo commented 1 year ago

Anyone know why homebrew is lacking bottles for Intel-based Macs?

For me it is not.

brew install homebrew/core/mpv
==> Fetching mpv
==> Downloading https://ghcr.io/v2/homebrew/core/mpv/manifests/0.35.0
######################################################################## 100.0%
==> Downloading https://ghcr.io/v2/homebrew/core/mpv/blobs/sha256:84bd369996a53ea59179b0388074b89acdacf8b67ecc9858d4d67e9bb8677872
==> Downloading from https://pkg-containers.githubusercontent.com/ghcr1/blobs/sha256:84bd369996a53ea59179b0388074b89acdacf8b67ecc9858d4d67e9bb8677872?se=2022-
######################################################################## 100.0%
==> Pouring mpv--0.35.0.ventura.bottle.tar.gz
==> Caveats
zsh completions have been installed to:
  /usr/local/share/zsh/site-functions
==> Summary
🍺  /usr/local/Cellar/mpv/0.35.0: 32 files, 11.0MB
forthrin commented 1 year ago

Using a MacBook Air 2017 with 1440x900 screen using a locally built mpv-build:

# mpv 25b6625 libplacebo version: v5.233.0 FFmpeg version: git-2022-12-08-f0f19f3
$ defaults write com.apple.universalaccess reduceMotion 1 # remove horizontal sliding in native fullscreen
$ defaults delete -g NSWindowResizeTime # is this the same as macos-fs-animation-duration?
$ for f in yes no; do mpv --config=no --pause --native-fs=$f --macos-fs-animation-duration=0 720p.mp4; done

Steps taken from #10311 (feel free to close). See attached screens.

  1. f (Enter)
  2. f (Exit)
  3. f (Enter)
  4. ⌘2
  5. f (Exit)
  6. ⌘1

The following unwarranted effects were observed for --native-fs=yes:

The following unwarranted effects were observed for --native-fs=no:

I couldn't reproduce these previously mentioned bugs in either fullscreen mode anymore:

Also, I couldn't reproduce the previously mentioned lagging during transition in native fullscreen, but there is an abrubt "snap" at the beginning and the end of the transition. Don't know if mpv or macOS is at fault. See attached video.

$ mpv --config=no --pause --native-fs=yes --macos-fs-animation-duration=1000 video.mp4

https://user-images.githubusercontent.com/1167421/206840807-2bcb9ae1-359a-49e4-ac1b-556cdfb59264.mp4

low-batt commented 1 year ago

@makigumo Thanks for pointing out my mistake. When I checked core/mpv the font was small enough that I thought all were marked "arm64". Checking again I see some bottles are marked as "amd64".

Of course this means I can't explain why @forthrin reported brew install mpv compiled lots of dependencies.

forthrin commented 1 year ago

@low-batt: As you know, Homebrew stopped supporting options, so FFmpeg installs a ton of dependencies which I don't need.

Also since I have modified the source, I don't need (or want) Homebrew to install another FFmpeg, as this may create confusion as to which FFmpeg version is used for different purposes. (Eg. when building mpv without mpv-build, I couldn't figure out how to tell waf to use my own version.

Since I now use mpv-build, which does this automatically, I do my modifications in that FFmpeg version.

Finally, is there any way to tell Homebrew, "Never install brew X (eg. FFmpeg) and direct all dependencies to my own installation (which comes directly from the Git head)?"

$ brew info mpv
==> mpv: stable 0.35.0 (bottled), HEAD
From: https://github.com/Homebrew/homebrew-core/blob/HEAD/Formula/mpv.rb
==> Dependencies
Build: docutils ✘, pkg-config ✔, python@3.10 ✔
Required: ffmpeg ✘, jpeg-turbo ✔, libarchive ✘, libass ✔, little-cms2 ✔, luajit ✘, mujs ✘, uchardet ✘, vapoursynth ✘, yt-dlp ✔

~$ brew info ffmpeg
==> ffmpeg: stable 5.1.2 (bottled), HEAD
From: https://github.com/Homebrew/homebrew-core/blob/HEAD/Formula/ffmpeg.rb
==> Dependencies
Build: pkg-config ✔, nasm ✔
Required: aom ✔, dav1d ✔, fontconfig ✔, freetype ✔, frei0r ✘, gnutls ✔, lame ✔, libass ✔, libbluray ✘, librist ✘, libsoxr ✘, libvidstab ✘, libvmaf ✔, libvorbis ✔, libvpx ✘, opencore-amr ✘, openjpeg ✔, opus ✔, rav1e ✘, rubberband ✘, sdl2 ✔, snappy ✘, speex ✘, srt ✘, tesseract ✘, theora ✘, webp ✔, x264 ✔, x265 ✔, xvid ✘, xz ✔, zeromq ✘, zimg ✔
low-batt commented 1 year ago

On this:

As you know, Homebrew stopped supporting options, so FFmpeg installs a ton of dependencies which I don't need.

I don't know homebrew that well. I still see options listed for the install command in the brew man page so I'm confused about what homebrew stopped supporting. I did find issues complaining about --ignore-dependencies not being an officially supported option.

Install has this option:

--HEAD: If formula defines it, install the HEAD version, aka. main, trunk, unstable, master.

That has to do with this homebrew feature: Unstable versions (head). The FFmpeg formula does provide a definition for HEAD. Possibly installing FFmpeg with --HEAD and then using the pin command:

pin installed_formula […]

Pin the specified formula, preventing them from being upgraded when issuing the brew upgrade formula command. See also unpin.

Would provide a homebrew installed version from the latest FFmpeg sources. Not sure if that will cause trouble with the mpv install which is expecting a specific version.

Maybe somebody who really knows homebrew will chime in.

forthrin commented 1 year ago

@low-batt: Options are (were) build options, whereas dependencies are external libraries (not compiled into the app, but required to reside in $PATH or elsewhere).

I assume (fear) that Homebrew is built on the premise that it controls which versions are used as dependencies, to avoid bugs caused by using slightly "off" versions, so probably they intentionally dropped ways to skip specific dependencies.

https://stackoverflow.com/questions/55190888/how-to-prevent-homebrew-from-installing-a-certain-formula-dependency

forthrin commented 1 year ago

Well, I just used mpv-build and it does not crash. The glitches also seem to be better (or possibly non-existant). I want to test this properly before I give my final verdict. "I'll be back."

I just started using mpv-build (latest version) on a MacBook Air M1 on macOS 13.2, and the exception issue related to entering full screen has re-emerged though with a slightly different stack trace, identical to #11018 and #11141.

This workaround is not viable as I would like my menu bar to behave as default ("In Full Screen Only").

Akemi commented 9 months ago

i would like to close this issue. the initial reported issue has been 'resolved', or rather the problem was fixed with newer SDKs, we dropped support for 10.14 and on 10.15 the newest SDK is also 11.0. for now i also don't see a reason to remove the none native fullscreen since it works properly.

@forthrin you reported some more issues in you comment https://github.com/mpv-player/mpv/issues/10964#issuecomment-1345183040. if you still experience those issues it probably makes sense to report those as separate issues, otherwise i will also lose the overview.

forthrin commented 9 months ago

Running macOS 14.1.1 and f551a9d.

1) Switching between window and full screen mode:

@Akemi: You said native-fs would be deprecated. Will it? And if so, will yes or no be kept as the standard?

2) 1080p video on M1, entire window ends off-screen. Admittedly more an academic problem than a practical one.

  1. f (Enter)
  2. f (Exit)
  3. f (Enter)
  4. ⌘2
  5. f (Exit)
  6. ⌘0
Akemi commented 9 months ago

i just answered those questions in my comment above yours and the whole discussion. like i said, please report your issues as new issues otherwise it becomes completely confusing, eg one problem per issue, since this issue is closed now.