Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#2235 closed Patch (fixed)

outputpage rewrite

Reported by: demi_alucard Owned by: demi_alucard
Priority: normal Milestone: 1.6.3
Component: General Version: 1.6.2.4902
Severity: major Keywords: rewrite, output, renderer
Cc: Underground78, XhmikosR Evaluation:

Description

PPageOutput rewrite. A picture is worth a thousand words:
http://i.imgur.com/1zKVn.png

All comboboxes items are dynamically generated. All renderers are only shown if they are present in the system.

This should be extensivelly tested before committing.

Attachments (5)

outputpage_rewrite.zip (58.9 KB) - added by demi_alucard 5 years ago.
outputpage_rewrite_v2.zip (58.8 KB) - added by demi_alucard 5 years ago.
#2234_outputpage_rewrite_v3.zip (58.9 KB) - added by xhmikosr 5 years ago.
outputpage_rewrite_v3.rar (43.6 KB) - added by demi_alucard 5 years ago.
ouputpage_rewrite_v4.rar (48.4 KB) - added by demi_alucard 5 years ago.

Download all attachments as: .zip

Change History (28)

Changed 5 years ago by demi_alucard

comment:1 Changed 5 years ago by demi_alucard

btw, I made use of lambdas in this code. This will make mpc-hc buildable only on MSVC2010+ or ICL11+. I could probably have used macros instead, but I hate them with a passion. If this change is needed for the acceptance of this patch, feel free to change it, because I wont.

Changed 5 years ago by demi_alucard

comment:2 Changed 5 years ago by demi_alucard

Patch v2 adds Sync Video Renderer to the list, plus a few variable cleanups

comment:3 Changed 5 years ago by xhmikosr

Updated the patch for r4542.

Changed 5 years ago by xhmikosr

comment:4 Changed 5 years ago by demi_alucard

What did you change?

comment:5 Changed 5 years ago by demi_alucard

Ah, nevermind, didnt read the other comment

comment:6 Changed 5 years ago by demi_alucard

btw, janwillem32 in the channel suggest yesterday that we remove the option of Old Video Renderer since it is the same as VMR-7 Windowed on XP+. What do you think?

comment:7 Changed 5 years ago by thevbm

Old Renderer is a leftover when <XP support was removed i guess. So indeed it should be removed now.

comment:8 Changed 5 years ago by xhmikosr

Is there any way to keep the tooltips? Or show some of that info like which renderers support subtitles?

comment:9 follow-up: Changed 5 years ago by demi_alucard

I think I added that info to the combobox tooltip. But that certainly isn't as visibile as it were before. Maybe we should considerer readding the stars and putting the remarks on the bottom of the page again?

comment:10 in reply to: ↑ 9 Changed 5 years ago by maladiementale

Replying to demi_alucard:

Maybe we should considerer readding the stars and putting the remarks on the bottom of the page again?

This is too confusing. I suggest you should create a Label under the "Direct Show Video" Combo Box. There should then stay the relevant information about the current selected render.

comment:11 Changed 5 years ago by maladiementale

Is there any test build?

Changed 5 years ago by demi_alucard

comment:12 Changed 5 years ago by demi_alucard

Updated patch to r4607. Also removed the weird numbering in the audio renderer combobox.

comment:13 Changed 5 years ago by demi_alucard

Updated patch to r4650. Also added tooltips to the Video/QT/RM/Audio renderer comboboxes.

This patch should now be complete. Now we need people to test the modifications.
The removal of duplicated video renderer options and the renaming of evr-cp and vmr-7/9 renderless to "Video Renderer Name (custom)" should come in different patches after this one gets committed.

I noticed that there are quite a few renderer options without tooltips, if anyone have any ideas how I should describe them, please contact me in IRC.

Changed 5 years ago by demi_alucard

comment:14 Changed 5 years ago by demi_alucard

Did a few modifications so there is more space for translations.
http://i.imgur.com/kk59g.png
What do you think?

comment:15 Changed 5 years ago by underground78

I don't really have an opinion but it doesn't strike me as strange or bad looking at least.

Just wondering why the "YUV" checkbox is hidden completely instead of being disabled like the "VMR-9 Mixer Mode" checkbox?

comment:16 Changed 5 years ago by demi_alucard

I don't know. I only replicated the old behaviour.

comment:17 Changed 5 years ago by kasper93

I think we should discuss order in the renderer list.
My proposal is that:
(1)

