Support automatic screen refresh rate to match video in fullscreen mode à la Kodi #3414
Comments
|
I imagine you mean when IINA is in legacy fullscreen mode -- or is it possible for IINA to change macOS' refresh rate in other modes? As an aside I'm having trouble seeing how IINA and Kodi overlap so much as to popularize one as a replacement for the other for HTPCs just by adding this. It has no 3 m interface and no library. |
|
Well, I see what you mean, it's just that I use(d) Kodi primarily because of this feature, not because of its library features :) The display refresh change feature is the most important feature to me, and I assume others looking to use a box as a media playback device (which I dubbed "HTPC" here, a little incorrectly). As it turns out, having spoken to a member of team Kodi, this feature no longer works on Big Sur in Kodi, and neither do they support HDR, so you're already one-upping them there :D If IINA supported auto-refresh rate changing to match the video, that'd be a massive thing for people like me who don't want to watch content with stuttering issues, even if they're minute :) Thanks for replying! :) |
|
I too look for refresh rate matching as my number one feature. All other features are second to displaying the content in its native refresh rate. 23.976Hz for example... almost no set top boxes can do this today (NVidia Shield I believe can do it, and it's coming soon to Apple TV 4K), and the only player I've seen that will change the refresh rate for my is MPC (Windows-only). In Windows I can control the refresh rate via NVidia Control Panel, but it's pretty tedious. In macOS, I don't think there's any working method of adjusting it at the moment. |
|
@NickStrupat — in Windows, Kodi should do this perfectly. There are two settings for this; changing refresh to match video, and a secondary one that is match video playback to screen refresh. The latter is used if it cannot precisely match output refresh with the media content, e.g. 23.976 fps on 24 Hz output, but it does require resampling of the audio which means it cannot do DTS/DD passthrough at the same time. |
Allow to switch to a matching refresh rate (if there is any) when the player goes fullscreen. This can eliminate stuttering, and, on some external displays, enable frame interpolation. Bug: iina#3414
Allow to switch to a matching refresh rate (if there is any) when the player goes fullscreen. This can eliminate stuttering, and, on some external displays, enable frame interpolation. Bug: iina#3414
Allow to switch to a matching refresh rate (if there is any) when the player goes fullscreen. This can eliminate stuttering, and, on some external displays, enable frame interpolation. Bug: iina#3414
Allow to switch to a matching refresh rate (if there is any) when the player goes fullscreen. This can eliminate stuttering, and, on some external displays, enable frame interpolation. Bug: iina#3414
Allow to switch to a matching refresh rate (if there is any) when the player goes fullscreen. This can eliminate stuttering, and, on some external displays, enable frame interpolation. Bug: iina#3414
Allow to switch to a matching refresh rate (if there is any) when the player goes fullscreen. This can eliminate stuttering, and, on some external displays, enable frame interpolation. Bug: iina#3414
Allow to switch to a matching refresh rate (if there is any) when the player goes fullscreen. This can eliminate stuttering, and, on some external displays, enable frame interpolation. Bug: iina#3414
ConcernsSome concerns from another IINA user on this feature proposal and the current proposed changes to implement it. This seems to me like a useful feature that should be added. Therefore it is deserving of a comprehensive intense detailed critical review. Commit e4681bb is a GREAT start, but I have a few concerns... This list of concerns may make it seem like a lot new code is needed, but now that the "core" of this feature has been implemented many of these concerns are easy to address. Of course the first step is to nail down exactly how this feature should behave. I'm new to IINA and not an audio/video expert, so if anyone reading this thinks I'm wrong about something, speak up! Experts please correct my misunderstandings! Since for many people this post will fall into the To Long Didn't Read category, here is a quick summary of some of the issues I'm concerned about:
Full list below. @jesec Still around and willing to tweak your pull request? Contributing Guidelines on Playback FeaturesIINA's Contributing guidelines say:
This is definitely a playback related feature. Need to justify why IINA should make an exception for this feature. From the mpv wiki post Display synchronization, initially authored by Niklas Haas, @haasn and last edited by Valerii Zapodovnikov, @ValZapod:
Seems mpv is relinquishing responsibility for adjusting the display refresh rate to the Lua script xrandr.lua, coded by Lutz Vieweg, @lvml. Further confirmation is that this mpv issue requesting such a feature was closed citing the availability of scripts: Implement changing refresh rates based on the video's framerate #552 From the mpv-plugin-xrandr script's home page:
The cited script does not support macOS. 😡 The mpv wiki User Scripts page also lists these scripts: No Digging around some more I found change-refresh. That script is only for Windows. No luck finding a mpv Lua script for macOS. 😡 I'm thinking this justifies adding this feature to IINA. Will have to see if the IINA developers agree or if they think this feature needs to be implemented as a mpv Lua script. Next File in PlaylistWhat happens if IINA is in full screen mode and it finishes playing the current video and proceeds to play the next video in the playlist and that video has a different FPS potentially requiring a different display mode? The code in commit e4681bb does not detect this. The user must leave full screen and then enter it again to get IINA to reassess the display mode selection. This IINA feature should automatically adjust the display mode in this case. Monitor ChangesWhat happens if IINA is in full screen mode and the user unplugs the external monitor the video is playing on causing playback to switch to another display? Or the user brings up the control panel and mirrors to another display using AirPlay? This IINA feature should automatically adjust the display mode if the monitor changes. Preference ChangesWhat happens if IINA is in full screen mode and the user selects Manual ActivationIn addition to the automatic refresh rate matching provided by the proposed new IINA preference should there be a way to invoke this feature manually through a menu item, along with a shortcut key? Something under the Tiny OSC, OSD and macOS Menu BarWith my MacBookPro18,2 attached to a 27" Apple / LG 5k monitor after the transition to full screen mode all UI elements are tiny and unreadable. The IINA OSC and OSD and the macOS menu bar are the most obvious, but all of the UI is affected. The screen's backingScaleFactor as changed from 2.0 to 1.0. Some calls to the logger: Logging Code:Logger.log("Current display mode: \(userDisplayMode!.width)x\(userDisplayMode!.height) @ \(userDisplayMode!.refreshRate) Hz \(String(format:"0x%08X", userDisplayMode!.ioFlags))\(userDisplayMode!.isNative ? " Native" : "")", subsystem: displaySubsystem)
displayModes.forEach { displayMode in
Logger.log("Display mode: \(displayMode.width)x\(displayMode.height) @ \(displayMode.refreshRate) Hz \(String(format:"0x%08X", displayMode.ioFlags))\(displayMode.isNative ? " Native" : "")", subsystem: displaySubsystem)
}Shows the all the display modes available on the LG 5K: IINA Log:The feature changed the display from: To: The code is ignoring all display modes that do not have the userDisplay = videoView.currentDisplay
userDisplayMode = CGDisplayCopyDisplayMode(userDisplay!)But when all display modes are obtained using CGDisplayCopyAllDisplayModes(::) that mode is not marked as native: I'm confused about what is happening with the This comment from this stackoverflow post How to programmatically determine native pixel resolution of Retina MacBook Pro screen on OS X? says:
If true that is odd. Can this flag be trusted? If this feature really needs to change the display mode to one that results in tiny UI elements is there a way to then scale up the UI? Changing More Than The Display Refresh RateIn addition to changing the display mode the code in commit e4681bb also sets the mpv option The mpv manual Options / Video / override-display-fps says:
Worries me that the other scripts are not doing this. Worries me that mpv cautions against setting this. And yet the code just set the display mode and knows the refresh rate so this seems correct. Need to hear what the experts think. Rate Selection Algorithm Differs from mpv Lua ScriptThe rate selection algorithm in commit e4681bb differs from the algorithm used by the mpv-plugin-xrandr script. The script initially looks for a "perfect" match (0.002) and if it does not find a match then falls back to a "less precise" match (0.2). If that does not find a match it selects the highest available frame rate. The change-refresh script is taking into account the resolution in addition to the refresh rate. What should IINA be doing? Is there any best practice in the video world for aligning monitor refresh rates with video FPS that should be followed? Video Playing When Refresh Rate ChangedCommit e4681bb is changing the display refresh rate while playback continues. This is triggering audio / video desynchronization as seen in this clip from the IINA log file: Shouldn't IINA pause playback while changing the display mode? Missing OSD NotificationCommit e4681bb does not contain code to notify the user of this feature's actions using IINA's On Screen Display system. Maybe something like?: And: And maybe a message when the feature can't find an appropriate rate? The OSD message would have to be displayed after the transition to full screen is completed (and back) for the message to be visible. Missing LoggingCommit e4681bb does not contain any calls to the logger whatsoever. This feature will encounter all kinds of different displays. It is important that given an IINA log file we can determine what action this feature took and why it decided that was the correct action to perform. Such reporting must be a requirement for this feature. As an experiment I quickly added this log related code in the appropriate places to generate the log messages shown from the tests I ran below: Logging Code:class RateFormatter: NumberFormatter {
override init() {
super.init()
maximumFractionDigits = 3
numberStyle = .decimal
roundingMode = .down
}
required init?(coder: NSCoder) {
fatalError("init(coder:) has not been implemented")
}
func string(for rate: Double) -> String {
super.string(for: rate)! + " Hz"
}
}
let displaySubsystem = Logger.Subsystem(rawValue: "display")
let rateFormatter = RateFormatter()
Logger.log("Current display refresh rate: \(userDisplayMode!.width)x\(userDisplayMode!.height) @ \(rateFormatter.string(for: userDisplayMode!.refreshRate))", subsystem: displaySubsystem)
Logger.log("Available native display refresh rates: \(displayModes.filter{$0.isNative}.map{rateFormatter.string(for: $0.refreshRate)}.joined(separator: ", "))", subsystem: displaySubsystem)
Logger.log("Searching for display refresh rate matching: \(rateFormatter.string(for: refreshRate))", subsystem: displaySubsystem)
Logger.log("Container FPS is \(rateFormatter.string(for: videoFps)), the best fitting display refresh rate is \(displayMode.width)x\(displayMode.height) @ \(rateFormatter.string(for: displayMode.refreshRate))", subsystem: displaySubsystem)
Logger.log("No suitable display refresh rate found", subsystem: displaySubsystem)
Logger.log("Restoring original display refresh rate: \(userDisplayMode!.width)x\(userDisplayMode!.height) @ \(rateFormatter.string(for: userDisplayMode!.refreshRate))", subsystem: displaySubsystem)Take this as just a quick example of some of the logging that is needed. Defective Rate SelectionWith the added logging code, running on one of the new 16" MacBook Pros with the M1 chip the IINA log shows: The code picked 120 Hz. I don't think that was the intended behavior. This code is suspect: if ((displayMode.ioFlags & UInt32(kDisplayModeNativeFlag) != 0) &&
displayMode.refreshRate.distance(to: refreshRate) < 0.02) {Distances can be negative. Tweaking the code to properly handle negative distances: if ((displayMode.ioFlags & UInt32(kDisplayModeNativeFlag) != 0) &&
abs(displayMode.refreshRate.distance(to: refreshRate)) < 0.02) {And rerunning the test, the code picks 60 Hz: That statement definitely needs to be fixed. Result Code Not CheckedThe method CGDisplaySetDisplayMode(::_:) returns a CGError result code. Code in commit e4681bb is not checking the result code. IINA recently had a case where a macOS failure triggered a problem that was very hard to diagnose due to failing to log the error code returned by macOS. By adding this code: CGError Extension:extension CGError {
/// Validate that this Core Graphics result code indicates the operation was completed successfully.
///
/// If this return code indicates the operation failed then this is deemed a fatal internal error and this method will:
/// - Log an error message.
/// - Show the user an alert.
/// - Terminate the application when the user dismisses the alert.
/// - Parameter fileID: The name of the file and module in which the operation was invoked.
/// - Parameter line: The line number on which the method was called.
/// - Parameter method: The name of the method that returned the result code.
func mustSucceed(_ fileID: String, _ line: Int, _ method: String) {
guard self != .success else { return }
DispatchQueue.main.async {
Logger.fatal("\(fileID):\(line) \(method) error: \(self)")
}
}
}Calls to CGDisplaySetDisplayMode(userDisplay!, displayMode, nil).mustSucceed(#fileID, #line, "CGDisplaySetDisplayMode")If the operation failed the user would be shown an alert with a message similar to: Saved Display Not ClearedThe code saves the current display mode in Contributing Guidelines on mpv APIsThe commit adds calls to mpv APIs to player.mpv.setDouble(MPVOption.Video.displayFps, displayMode.refreshRate)IINA's Contributing guidelines say:
That would seem to prohibit calls to Option display-fps DeprecatedFrom the mpv manual Options / Video / display-fps:
A new commit should not use deprecated APIs unless there is a good reason for doing so. Using this option will generate a warning from mpv. See issue #3464 for details. The code should be using the
Thoughts? |
Allow to switch to a matching refresh rate (if there is any) when the player goes fullscreen. This can eliminate stuttering, and, on some external displays, enable frame interpolation. Bug: iina#3414
Allow to switch to a matching refresh rate (if there is any) when the player goes fullscreen. This can eliminate stuttering, and, on some external displays, enable frame interpolation. Bug: iina#3414
Allow to switch to a matching refresh rate (if there is any) when the player goes fullscreen. This can eliminate stuttering, and, on some external displays, enable frame interpolation. Bug: iina#3414
Allow to switch to a matching refresh rate (if there is any) when the player goes fullscreen. This can eliminate stuttering, and, on some external displays, enable frame interpolation. Bug: iina#3414
Allow to switch to a matching refresh rate (if there is any) when the player goes fullscreen. This can eliminate stuttering, and, on some external displays, enable frame interpolation. Bug: iina#3414
|
@jesec I've cherry picked your commit from PR #3527 and will be adding additional commits to address some of the concerns mentioned above such as needing to reassess display mode when moving to the next file in the playlist. Everyone else following this issue, I would be glad to hear your thoughts on the concerns listed above and how you think this feature should operate. |
|
Great writeup. Indeed, we can add the core functionality first and then incrementally make the feature better by handling more situations and edge cases. Lua script is not going to work out for us. We need access to macOS APIs, and Lua just can't do that well. It is better to do it in IINA. Plus, we have more context and control over the whole experience in the IINA space, and that could make it possible to handle more complicated cases, and allow more customizations. macOS API has many caveats. You can't trust the returned resolution. It is not going to be real if the user uses HiDPI scaling. You can't set it either, or the actual resolution could be degraded to the stated one. The behavior is inconsistent and pretty much never what you want. So focus on the refresh rate only and ignore the resolution caveats. That's why the code only uses the native resolution. The flag is relatively reliable (verified on new machines and external displays) and definitely better than degrading the user's awesome 6K display to some 2K s**t. As for the negative result of
edit: I found a way to deal with the HiDPI issue. Check out the latest revision: jesec/iina@a928ef5 . Instead of using the |
|
Hi Jesse! I cherry picked your commit and was planning on adding my changes as a second commit and posting that in a PR. I'm pretty close to having that ready. You caught me testing edge cases. For example, if you are in full screen mode and the display's refresh rate has been changed and you switch to another space, the refresh rate is restored. I did not change the core algorithm, other than add abs around the distance check. I did switch to a menu item with a key shortcut instead of the control in the preference window. The menu item is tied to the preference you coded. It is a global control, rather than per player window. I remember the days where a refresh rate change might cause the display to malfunction and all you get is a black screen. So I made this change to make it really easy to turn this feature on and off incase a user is sometimes using a problematic display. On my concern about this causing all the UI elements to be tiny on retina displays. I too didn't find an easy way to get macOS to scale up all of the UI elements. Looked like an app might be able to do that for the UI elements it displays. But it sounded like the menu would still be tiny. I didn't have time to really dig into this. This is not a critical concern. The expectation is that you are in full screen watching the video and want the best picture. With this now being controlled by a menu item you can quickly turn it off if you want to extensively interact with the menus or OSC. I been assuming the requirement for only using native resolution was to have mpv do all the scaling and avoid a second scaling by the display. That makes sense, although I was wondering if the display is doing the straight forward 2x retina scaling would a second scaling by the display really need to be avoided? So I'm unsure if we should change the algorithm over my concerns about the tiny UI elements. On the macOS APIs. I recently upgraded my development machine to a MacBookPro18,2, one of the new 16" MacBook Pros running macOS 12.3.1. The APIs seem to be working on that machine. But testing on my older MacBook Air running macOS 10.15.7 CGDisplayMode.refreshRate was always zero. Same thing testing on a Big Sur VM. So I have conditionalized the code to require Monterey. Did you find it working in some cases on earlier versions of macOS? I was in the middle of making changes. I will finish up those changes and post a PR so you can see what I have been doing. If we decide to go with the latest changes to the algorithm I can update that PR. I'm hoping you have time to review these changes. Brutal mean review comments welcome! |
Allow to switch to a matching refresh rate (if there is any) when the player goes fullscreen. This can eliminate stuttering, and, on some external displays, enable frame interpolation. Bug: iina#3414
This commit builds on top of the core code developed by @jesec to add support for detecting additional cases where the display needs to be adjusted, notifying the user using the OSD and logging the decisions made by the feature. The original code has been refactored out of MainWindowController and into a new DisplaySynchronizer class. That class contains the core code that manages a display a selects an appropriate refresh rate. That class is used by a new RefreshRateMatcher class that handles the higher level aspects of the feature. Control of the feature has been moved from IINA Preferences to a menu item in the Video menu. This provides a way to quickly turn the feature off if a display proves problematic. IINA must be running under macOS 12 or above for this feature to be available. Older versions of macOS return zero for a display's refresh rate preventing this feature from working. When running on an older version of macOS the menu item will be hidden. The commits in the pull request will: - Add a new item "Match Refresh Rate" to video menu - Bind the menu item to the ctrl+meta+r key - Add a new class DisplaySynchronizer to control displays - Add a new class RefreshRateMatcher to control the feature - Change MainWindowController to use RefreshRateMatcher - Add a new OSD message to show the display refresh rate - Change PlayerCore to support short internal pause/resume cycles - Add the ability to suppress pause/resume OSD messages
|
I've posted the code I have been working on in PR #3745. It currently uses your original rate matching algorithm. I want to make sure you think moving away from native resolution is a good idea before changing the algorithm. I would not be surprised if there are problems lurking in the new code. I'm worried about the asynchronous aspects. I saw some odd behaviour where the window's size was not adjusted when going into full screen mode and changing the display mode. Very hard to reproduce. Haven't seen it for a while. I'll say more later. Currently starving. Need to fix dinner. |
After taking a closer look at the returned list of display modes, I am certain that the macOS uses pseudo resolutions for HiDPI scaling purposes, and we can preserve the HiDPI scaling (thus avoiding small OSD issue) by matching all four fields (pseudo height/width and actual aka pixel height/width). So far, the new logic works flawlessly on my machine, and I think it is better than matching the Your addition looks good to me in general. However:
Also, the original commit (100-ish line additions) already works well even in complicated cases (mirrored, extended, plugging out external display while playing, etc.). The only case that's missing is to handle the next file while in the full screen. I think you might want to review the design to avoid potential over-engineering (with 9x code additions), which could increase the maintenance burden and could harbor more bugs. [1]: https://developer.apple.com/documentation/coregraphics/cgdisplaymode/1454661-refreshrate |
|
Excellent review comments. The issues you listed are ones I too am concerned about with these changes.
I had that same thought as well. The file header for the new files was generated by Xcode. It is the same header found in other IINA sources.
I think the issue is this from the documentation of the
I don't have enough hardware and Macs to really test this feature. What I can say for sure is that for the same external display and Apple TV, Catalina returned a refresh rate of 0 for everything I could test. Monterey returned the expected values. I tested in a Big Sur VM and it was returning 0 as well, but that could be VM related. Do you have the ability to test with something other than Monterey?
I am very unsure if this change is needed. It used to be a common problem with CRTs that you would get a black screen. I wanted to make it easier to turn the feature on and off if problematic displays were encountered. Maybe better to start with the control in preferences and only consider switching if problematic devices are encountered?
The feature is sensitive to two settings:
With full animations enabled and playing 4k videos mpv would fairly often report desynchronization during the transition to and from full screen mode. And this is on a 16" MacBook Pro with the M1 Max chip. I really think this feature should be trying to avoid triggering A/V desynchronization. I am particularly worried about the logic that suppresses the OSD messages during pause/resume. A bunch of that logic could be avoided by creating a OSD system that could deal with multiple OSD messages being posted at once.
Yes, that logic is tied to using native resolutions.Busy today, so I may not have the time to have a look at the new algorithm until tomorrow.
I am very concerned about the added complexity. I can go back and test the original code, but I did not find it automatically handing these situations. I was expecting that IINA should not require the user to leave full screen and reenter in these cases to get the feature to synchronize the display. When playing in full screen, if the user switches to another space for a moment, shouldn't IINA restore and apply display changes as the space IINA is becomes inactive/active? When playing in full screen on an external monitor, if you unplug that monitor and everything moves to the internal screen, shouldn't that screen be adjusted properly? The original code did not adjust the display in these cases. The user would have to leave full screen mode before triggering these changes. I suppose we could impose that on the user, but it takes some of the "automatic" out of this feature. Great feedback. I will need time to digest. I will try out the new algorithm. |
Let's make it right starting with this.
I'd say let's just allow it on macOS 10.6+. Also, you can check if multiple refresh rates are supported in System Preferences -> Displays. I knew that some devices simply do not support refresh rate adjustment, and I am afraid that it is not going to be much useful to greatly increase complexity to introduce logic to detect and hide the option.
If a display mode is in the returned list, it is supported. The user can just exit the fullscreen and restore display mode with the Escape key if something went wrong.
The whole feature simply won't work with the legacy full-screen mode, and we probably should grey it out (with some tips perhaps) if the legacy full-screen mode is on. Reduced motion is about animation, and I didn't find it to interfere. Note that, eventually (once the transition is completed), there is no desync. Any glitch happens during the transition. So I am not worried about the desync. It is just about if the user is willing to miss a few seconds of content (when the video is playing while switching the display mode). IMO that's mostly fine if they choose to switch when the video is playing.
Indeed.
Makes sense. |
|
I will be closing my PR due to several of the issues you raised. Also because now that you are back I feel like I am trespassing on your issue. Do you want to take over this one? If so you are welcome to use anything you find useful in my commit. I had a chance to test your new commit and think some more on the issues. Here are some thoughts plus a few questions... The idea of a menu item goes into the trash heap. Stay with the setting in Preferences/Video. On this:
I listed those because they both made desynchronization less likely as they lowered the amount of animation being performed during the transition to full screen. I'm not understanding why the feature won't work with legacy full screen mode?
I was not able to find an authoritative statement on the mpv site as to how bad triggering desynchronization is. The mpv log message sure makes it seem bad. I read somewhere that mpv sometimes had trouble recovering from severe desynchronization and you would have to pause and resume to recover. That may have been a defect that has since been fixed. I tested today while inspecting the "avsync" property and mpv always quickly recovered. I did get some irritating pops in the audio. The change in the algorithm is also making this much less likely as the transition to full screen is not as disruptive as it was when it included a resolution change. You have sold me on not pausing. On this:
Yes, I was already worried about the complexity and after your review that code going into the trash. Thank you for pointing out issues! But on this:
That depends upon how we think this feature should operate with "edge" cases. I was taking the stance that if IINA is playing in full screen mode with the feature enabled and it is possible to rate match then the display should always be properly adjusted. When not playing in full screen mode then the display mode must be restored. This was particularly important with the original matching algorithm. Here is one example of what I feel is unacceptable behavior: You are playing the video in full screen mode and the feature has appropriately adjusted the display mode. In a second space you have a chat program running. You hit control and an arrow key to quickly peek at the chat program. With the original code everything in that space is tiny because IINA did not restore the display when the space that the IINA full screen window is in became inactive. That means you have to go back to the original space and take IINA out of full screen mode before going back to the other space to check on chat, email, whatever. And once you get back to the space with IINA you have to put it back into full screen mode. Quickly checking on the other space has been disrupted. A less severe example happens when playing in full screen on an external display. When the display is disconnected IINA will automatically be moved to the internal display. Now IINA is in full screen mode with the feature enabled, but the display has not been adjusted. Not as big of a deal as ending up with a tiny UI as with the spaces issue. But should the feature make you exit and re-enter full screen mode in this case? Possibly a more important problem is that when you exit full screen IINA will try and "restore" the display mode that came from the external display. I would expect macOS to not allow that to cause a problem, but should we be testing that? The new matching algorithm eliminates the tiny UI issue when changing spaces. That was the problem that made me decide this feature must handle such changes. Has the new algorithm made the handling of these display changes a moot point? I'm now unsure. It eliminated the main problem (tiny UI in space) that I thought was unacceptable. Still telling macOS to set a display mode from a different display worries me. On the restricting the feature to macOS 12+. I can't point to anything bad happening if this restriction is eliminated. I wanted to eliminate questions from users as to why the feature wasn't working. It would be nice to know that this feature actually works on something other than Monterey. The following examples are with your new commit and some debug logging quickly hacked in. The display is the Apple/LG 5K. The Catalina example is a MacBook Air. The Monterey example is my development machine, a new MacBook Pro. CatalinaMontereySure seems like this feature is not going to work on Catalina. On logging, I really think it must be possible to tell from the log what action this feature took and why. Need to be able to answer a user's question as to why rate matching did not adjust the display. Based on previous issues users will complain if there is no OSD notification. So I do think it is desirable to post an OSD message when changing the display mode. A couple comments on the new commit... This statement: if userDisplay != nil && userDisplayMode != nil {Can be changed to use optional binding like this: if let userDisplay = userDisplay, let userDisplayMode = userDisplayMode {That establishes a constant that is not optional, eliminating the need to use the force unwrapping operator within the body of the if statement. The I'll keep thinking on this. Busy Thursday. Will have more time Friday. |
I will work on this when I have time, but feel free to continue working on this. I think the PR does provide an extensible framework to handle more complicated cases, amid the added complexity.
The legacy full-screen mode simply enlarges the window. It is probably not a good idea to change the refresh rate if the player is not the only app on the screen.
Indeed, there should be less disruption without the HiDPI scaling factor change.
Depending on your display, there is a non-trivial latency (some even need seconds) to change the display mode. Now with the new algorithm, there is no change in HiDPI scaling, so the window layout should remain the same. The user won't have difficulty using other apps, though the adjusted refresh rate could be lower, which makes things slower. The user may prefer to quickly switch out to reply to a message instead of having to wait for their display to switch to its original mode (and switch back). On the other hand, some users could prefer to see the display mode restored.
Indeed, that's a valid case. It is called with the displayId, so that won't be a problem.
Let's add that after someone actually files an issue. Also, Catalina is going to EOL soon. Have you checked the System Preferences -> Display? Is the refresh rate adjustable?
Makes sense.
It is going to be no-op unless the user changes the display mode when the player is in full-screen. I found it extremely unlikely for that to happen. If you want to handle that case, more logic has to be implemented. OSD is a valid concern, though. |
I'll let you handle it. I've gotten overloaded. If I free up and you haven't gotten to it I'll check in with you before starting on it.
It enlarges the IINA window to take up the entire screen. It is a form of "full screen mode". I'm thinking uses would want this feature to kick in regardless of whether legacy full-screen mode is used or not.
I think adjusting the refresh rate was added for Monterey. I don't see a way to adjust it in System Preferences -> Display under Catalina. I also saw zero for the refresh rate in Big Sur, although that was running in a VM which could have disrupted things. I'm good with not explicitly disabling this feature in earlier macOS versions just in case the behavior I'm seeing is somehow tied to my Macs. |
Allow to switch to a matching refresh rate (if there is any) when the player goes fullscreen. This can eliminate stuttering, and, on some external displays, enable frame interpolation. Bug: iina#3414
|
See #3527. |
|
A user has requested |
What you want IINA to do:
Switch the display resolution automatically to match the framerate of the video playing back, like Kodi does.
What IINA does currently:
Why you think this should be added:
This would make IINA a possible replacement for Kodi and useful as an HTPC option.
Examples of other projects that have something similar:
Kodi, possibly Plex?
The text was updated successfully, but these errors were encountered: