clsid2 / mpc-hc

Media Player Classic
GNU General Public License v3.0
10.95k stars 491 forks source link

Access Violation while switching between full screen and windowed #1787

Closed cospking closed 2 years ago

cospking commented 2 years ago

image image

MPC-HC: 1.9.2.2 CPU: i9-9900k GPU: 2080TI - Drivers: 516.59 OS: Windows 10 MadVR: Yes, version 0.9.2.17 KB5011831: Not installed or at least not visible in installed updates, probably merged with another update.

I can reproduce this by playing a video, putting it in full-screen and clicking the screen to have it play/stop playing while going into fullscreen and windowed mode. This is at least the way to force it by spam clicking it like that, but it unfortunately also happens when I occasionally pause/go out of fullscreen to do something else.

I just tried the same in 1.9.21.2 and 1.9.20 and it crashes as well, strangely enough I've never had this happen before. Aside from Windows Updates, the newest MPC-HC I have not changed or updated anything related such as graphics drivers or MadVR. The most recent Windows Update is from 13-07-2022: KB5015807

When I changed the Video Renderer to Video Mixing Renderer 9 (renderless) it doesn't seem to happen, but then again there is no specific set amount of clicks I have to do for it to happen, it can happen after doing it just the one time while I'm watching something but it can also happen after spam clicking for a minute or perhaps a bit longer.

Can somebody else with the same MPC-HC version + MadVR check and see if this happens as well?

clsid2 commented 2 years ago

Enable the crash reporter, and copy the DrDump link of the page that opens after a crash. Use the ZIP download from HERE.

It doesn't appear to be crashing in player code, so I doubt there is anything I can do about it.

cospking commented 2 years ago

Enable the crash reporter, and copy the DrDump link of the page that opens after a crash. Use the ZIP download from HERE.

It doesn't appear to be crashing in player code, so I doubt there is anything I can do about it.

https://drdump.com/UploadedReport.aspx?DumpID=98179366

Yeah I figure it might be something related to MadVR, but it's strange how this suddenly started happening. I wonder if other people are experiencing the same with MadVR/MPC-HC combined or if it's related to another Windows Update.

clsid2 commented 2 years ago

It is crashing in a DLL from Windows. I see no direct involvement of madvr.

The number of reported occurrences of this specific crash is low, so it is not a common problem.

cospking commented 2 years ago

It is crashing in a DLL from Windows. I see no direct involvement of madvr.

The number of reported occurrences of this specific crash is low, so it is not a common problem.

The weird thing is that if I change the renderer it doesn't seem to occur, or maybe it's just messing with my head by not occurring while having changed the renderer.

EDIT: Just tried the same with MPC-BE, it uses the same MadVR(settings as well) but it doesn't crash. This is making my head spin because it just doesn't make any sense that this started happening out of nowhere.

clsid2 commented 2 years ago

The Windows API function that crashes (SystemParametersInfoW) gets called many times in the exact same way. So it is weird that it crashes only so rarely. Do you perhaps have any software installed that modifies the Windows interface? For example a Start menu replacement. That might possibly be hooking into Windows code.

@adipose Do you have any idea what might be wrong?

Currently it re-creates the fonts each time a menu entry is redrawn. Maybe we could cache them, and only replace in case of a DPI/screen change? I dunno if this would give much improvement performance wise.

adipose commented 2 years ago

If the fonts aren't being destroyed properly, it could cause a memory leak. A static member would be better anyway.

I found this: https://github.com/dotnet/wpf/issues/3143

clsid2 commented 2 years ago

That looks like a different issue. For us it crashes with SPI_GETNONCLIENTMETRICS.

There is just 50 reports on DrDump, so probably just a rare bug in Windows.

adipose commented 2 years ago

What OS was this run on?

https://docs.microsoft.com/en-us/previous-versions//ms724506(v=vs.85)?redirectedfrom=MSDN

**If an application that is compiled for Windows Server 2008 or Windows Vista must also run on Windows Server 2003 or Windows XP/2000, use the GetVersionEx function to check the operating system version at run time and, if the application is running on Windows Server 2003 or Windows XP/2000, subtract the size of the iPaddedBorderWidth member from the cbSize member of the NONCLIENTMETRICS structure before calling the SystemParametersInfo function.

cospking commented 2 years ago

@adipose @clsid2 This is on Windows 10 image

I don't use any tools to change the Start Menu or the interface in general. I am only using a single monitor 35' at 3440x1440 resolution with HDR turned on.

Running applications(background): Firefox Discord Whatsapp ShareX (screenshot tool) MSI Afterburner Corsair ICUE

I have turned the Overlays off in these programs.

Installed Windows updates: image

Recently installed Windows updates: image image

What I wonder about is if you are able to reproduce this behaviour with the latest MPC-HC and MadVR installed, perhaps in combination with the same Settings.bin file. I use the MadVR settings from this page as he conveniently uses the same GPU: https://forum.doom9.org/showthread.php?t=171787 Link: Max Quality settings.bin

clsid2 commented 2 years ago

The crash happens outside of the player code, so even if I could reproduce, I can not fix it. I am not going to waste any time on this.

Test if it also happens if you reset madvr settings to defaults. Also test with all those background apps fully disabled/closed. Because the problem might be caused by another app that is hooking into Windows code.

adipose commented 2 years ago

https://mega.nz/file/MZQ2hKjZ#Jad9pIvf0xVdC6YJyEd-hGh4d6LL-QcABCTK_SR0PTg

Try this build to see if it helps.

clsid2 commented 2 years ago

@cospking, can you test the build that adipose made?

adipose commented 2 years ago

https://github.com/clsid2/mpc-hc/pull/1812

So this has two things that should help:

It doesn't relookup metrics unless SPI_SETNONCLIENTMETRICS happens, which is basically never, unless you change your default font zoom level in Windows.

It doesn't recreate menu fonts unless metrics are reset.

cospking commented 1 year ago

@clsid2 @adipose I'm not super-active on Github so forgot this was still open and only noticed the notification, I haven't noticed it happening after the update so seems to be fixed. Thanks again.