-
Notifications
You must be signed in to change notification settings - Fork 29
IIna freezes/crashes video frame freeze when playback is fast and then paused and unpaused #11
Comments
@low-batt Can you please have a look at this issue? |
@ViRo3 Thank you for reporting this. I will try and reproduce the freeze. Need a spindump of IINA when it is frozen. @CarterLi Are IINA=Plus releases built without including symbols? The stack trace from the crash report does not contain method names:
So we know IINA was notifiied about something it was observing and tried to update mpv. My guess is that during shutdown observers are not being released before shutting down mpv. I will look into that. |
@ViRo3 Just noticed the PS title issue. Which version of macOS? Do you have the preference "Use legacy full screen" enabled? Can you say a little more about the behavior you are seeing? Are you experiencing the title issue in both IINA and IINA-Plus? The only window title issue I am aware of is issue iina#3543 which only happens when legacy full screen is used when running under Big Sur and Monterey. That is a defect in AppKit. IINA-Plus has a fix (workaround) for that involving setting the title in a different way. That fix is not in the released version of IINA which normally crashes as described in that issue. |
I did get method names on the stack trace when I was encountering the Code Signature Invalid issue |
@CarterLi Have you been able to reproduce the freeze or the title issue? Please see if you can reproduce it and if so, take a spindump. I've tried this sequence:
So far I've not generated a freeze. Tried a couple of different movie files. I was only able to generate these warnings from mpv:
I have successfully reproduced a crash in I also encountered an issue with legacy full screen where the black window that blacks out the area around the camera housing was not closed. I will be fixing that as well. Might not get these fixes done until Friday. The crash does not look to be related to the freeze or the title issue. My guess on why the crash report does not contain symbols is that this crash occurred late in the application termination process and macOS was in the process of unloading code and symbols from memory. But that is just a guess. |
Here's a video of the title menu issue : Screen.Recording.2021-12-30.at.12.27.56.PM.mov(my bad but the title menu issue is separate and in hindsight I should have set it separately) |
This was also reported against IINA in issue iina#3596 The commit in the pull request will: - Rename the PlayerCore property isMpvTerminated to isMpvTerminating to reflect that termination has been initiated and being processed asynchronously - Change existing references to isMpvTerminated to use new name - Change PlayerCore.terminateMPV to set isMpvTerminating to true right before starting the termination process - Change PlayerCore.syncUI to do nothing if mpv is terminating - Change MPVController.mpvQuit to remove observers MPVController registered before sending the quit command to mpv - Change many MPVController methods to check to see if mpv is being terminated
I have reported the crash against IINA in issue iina#3596. That leaves two other problems, freeze and title. The "frame freezes" is more concerning. Is the IINA application freezing, as in macOS shows the spinning beachball wait cursor? Or is it that IINA acts like it is still playing the movie, with the progress slider still moving, but the picture is frozen? If it is the former we need a spin dump. If it is the later then this may be an issue with mpv. It may be sensitive to the format of the particular movie being played and the type of Mac hardware it is being played on and the version of macOS. Logs may provide a clue, especially the mpv log. The title issue is that when going from full screen to windowed most of the window shrinks back to the previous size, but while that is happening the title part doesn't animate and remains the full width of the screen? That is what it looked like from the posted movie. That is definitely odd and looks broken. |
Screen.Recording.2021-12-31.at.5.59.52.AM.movAttached is a screen recording of the freeze. |
@ViRo3: Thanks again for your help with these issues. Good news. My guesses on the freeze were totally wrong. From a quick look this seems to be related to a recent IINA fix, not mpv. @CarterLi: We must resolve the issue with missing debug symbols. I downloaded the latest IINA-Plus release and confirmed I see the same issue of not getting symbols in the spindump. If I run IINA I do get symbols. I looked into this a little. The Xcode project for both IINA and IINA-Plus is configured for "DWARF with dSYM File" for release builds. Which means debug symbols end up in a separate file. I didn't see a separate symbol file in either application directory trees but I could have missed it. How are you building IINA-Plus for release? |
I was able to successfully build an archive using the Xcode line from the Makefile. This is what I see in the archive:
The debug symbols are not included in the executables in IINA.app and are instead separated out into these files. This is why names of methods are missing from crash reports and spin dumps. It might work on the machine upon which the build was done as macOS will search the local machine for dSYM files using spotlight. The idea behind this is that some creators of propritary software do not want to make it easy for hackers to decode. They don't want the hackers to know the names of modules and methods of their software. There is also a tiny performance advantage as the symbols add to the size of the executable. That used to be an important issue. Probably not signifcant these days. I've not found where IINA keeps its build script that creates the release, only the homebrew scripts for building dependencies. My guess it that they are not using the release configuration we see in the Xcode project, or somehow overriding the debug symbol handling. In Xcode open the project and go to the Build Settings tab. Then look for the "Debug Information Format" section. Under that you will see configuration for Debug and Release. Debug is just "DWARF" whereas Release is "DWARF with dSYM File". That is what is causing the symbols to not be included. Change that to just "DWARF" like it is for Debug. We need a build with working symbols that ViRo3 can use to generate another spindump, this time with symbols. |
Just FYI, that gives me the info I needed. Currently researching this a lot to make sure the next fix works. |
The commit changes the order of acquiring locks in both of the ViewLayer draw methods to first lock the OpenGL context before locking uninitLock.
@ViRo3, @CarterLi, Thank you both for all the help on the freeze. The short story on this one is that I added locking and setting of the OpenGL context that was missing from one of the Let's see if this fixes for ViRo3. I'm keeping my fingers crossed. I have not forgotten about the title issue. ReproducingThis is a thread race condition so you have to try reproducing it over and over again until you get lucky. The way to do that is to play a video and set a very short A-B loop so that playback is quickly restarted over and over again and just let it run until it hangs. AnalysisFrom the spin dump 1.txt we can see the controller thread is stopped in
That must be this call to override func draw(inCGLContext ctx: CGLContextObj, pixelFormat pf: CGLPixelFormatObj, forLayerTime t: CFTimeInterval, displayTime ts: UnsafePointer<CVTimeStamp>?) {
let mpv = videoView.player.mpv!
needsMPVRender = false
videoView.uninitLock.lock() The mpvgl thread is stopped in
That is the call to func draw(forced: Bool = false) {
needsMPVRender = true
if forced { forceRender = true }
display()
if forced {
forceRender = false
return
}
if needsMPVRender {
videoView.uninitLock.lock()
// draw(inCGLContext:) is not called, needs a skip render
if !videoView.isUninited, let renderContext = videoView.player.mpv?.mpvRenderContext {
videoView.player.mpv?.lockAndSetOpenGLContext() The attempt to lock the OpenGL context happens after locking the So which thread has the OpenGL context? The Apple documentation is terrible. This is the documentation for the draw method the controller thread is stopped in: draw(inCGLContext:pixelFormat:forLayerTime:displayTime:) What is the status of the OpenGL context in the This example from CALayerEssentials: // 4) [Required] Implement this message in order to actually draw anything.
// Typically you will do the following when you recieve this message:
// 1. Draw your OpenGL content! (the current context has already been set)
// 2. call [super drawInContext:pixelFormat:forLayerTime:displayTime:] to finalize the layer content, or call glFlush().
// NOTE: The viewport has already been set correctly by the time this message is sent, so you do not need to set it yourself.
// The viewport is automatically updated whenever the layer is displayed (that is, when the -display message is sent).
// This is arranged for when you send the -setNeedsDisplay message, or when the needsDisplayOnBoundsChange property is set to YES
// and the layer's size changes.
-(void)drawInCGLContext:(CGLContextObj)glContext pixelFormat:(CGLPixelFormatObj)pixelFormat forLayerTime:(CFTimeInterval)timeInterval displayTime:(const CVTimeStamp *)timeStamp
{
// Set the current context to the one given to us.
CGLSetCurrentContext(glContext); Says "(the current context has already been set)" in the header comment and then the first line of the code calls I am thinking code in I was able to find this statement in OpenGL Programming Guide for Mac:
So it is possible for The change that introduced this problem was the addition of proper OpenGL context management to the method FixingThe fix for the fix changes the order of locking in both |
No problem! And Happy New Year @CarterLi @low-batt ! Hope the best year for you! But here's additional log for a crash that came without the freeze |
This commit will add locking of the OpenGL context to the VideoView.uninit method in order to match the order of lock acquisition the ViewLayer.draw methods use.
My bad yet again. I did a bunch of testing before pushing that last fix, but for whatever reason the timing of thread interaction on my Mac is not picking up these problems. The latest spindump shows the main thread blocked in
With the controller thread blocked in
I'm thinking the The I have made that change. A little worried about whether there are other issues due to this change in lock ordering. I have just done two 500 test runs using this script: #!/bin/bash
for ((i = 1 ; i <= 500 ; i++)); do
echo "Run $i"
./iina-cli --keep-running ~/Movies/issue-3331.mp4
if [ $? -ne 0 ]; then
echo "FAILED"
fi
done No crashes. No hangs. But with thread race conditions that doesn't mean a problem can't appear on another Mac. |
Always need a full spin dump to investigate hangs. This tells us mpv for some reason blocked a call to The call to I will see if I can reproduce. |
I was unable to reproduce the problem. My guess is that I'm going to hold off pushing any more changes for now so we can be sure IINA-Plus has been stabilized. |
This was also reported against IINA in issue iina#3596 The commit in the pull request will: - Rename the PlayerCore property isMpvTerminated to isMpvTerminating to reflect that termination has been initiated and being processed asynchronously - Change existing references to isMpvTerminated to use new name - Change PlayerCore.terminateMPV to set isMpvTerminating to true right before starting the termination process - Change PlayerCore.syncUI to do nothing if mpv is terminating - Change MPVController.mpvQuit to remove observers MPVController registered before sending the quit command to mpv - Change many MPVController methods to check to see if mpv is being terminated
The commit changes the order of acquiring locks in both of the ViewLayer draw methods to first lock the OpenGL context before locking uninitLock.
This commit will add locking of the OpenGL context to the VideoView.uninit method in order to match the order of lock acquisition the ViewLayer.draw methods use.
This was also reported against IINA in issue iina#3596 The commit in the pull request will: - Rename the PlayerCore property isMpvTerminated to isMpvTerminating to reflect that termination has been initiated and being processed asynchronously - Change existing references to isMpvTerminated to use new name - Change PlayerCore.terminateMPV to set isMpvTerminating to true right before starting the termination process - Change PlayerCore.syncUI to do nothing if mpv is terminating - Change MPVController.mpvQuit to remove observers MPVController registered before sending the quit command to mpv - Change many MPVController methods to check to see if mpv is being terminated
The commit changes the order of acquiring locks in both of the ViewLayer draw methods to first lock the OpenGL context before locking uninitLock.
This commit will add locking of the OpenGL context to the VideoView.uninit method in order to match the order of lock acquisition the ViewLayer.draw methods use.
This commit will add locking of the OpenGL context to the VideoView.uninit method in order to match the order of lock acquisition the ViewLayer.draw methods use.
This was also reported against IINA in issue iina#3596 The commit in the pull request will: - Rename the PlayerCore property isMpvTerminated to isMpvTerminating to reflect that termination has been initiated and being processed asynchronously - Change existing references to isMpvTerminated to use new name - Change PlayerCore.terminateMPV to set isMpvTerminating to true right before starting the termination process - Change PlayerCore.syncUI to do nothing if mpv is terminating - Change MPVController.mpvQuit to remove observers MPVController registered before sending the quit command to mpv - Change many MPVController methods to check to see if mpv is being terminated
The commit changes the order of acquiring locks in both of the ViewLayer draw methods to first lock the OpenGL context before locking uninitLock.
This commit will add locking of the OpenGL context to the VideoView.uninit method in order to match the order of lock acquisition the ViewLayer.draw methods use.
This was also reported against IINA in issue iina#3596 The commit in the pull request will: - Rename the PlayerCore property isMpvTerminated to isMpvTerminating to reflect that termination has been initiated and being processed asynchronously - Change existing references to isMpvTerminated to use new name - Change PlayerCore.terminateMPV to set isMpvTerminating to true right before starting the termination process - Change PlayerCore.syncUI to do nothing if mpv is terminating - Change MPVController.mpvQuit to remove observers MPVController registered before sending the quit command to mpv - Change many MPVController methods to check to see if mpv is being terminated
The commit changes the order of acquiring locks in both of the ViewLayer draw methods to first lock the OpenGL context before locking uninitLock.
This commit will add locking of the OpenGL context to the VideoView.uninit method in order to match the order of lock acquisition the ViewLayer.draw methods use.
This was also reported against IINA in issue iina#3596 The commit in the pull request will: - Rename the PlayerCore property isMpvTerminated to isMpvTerminating to reflect that termination has been initiated and being processed asynchronously - Change existing references to isMpvTerminated to use new name - Change PlayerCore.terminateMPV to set isMpvTerminating to true right before starting the termination process - Change PlayerCore.syncUI to do nothing if mpv is terminating - Change MPVController.mpvQuit to remove observers MPVController registered before sending the quit command to mpv - Change many MPVController methods to check to see if mpv is being terminated
The commit changes the order of acquiring locks in both of the ViewLayer draw methods to first lock the OpenGL context before locking uninitLock.
This commit will add locking of the OpenGL context to the VideoView.uninit method in order to match the order of lock acquisition the ViewLayer.draw methods use.
This was also reported against IINA in issue iina#3596 The commit in the pull request will: - Rename the PlayerCore property isMpvTerminated to isMpvTerminating to reflect that termination has been initiated and being processed asynchronously - Change existing references to isMpvTerminated to use new name - Change PlayerCore.terminateMPV to set isMpvTerminating to true right before starting the termination process - Change PlayerCore.syncUI to do nothing if mpv is terminating - Change MPVController.mpvQuit to remove observers MPVController registered before sending the quit command to mpv - Change many MPVController methods to check to see if mpv is being terminated
Resolved merge conflicts. The commit changes the order of acquiring locks in both of the ViewLayer draw methods to first lock the OpenGL context before locking uninitLock.
This was also reported against IINA in issue iina#3596 The commit in the pull request will: - Rename the PlayerCore property isMpvTerminated to isMpvTerminating to reflect that termination has been initiated and being processed asynchronously - Change existing references to isMpvTerminated to use new name - Change PlayerCore.terminateMPV to set isMpvTerminating to true right before starting the termination process - Change PlayerCore.syncUI to do nothing if mpv is terminating - Change MPVController.mpvQuit to remove observers MPVController registered before sending the quit command to mpv - Change many MPVController methods to check to see if mpv is being terminated
Resolved merge conflicts. The commit changes the order of acquiring locks in both of the ViewLayer draw methods to first lock the OpenGL context before locking uninitLock.
This was also reported against IINA in issue iina#3596 The commit in the pull request will: - Rename the PlayerCore property isMpvTerminated to isMpvTerminating to reflect that termination has been initiated and being processed asynchronously - Change existing references to isMpvTerminated to use new name - Change PlayerCore.terminateMPV to set isMpvTerminating to true right before starting the termination process - Change PlayerCore.syncUI to do nothing if mpv is terminating - Change MPVController.mpvQuit to remove observers MPVController registered before sending the quit command to mpv - Change many MPVController methods to check to see if mpv is being terminated
Resolved merge conflicts. The commit changes the order of acquiring locks in both of the ViewLayer draw methods to first lock the OpenGL context before locking uninitLock.
This was also reported against IINA in issue iina#3596 The commit in the pull request will: - Rename the PlayerCore property isMpvTerminated to isMpvTerminating to reflect that termination has been initiated and being processed asynchronously - Change existing references to isMpvTerminated to use new name - Change PlayerCore.terminateMPV to set isMpvTerminating to true right before starting the termination process - Change PlayerCore.syncUI to do nothing if mpv is terminating - Change MPVController.mpvQuit to remove observers MPVController registered before sending the quit command to mpv - Change many MPVController methods to check to see if mpv is being terminated
Resolved merge conflicts. The commit changes the order of acquiring locks in both of the ViewLayer draw methods to first lock the OpenGL context before locking uninitLock.
This was also reported against IINA in issue iina#3596 The commit in the pull request will: - Rename the PlayerCore property isMpvTerminated to isMpvTerminating to reflect that termination has been initiated and being processed asynchronously - Change existing references to isMpvTerminated to use new name - Change PlayerCore.terminateMPV to set isMpvTerminating to true right before starting the termination process - Change PlayerCore.syncUI to do nothing if mpv is terminating - Change MPVController.mpvQuit to remove observers MPVController registered before sending the quit command to mpv - Change many MPVController methods to check to see if mpv is being terminated
Resolved merge conflicts. The commit changes the order of acquiring locks in both of the ViewLayer draw methods to first lock the OpenGL context before locking uninitLock.
This was also reported against IINA in issue iina#3596 The commit in the pull request will: - Rename the PlayerCore property isMpvTerminated to isMpvTerminating to reflect that termination has been initiated and being processed asynchronously - Change existing references to isMpvTerminated to use new name - Change PlayerCore.terminateMPV to set isMpvTerminating to true right before starting the termination process - Change PlayerCore.syncUI to do nothing if mpv is terminating - Change MPVController.mpvQuit to remove observers MPVController registered before sending the quit command to mpv - Change many MPVController methods to check to see if mpv is being terminated
Resolved merge conflicts. The commit changes the order of acquiring locks in both of the ViewLayer draw methods to first lock the OpenGL context before locking uninitLock.
This was also reported against IINA in issue iina#3596 The commit in the pull request will: - Rename the PlayerCore property isMpvTerminated to isMpvTerminating to reflect that termination has been initiated and being processed asynchronously - Change existing references to isMpvTerminated to use new name - Change PlayerCore.terminateMPV to set isMpvTerminating to true right before starting the termination process - Change PlayerCore.syncUI to do nothing if mpv is terminating - Change MPVController.mpvQuit to remove observers MPVController registered before sending the quit command to mpv - Change many MPVController methods to check to see if mpv is being terminated
Resolved merge conflicts. The commit changes the order of acquiring locks in both of the ViewLayer draw methods to first lock the OpenGL context before locking uninitLock.
This was also reported against IINA in issue iina#3596 The commit in the pull request will: - Rename the PlayerCore property isMpvTerminated to isMpvTerminating to reflect that termination has been initiated and being processed asynchronously - Change existing references to isMpvTerminated to use new name - Change PlayerCore.terminateMPV to set isMpvTerminating to true right before starting the termination process - Change PlayerCore.syncUI to do nothing if mpv is terminating - Change MPVController.mpvQuit to remove observers MPVController registered before sending the quit command to mpv - Change many MPVController methods to check to see if mpv is being terminated
Resolved merge conflicts. The commit changes the order of acquiring locks in both of the ViewLayer draw methods to first lock the OpenGL context before locking uninitLock.
This was also reported against IINA in issue iina#3596 The commit in the pull request will: - Rename the PlayerCore property isMpvTerminated to isMpvTerminating to reflect that termination has been initiated and being processed asynchronously - Change existing references to isMpvTerminated to use new name - Change PlayerCore.terminateMPV to set isMpvTerminating to true right before starting the termination process - Change PlayerCore.syncUI to do nothing if mpv is terminating - Change MPVController.mpvQuit to remove observers MPVController registered before sending the quit command to mpv - Change many MPVController methods to check to see if mpv is being terminated
Resolved merge conflicts. The commit changes the order of acquiring locks in both of the ViewLayer draw methods to first lock the OpenGL context before locking uninitLock.
This was also reported against IINA in issue iina#3596 The commit in the pull request will: - Rename the PlayerCore property isMpvTerminated to isMpvTerminating to reflect that termination has been initiated and being processed asynchronously - Change existing references to isMpvTerminated to use new name - Change PlayerCore.terminateMPV to set isMpvTerminating to true right before starting the termination process - Change PlayerCore.syncUI to do nothing if mpv is terminating - Change MPVController.mpvQuit to remove observers MPVController registered before sending the quit command to mpv - Change many MPVController methods to check to see if mpv is being terminated
Resolved merge conflicts. The commit changes the order of acquiring locks in both of the ViewLayer draw methods to first lock the OpenGL context before locking uninitLock.
When watching a video if video playback is fast (2x/4x) and then paused/rewinded and upaused. The frame freezes and and on completion of playback Iina crashes
IINA-2021-12-28-174354.ips.txt
PS : title bar issues when fullscreen to minimised/not fullscreen
The text was updated successfully, but these errors were encountered: