MediaArea / MediaInfo

Convenient unified display of the most relevant technical and tag data for video and audio files.
https://MediaArea.net/MediaInfo
BSD 2-Clause "Simplified" License
1.26k stars 149 forks source link

24.05 Windows GUI broken on < Windows 10 Version 1803 #858

Closed nomakewan closed 3 weeks ago

nomakewan commented 4 weeks ago

As of the latest update, MediaInfo no longer opens successfully on any version of Windows prior to Windows 10 Version 1803. This is because the API call to get the DPI info (added in #835 ) was only introduced into the Windows kernel as of that version (see here).

On versions of Windows prior to this, the installer will work just fine, but attempting to run MediaInfo itself will result in the following error:

mediainfoerror

The release page still says that MediaInfo supports Vista, 7, and 8 but as of 24.05 that is not the case. Of course the easiest solution would be to simply change the documentation to clearly state that versions of Windows prior to Windows 10 Version 1803 are no longer supported, but I thought it prudent to bring this to your attention.

Thank you!

JeromeMartinez commented 4 weeks ago

Argh, we checked on Win7 that it was fine with Dark Mode, but we forgot to do same with High DPI support, @cjee21, do you have an idea about how to have HigH DPI support while keeping support of older Windows?

cjee21 commented 4 weeks ago

Argh, we checked on Win7 that it was fine with Dark Mode, but we forgot to do same with High DPI support, @cjee21, do you have an idea about how to have HigH DPI support while keeping support of older Windows?

Let me check. The high DPI support is mostly built into VCL framework, I did not implement API calls manually.

JeromeMartinez commented 4 weeks ago

I did not implement API calls manually.

In this case, it is. float DPIScale=static_cast<float>(GetSystemDpiForProcess(GetCurrentProcess()))/96;

So there is a need to check the platform version before calling it.

cjee21 commented 4 weeks ago

I did not implement API calls manually.

In this case, it is. float DPIScale=static_cast<float>(GetSystemDpiForProcess(GetCurrentProcess()))/96;

So there is a need to check the platform version before calling it.

Oh this part. The window size part.

JeromeMartinez commented 4 weeks ago

@cjee21 it is fine, I see that GetVersionEx() provides the Windows 10 version, I am adding a version check.

cjee21 commented 4 weeks ago

@cjee21 it is fine, I see that GetVersionEx() provides the Windows 10 version, I am adding a version check.

I replace it with another API that works way back to Windows 2000.

JeromeMartinez commented 4 weeks ago

I replace it with another API that works way back to Windows 2000.

Ho, great! I let you send the patch then.

cjee21 commented 4 weeks ago

Ho, great! I let you send the patch then.

Took longer because GitHub desktop auto-updated and seems to have broke commit signing. PR submitted. Need to test on various Windows versions to make sure the window sizing is set properly at different DPI settings. I also don't know what will happen if the user has multi monitors with different DPIs.

JeromeMartinez commented 4 weeks ago

Need to test on various Windows versions to make sure the window sizing is set properly at different DPI settings.

We don't have such HW so we rely on you on that (and users feedback).

I also don't know what will happen if the user has multi monitors with different DPIs.

It was the case also before, isn't it? Anyway, this is already a good first step and we'll see if/when someone has an issue with that.

cjee21 commented 4 weeks ago

I also don't know what will happen if the user has multi monitors with different DPIs.

Looks like it will use the DPI of the primary monitor. If using the newer API, it will use the DPI of the monitor that the application window is on.

JeromeMartinez commented 4 weeks ago

Looks like it will use the DPI of the primary monitor. If using the newer API, it will use the DPI of the monitor that the application window is on.

Argh. fine as a hot fix, but if you don't mind you may change the code to something like (I didn't test on Win11):

    OSVERSIONINFO osvi;
    BOOL bIsWindowsXPorLater;
    ZeroMemory(&osvi, sizeof(OSVERSIONINFO));
    osvi.dwOSVersionInfoSize = sizeof(OSVERSIONINFO);
    GetVersionEx(&osvi);
    int DPI;
    if (osvi.dwMajorVersion >= 10 && (osvi.dwMajorVersion > 10 || osvi.dwMinorVersion > 0 || osvi.dwBuildNumber >= 17134))
        DPI=GetSystemDpiForProcess(GetCurrentProcess());
    else
        DPI=GetDeviceCaps(GetDC(NULL), LOGPIXELSX);
    float DPIScale=static_cast<float>(DPI)/96;
JeromeMartinez commented 4 weeks ago

@nomakewan new build, it works in Win7.

cjee21 commented 4 weeks ago

Argh. fine as a hot fix, but if you don't mind you may change the code to something like (I didn't test on Win11):

Done. Fortunately I kept the GitHub Desktop installer so I can downgrade it else would have taken longer.

nomakewan commented 4 weeks ago

@nomakewan new build, it works in Win7.

Confirmed working on my Windows 7 box. Thank you for the swift hotfix!

cjee21 commented 4 weeks ago

I didn't test on Win11

Should be fine. Windows 11 is Version 10.0.22631.3593 right now so if the check works fine on Windows 10, it should work on Windows 11 too.