-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
AdvancedWidget: Add UTVideo codec option #13233
base: master
Are you sure you want to change the base?
Conversation
// Update EnableBitrate() if options are changed. | ||
|
||
const std::vector<std::pair<QString, QString>> options{ | ||
{tr("Auto"), QStringLiteral("")}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does the Auto
option do? It's not explained in the popup, and I personally don't think it's needed at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It sets nothing, which is currently how it works. I'm not sure of how every OS handles that. It may be safe to not have Auto.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it's set to nothing then what happens when one tries to dump frames on Auto
?
I recommend putting FFV1 last in the options (and popup) due to how slow and incompatible it is. |
I'm not certain about using High./Low instead of Good/Poor/Bad. I just want it to be clear which is better, especially when considering translations. Maybe it's a non-issue. I added more code to toggle off the old FFV1 bool option. It will only trigger when opening the options though. |
"Poor compression" to me means "really bad compression, little to none, it does poor job at it", which is far from the case for UT. It's just not as heavy as FFV1 or something like Lagarith. Which is compensated by high speed etc. Lossy is better for regular usage, UT is better for high quality encoding (it's reencoded to lossy on our end afterwards). |
What if we drop mentioning compression and just say Ut Video produces larger files? If not, I'll just go with whatever you suggest. |
I think there's potential to think larger = better quality when in reality video compression is more complex than that, but I imagine someone using the frame-dumping option in an emulator for video probably knows the distinction between the codecs. |
I think if it's changed into Oh and then for FFV1 it'd be symmetrical to use This way, compression is worded in an objective way because both are fine at it, one is just stronger than the other. But with compatibility, FFV1 is actually bad (because these days you can't even install a VFW decoder for it without breaking all other VFW codecs), therefore "poor". |
What are the use cases for FFV1, if any? Considering the difficulties in converting/editing the video after the dump, maybe drop FFV1 in favour of UTVideo? Seems a bit redundant to have multiple lossless codecs as choices... |
Honestly I agree. Maybe it's time to finally let it go indeed. I don't know of a single use case for FFV1 anymore where UT is unfeasible. |
It'd be easy to just change the FFV1 checkbox into a Ut Video checkbox (or just call it lossless). I don't know if it's okay to just assume nobody uses a FFV1 setup though, as it produces smaller files. Are there any other options that should be added? I'm guessing xvid is enough for lossy. Is there another, better lossless option? It looks like h.264 with -crf0 is lossless? Then you can change the compression rate vs speed. Or is that not compatible with TASing? |
Isn't XVID the same situation as FFV1 regarding VFW support? I don't know if the FFmpeg build we compile currently allows this as AFAICT it can quickly get the final binary very bloated if we enable all features it has, but in the GUI I'd offer one lossy option, one lossless option, and that's it. There's already an INI-only setting if more tech-savvy users want to select a specific codec, and I think it's enough (we could expose that directly in the GUI if there's enough demand, though, the existing codec options lives in the "Advanced" tab after all). Stick with either H.264 or H.265 for lossy encoding (or perhaps VP9 if we want a royalty-free codec), since any of those are supported practically everywhere nowadays, and additionally offer a lossless option (FFV1 seems bad, so perhaps UTvideo, or other codec?) |
Also, I don't actually know how to do anything besides set the string for which codec to use. |
Back in the day when the exact format of the FFV1 option was barely usable, I only know of the TAS people asking to improve its compatibility. And they (we) already largely switched to UT because decoding FFV1 is such a PITA still. It's small but it's very slow during encoding, and even at seeking (important when editing the video). The assumption is not that nobody uses FFV1 anymore but that nobody is going to specifically need it when UT is there.
x264 is very fast and compresses very well, but its seeking has big problems in VFW based video editors (like avisynth). The codec got some updates over the years that import plugins for it didn't have, so with ffmpeg's libx264 seeking is outright broken. UT's seeking is instant even for giant 4K dumps.
I haven't specifically tested it but I think XVID is much more widely supported by decoders.
Personally I'd only enable libx264 and utvideo (and let the user send string arguments to them, ideally via nut container but that seems to involve overhauling the whole dumping system). h264 is super well supported and has crazy amount of options, and likely compresses much more efficiently than xvid.
I don't know if this works, because of how few things are included in dolphin's ffmpeg.
h264's compatibility is still higher, and h265/vp9 don't offer dramatic improvements if we care about speed versus quality. Modern codecs are more efficient but also quite a bit slower than x264, so if you balance everything out they all have about the same sweet spot at speed/compression/quality, so there's little benefit for regular users. |
Thanks for all this info. So, do we want to drop the combobox and just change the FFV1 checkbox to UT Video? Any future plans can have two main options lossy/lossless, and then maybe more extra config options. If so, should it be called "Ut Video" or "Lossless"? I had never heard of Ut Video before, so as a User who knows little Lossless is more understandable. If not, then I think this PR should just keep the combobox how it is, and someone else could mess with it more later. |
I'd call the 2 options |
If in the end we still expose just two options in the GUI (essentially replacing FFV1 with UTVideo), I'd keep the current design from master, just updating the tooltip and perhaps migrating the If a third codec option gets exposed in the GUI, then switching to a ComboBox starts making more sense, otherwise it's just extra clicks for no reason... |
Ut Video is faster and more compatible with editing programs, but produces larger files.
I turned it back into a checkbox, for utvideo. I just removed UseFFV1 and didn't do anything with it. If someone wants to use ffv1, with how it's coded, they would have to set the dump string to "ffv1" and not enable Use Lossless (to avoid an override). The option will be off by default for everyone, they'd have to enable it even if ffv1 was enabled before. I think that's okay because it indicates that the option has changed? I updated the description a little. If this is the wrong approach let me know. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I have no particular preference in regards to migrating the old setting to the new one in this case, and I think having the new INI key be just "UseLossless" future proofs any further (lossless) codec change. The updated label/description is also fine...
@JMC47 Do you have any thoughts on how this change is being done? |
Requested here:
https://bugs.dolphin-emu.org/issues/13657
Problem: "UseFFV1" should no longer be needed, but I'm not sure how/where to convert that over so it doesn't collide.