-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Black window if video is paused and toggled out of PiP #4268
Comments
The change to shutdown the display link while the video is paused in order to not waste energy has caused a number of problems. I'm working on that issue right now. That might be the root cause of this issue. I will look into it. On |
I've found that you sometimes need to check for |
The class I would expect we need to probe what I see a some references to animation in PIPViewController.h a source from the project PiPHack, a PiP proof of concept. Possibly something can be learned from that project. There is also sliding animations in By the way, if I am non-responsive for a while it may be due to network access and/or power being cutoff by a Winter storm. Heavy wet snow is predicted starting tomorrow. |
Reproduced. |
Oh ha, didn't notice that! Nice.
I will look into this more at some point, but I never use PiP and so it's a bit low-priority for me. I don't know why it resorted to a hack. I've been playing around with MacOS animation more and more and it's ugly but it's not crazy-complicated... Regarding the pause issue, from my experience with OpenGL (many years ago) it's likely writing to a separate buffer than the other views when it does the drawing, so if you give it any chance to drop the data (like maybe when detaching it from a view, or sending it off screen) it might discard it without warning. I've been thinking it might be a good idea to basically capture a screenshot the video when it pauses and display that in a separate layer which is above the video. It's apparently standard practice in Cocoa development to do things like this. Older examples mention But so many things to work on. Hopefully you can find a simpler solution. But these redraw issues turn out to be a whack-a-mole kind of situation, it might be worth thinking about. |
AnalysisFixing this regression requires fixing two problems. First the fix in PR #4249 for issue #4055 must be applied to correct a regression in drawing. Once drawing is corrected then there is a problem with commits from PR #3973 as mentioned here that must be addressed. Adding logging showed that As a quick test I added this line to videoView.pendingRedrawsAfterEnteringPIP = 0 In branch While working on this I noticed this comment about // it seems the system doesn't call this function since macOS 10.15
- (BOOL)pipShouldClose:(PIPViewController *)pip __OSX_AVAILABLE_STARTING(__MAC_10_12,__IPHONE_NA);
// instead this is added in macOS 10.15
- (void)pipWillClose:(PIPViewController *)pip __OSX_AVAILABLE_STARTING(__MAC_10_12,__IPHONE_NA);
- (void)pipDidClose:(PIPViewController *)pip __OSX_AVAILABLE_STARTING(__MAC_10_12,__IPHONE_NA); Is incorrect. This debug logging I added as a test shows both
At least that is what is happening under macOS Ventura. Need to think on this some more as to whether clearing |
Do we have any idea why it takes 2 forced redraws after entering PiP? Such an odd number. A number like that smells like a race condition. ...Or maybe there was a race condition, and the other PRs fixed it, which means this "2 forced redraws" workaround is now breaking the code? Intuitively it seems like it would be bad for |
Those are some of the reasons why I did not post a PR with a fix and have turned by attention to fixing filter related problems. I may hand this one off to @uiryuu. |
This comment was marked as outdated.
This comment was marked as outdated.
I cannot reproduce the problem with #4249 applied. Am I missing anything? I've tried making |
I had not brought up this issue yet as I was thinking it was best to merge the pending PRs for other display related issues first and then see if this is still an issue. Let's get those changes into the |
I cannot reproduce using the latest develop build anymore. |
I've been waiting for merges before testing this again. I just pulled the latest
This test was run on my MacBookPro18,2 under macOS Ventura 13.2.1. I rebuilt it for Intel and took it to my MacBook Air running macOS Catalina. IINA acts slightly differently in that the main window does not minimize to the dock. When it transitions back from PiP the main window shows a frame for a moment and then goes black. I'm confused as to why it did not reproduce for you. |
It’s weird. I followed the exact steps you provided (except for the video, which I don't think that will make a difference), but it didn't reproduce for me. The behavior I got is when it transitioned back, the window will show a black frame for a moment and then the video frame come back. This has been existing for a long time if I remembered correctly. So this must be a race condition, where sometimes the forced redraw happens first (which will lead to your behavior), sometimes the forced redraw happens latter (which will lead to mine). |
I still see it too, 100% of the time. On my Macbook Pro M1 Pro, running MacOS 13.1, IINA Screen.Recording.2023-03-28.at.22.06.14.downscaled.movI tried toggling OSD on and off, different OSC positions, via an external display and without an external display. |
I'm experimenting with an alternative fix. At the moment looking good. Need to test some more… |
This commit will: - Add a VideoPIPViewController class that extends PIPViewController - This class will force drawing when entering and exiting PiP - Change MainWindowController to use VideoPIPViewController for the PiP controller - Remove the pendingRedrawsAfterEnteringPIP property from VideoView - Remove the layout method from VideoView - Remove code that set pendingRedrawsAfterEnteringPIP from the MainWindowController.enterPIP method
I've posted an alternative fix for the white window/black window PiP related problems. This is working for me under macOS 13.3 and 10.15.7. I'm unsure of this fix. Please test it out and try and break it. |
This commit will: - Add a VideoPIPViewController class that extends PIPViewController - This class will force drawing when entering and exiting PiP - Change MainWindowController to use VideoPIPViewController for the PiP controller - Remove the pendingRedrawsAfterEnteringPIP property from VideoView - Remove the layout method from VideoView - Remove code that set pendingRedrawsAfterEnteringPIP from the MainWindowController.enterPIP method Updated to address review comments and change VideoPIPViewController, adding a private forceDraw method that only forces drawing when required.
* Fix black window after exiting PiP, #4268 This commit will: - Add a VideoPIPViewController class that extends PIPViewController - This class will force drawing when entering and exiting PiP - Change MainWindowController to use VideoPIPViewController for the PiP controller - Remove the pendingRedrawsAfterEnteringPIP property from VideoView - Remove the layout method from VideoView - Remove code that set pendingRedrawsAfterEnteringPIP from the MainWindowController.enterPIP method Updated to address review comments and change VideoPIPViewController, adding a private forceDraw method that only forces drawing when required.
IINA 1.3.2 contains the fix for this issue. |
System and IINA version:
Steps to reproduce:
UI
>Picture-in-Picture
>Toggle Picture-in-Picture by minimizing/un-minimizing the window
. The other PiP settings don't seem to make a difference.Expected behavior:
Assuming
Accessibility
>Reduce motion
is disabled in System Settings, smooth scaling animation into and out of PiP, displaying the content of the video with no breaks.Actual behavior:
There is some flickering when coming out of PiP and the main window is displayed a bit too early, and when animation is done, the main video window is black (specifically, the VideoView appears to be hidden).
Bonus issue:
Reduce motion
does not appear to be honored when enabled.How often does this happen?
Always
The text was updated successfully, but these errors were encountered: