alamminsalo / orion

Cross platform Twitch.tv client
GNU General Public License v3.0
315 stars 60 forks source link

add hardware decoding options #246

Closed mrgreywater closed 5 years ago

mrgreywater commented 5 years ago

Adds the option to change the decoder of the video to the QtAV and mpv backend. On my test machine with windows 10 and a Intel i5 3570k and a AMD graphics card the performance differences are quite drastic for a normal 1080p stream:

Cpu Usage:
qtmultimedia: 25%
mpv (dxva_copy): 10%
mpv (no): 25%
QtAV (DXVA zero-copy): 3%
QtAV (FFmpeg): 8% 

Artifacts: https://ci.appveyor.com/project/mrgreywater/orion

This commit also gives an option to start multiple instances, fixes some gui issues and reorders the player controls to a more similar order as youtube etc. I can potentially split those up into more commits or PRs as necessary.

edit: there are still some issues, will push a fix for some things soon

alamminsalo commented 5 years ago

I think the work done here is good but I'd rather see each feature/fix separated incase something needs to be reverted

mrgreywater commented 5 years ago

It appears some Intel graphics have issues with the default ANGLE opengl backend, so i need to add some option for that. After adding that i'll split the PR into multiple commits.

alamminsalo commented 5 years ago

Fuck it, these changes are great. No need to split up as the list changes were minor enough.

After this merge I will tag a new release

alamminsalo commented 5 years ago

Also, I think we should think of just removing the single/multi-instance features, as nowadays OS's handle that pretty good so that in most cases there's single instance running. The current filelock-instance handling has already caused some problems where user had to manually remove the instance because permission-issues. Users may then start multiple instances as they please

mrgreywater commented 5 years ago

I would also prefer just allowing users to start multiple instances to view streams side by side. There are a few things that may (not) work correctly though:

Nothing of that is really problematic though, so just allowing multiple instances should be fine.

alamminsalo commented 5 years ago

Yeah, those are fair points.

I think traybar icon is currently really odd state as it has only features to show/hide application and quit the app. DE applicationbars most do that already and additionally can handle multiple windows too.

We would need to basically sync the settings via polling or something similar, and apply those in other clients. I'm unsure if this is going to be a big issue since settings doesn't have that drastic features.

I think we can solve the http-port problem (if I can understand the issue correctly), and in fact it would be good to know this early on (for the developers of low-latency support) so it can be taken into consideration

mrgreywater commented 5 years ago

I think this PR is ready.

There are still some unrelated issues with QtAV (see https://github.com/wang-bin/QtAV/issues/1149) which means we should pick a specific version in appveyor, and the OpenSSL in appveyor needs the msvc 2013 runtime, so it should probs be compiled in appveyor aswell.