Enhanced Video Renderer
Enhanced Video Renderer (custom)
Sync Video Renderer
madVR
Overlay Mixer Renderer
Video Mixing Renderer 7 w.
Video Mixing Renderer 7 r. 
Video Mixing Renderer 9 w.
Video Mixing Renderer 9 r.
Null (compressed)
Null (uncompressed)

or

(2)

Overlay Mixer Renderer
Video Mixing Renderer 7 w.
Video Mixing Renderer 7 r. 
Video Mixing Renderer 9 w.
Video Mixing Renderer 9 r.
Enhanced Video Renderer
Enhanced Video Renderer (custom)
Sync Video Renderer
madVR
Null (compressed)
Null (uncompressed)

I think more modern renderers should be on the top(1), but (2) is also good. Remove "old renderer" in both cases.

"EVR Sync Setting" in options should be renamed, because you renamed renderer on the list and it isn't EVR now, at least by name.

Also would be nice to detect (if possible) which renderer is "system default" and instead on making on the list one more item simply append "- default" to the renderer name. For example if default renderer is VMR-7 this would look like "Video Mixing Renderer 7 w. - default" and also you as developers should recommend one of the renderers for users. Not everyone need to know difference between renderers, for example "Enhanced Video Renderer (custom) - recommended".

And of course there is also MPC Audio Renderer which is simply unusable ;/ I think you should think on temporally remove it. Comment out the code before someone fix that renderer. But I think its' discussion for other ticket.

That are my thoughts of that patch, and you can have different view on this matter :)

P.S It is possible to make tooltip for each items in a combo box?

comment:18 Changed 5 years ago by demi_alucard

  • The renaming/removal of renderers is something to be discussed in another ticket.
  • Second ordering seems best to me. Maybe I'll change it.
  • No, you can't have different tooltips for each item in the combobox. The best I could do is append more info to the combobox's tooltip AFTER the user selected a renderer. IMO this is annoying for the user.

comment:19 follow-up: Changed 5 years ago by clsid2

I prefer order 2. Please maintain same registry values for the renderers for backwards compatibility.

"Overlay Mixer Renderer" -> "Overlay Mixer" (that is how it is named in Windows)

The "w." and "r." look weird imho. Why not use "(windowed)" and "(renderless)"? There is space enough. Or perhaps use same naming as for EVR (renderless = custom).

Perhaps you could add a label below the renderer that shows the capabilities of the selected renderer? Something like:
"DXVA: yes | ISR: yes | Shaders: yes | SaveImage: yes"

comment:20 in reply to: ↑ 19 Changed 5 years ago by demi_alucard

Replying to clsid2:

I prefer order 2. Please maintain same registry values for the renderers for backwards compatibility.

"Overlay Mixer Renderer" -> "Overlay Mixer" (that is how it is named in Windows)

The "w." and "r." look weird imho. Why not use "(windowed)" and "(renderless)"? There is space enough. Or perhaps use same naming as for EVR (renderless = custom).

Perhaps you could add a label below the renderer that shows the capabilities of the selected renderer? Something like:
"DXVA: yes | ISR: yes | Shaders: yes | SaveImage: yes"

To maintain compatibility is easy, we could add a translating function or just change the value of each entry of the enum in AppSettings. Take note that in the future some options like Old Renderer might be removed. The people in #MPC-HC is also considering the removal of the System Default option. The System Default option is a bit misleading. The user may think it is the recommended option, even though they aren't the optimal choices. We should mark which filter is recommended like kasper93 suggested.

I will use the full windowed/renderless name for this patch. But we are indeed considering replacing 'renderless' with '(custom)'. The same with EVR-CP. It will be simply EVR (custom).

The idea of adding a label enumerating each filter capabilities is indeed a good idea. I was thinking how to explain what each filter can do to the user without resorting to asterisks or tooltips, but I couldn't think of nothing. Thanks.

comment:21 follow-up: Changed 5 years ago by maladiementale

When you move the new outputpage into the trunk?

comment:22 in reply to: ↑ 21 Changed 5 years ago by xhmikosr

Replying to maladiementale:

When you move the new outputpage into the trunk?

In 10 years. Also spamming the trac will only make you happy and no one else since there are no people available to implement new stuff. So if you want something, provide a patch and not new feature requests/spamming.

comment:24 Changed 4 years ago by underground78

  • Component changed from New to Solved
  • Milestone changed from next release placeholder to next release
  • Resolution set to fixed
  • Status changed from new to closed
  • Version changed from 1.6.1.4235 to 1.6.2.4902

The new output change has been integrated 802a2057bb6141af644e5e732a2656f4e4d5f4e7. Some minors changes are to be commited to improve the translations with the new layout.

Note: See TracTickets for help on using tickets.