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

Qt GUI Improvements #876

Closed cjee21 closed 1 week ago

cjee21 commented 1 week ago

A collection of improvements made while I was exploring the Qt version of MediaInfo.

Tested with Qt 6.7.2 and MSVC2022.

Windows 11 screenshots: Screenshot 2024-06-19 192245 Process Explorer showing 64-bit Qt version with DEP, ASLR, CFG and Per-monitor DPI Awareness enabled Screenshot 2024-06-19 190541 Screenshot 2024-06-19 190557 Screenshot 2024-06-19 190618

Ubuntu screenshots: Screenshot from 2024-06-19 19-10-41 Screenshot from 2024-06-19 19-10-58 Screenshot from 2024-06-19 19-11-10

Previous screenshots for comparison: https://github.com/MediaArea/MediaInfo/pull/857#issuecomment-2152643509 https://github.com/MediaArea/MediaInfo/pull/857#issuecomment-2176644649

cjee21 commented 1 week ago

I prefer to keep a coherence so to change that everywhere.

I can change for others as well but it will be untested.

About $$PWD, what is the issue without it? It seems to work well without it.

Without this I get many linker errors of things not found during the linking stage.

Beside Qt project, if you don't mind, please check, enable if needed, test and send another PR for VS2022 as well as VS2019 (we didn't yet upgraded VS on our build farm), for all repositories. Nothing mandatory if you don't want to do that, just a wish.

I'll see what I can do when I'm free.

As we are there, you may be curious about CET, and try CET option. As we don't have a CPU supporting CET, we didn't try but it would be great if it is doable.

My CPU seems supported. I'll check it out.

There is also Spectre mitigations but I'm not sure if relevant for MediaInfo. This one is known to affect performance. This one is enabled in my Windows 11 context menu PoC.

JeromeMartinez commented 1 week ago

I can change for others as well but it will be untested.

Similar update so I take the risk :).

Without this I get many linker errors of things not found during the linking stage.

Weird, Fedora uses it and seems fine, @g-maxime can you check that mediainfo-qt package still compiles on Fedora?

There is also Spectre mitigations but I'm not sure if relevant for MediaInfo. This one is known to affect performance.

IMO the "cost" of this option is higher than the benefit we may have with it, so I wouldn't add it until they find a less impacting mitigation.

cjee21 commented 1 week ago

Weird, Fedora uses it and seems fine, @g-maxime can you check that mediainfo-qt package still compiles on Fedora?

Only affects Windows MSVC as far as I know. It built fine on Ubuntu without any changes.

JeromeMartinez commented 1 week ago

Only affects Windows MSVC as far as I know. It built fine on Ubuntu without any changes.

Oops, true, so @g-maxime no need to test.

cjee21 commented 1 week ago

It works! :D Screenshot 2024-06-19 212106 Tested on AMD CPU.

cjee21 commented 1 week ago

Beside Qt project, if you don't mind, please check, enable if needed, test and send another PR for VS2022 as well as VS2019 (we didn't yet upgraded VS on our build farm), for all repositories. Nothing mandatory if you don't want to do that, just a wish.

PRs created for MediaInfoLib, ZenLib and zlib.

Also for MediaInfo CLI. There are probably other MSVC Projects which I did not touch because I do not know their purpose.

cjee21 commented 1 week ago

FYI The HTML view in Qt version already has proper dark mode support on Windows.

Screenshot 2024-06-20 124639

EDIT: This is before the HTML patch for MediaInfoLib. It has extra cells on the first row. Can't find why it displays so many borders.

cjee21 commented 1 week ago

PRs created for MediaInfoLib, ZenLib and zlib.

Also for MediaInfo CLI. There are probably other MSVC Projects which I did not touch because I do not know their purpose.

@JeromeMartinez Confirmed that most recent dev build has CFG and CET enabled for MediaInfo DLLs and CLI Screenshot 2024-06-20 230358 Screenshot 2024-06-20 230420 Screenshot 2024-06-20 230500 Screenshot 2024-06-20 230620