Skip to content
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

forcing x264 level=4.2 breaks with certain localizations #745

Closed
kparal opened this issue Jun 1, 2019 · 2 comments
Closed

forcing x264 level=4.2 breaks with certain localizations #745

kparal opened this issue Jun 1, 2019 · 2 comments
Milestone

Comments

@kparal
Copy link

kparal commented Jun 1, 2019

I want a certain clip to be forced to x264 level 4.2. If I add level=4.2 to the Other tab during export, everything works fine on an English system. However, on my system with cs_CZ.UTF-8 system locale, it breaks in several different ways, all very unobvious to the user:

  • With the default H264 High profile values for x264, aac and mp4 container, only audio is exported (video is missing), but Shotcut shows it as a successful export.
  • With the default values for x264, aac but with mkv container, the job is immediately marked as failed, without explanation.
    In both cases, this is present in the log:
[aac @ 0x7f0530003bc0] [Eval @ 0x7f0536042f40] Invalid chars '.2' at the end of expression '4.2'
[aac @ 0x7f0530003bc0] Unable to parse option value "4.2"
[libx264 @ 0x7f0530001e80] Error parsing option 'level' with value '4.2'.

An additional sneaky thing is that this problem persists even if you switch your Shotcut language to English - the numeric settings are still pulled from the system.

This was actually my case, I switched Shotcut to English immediately after installing it (because the Czech translation is poor), and it was a herculean effort to figure out why level=4.2 doesn't work. I spent many hours debugging before realizing this could be locale-related (despite app language being English). The common user has no chance here.

So in my case, the "workaround" is to use level=4,2 (because we have a decimal comma). Of course that breaks when I load my saved profile on a system with English locale... 😞

And of course all the online guides speak of "level 4.2" (not "4,2") and ffmpeg itself also accepts just
-level 4.2 (not -level 4,2) regardless of system locale. Again, a common user is pretty much out of luck here.

Internationalization makes sense in GUI, but it doesn't make much sense in plumbing/APIs/command line arguments. Whatever is put into the Other tab, please consider it literal and don't try to process it the same way GUI fields are processed. The same thing applies for data saved in custom profiles, XML intermediary files, etc. Thank you.

Fedora 30, Shotcut 19.04.30 installed from Flathub

@ddennedy
Copy link
Member

ddennedy commented Jun 1, 2019

I consider this is as a duplicate of #713 . The reason this came into place is the fundamental properties object in MLT provides conversion services between strings and numerics, and properties including effect parameters and encoding options are not strongly typed. Then, some UIs and APIs could permit entering some numerics as strings, the user uses their numeric locale, and then it would not be interpreted correctly. We just have to ensure Shotcut is not doing that, but some areas like the Export > Other box is unavoidable, and it will be inconsistent with the rest of the UI after the change, which is unavoidable. I can add a text note there.
I am still testing the changes for #713, and I still have to add code to deal with backwards compatibility and test that.
Lastly, language selection in the app UI does not dictate locale as you noticed, and I consider that normal and will not be changed.

@kparal
Copy link
Author

kparal commented Jun 1, 2019

Thanks for working on this.

Lastly, language selection in the app UI does not dictate locale as you noticed, and I consider that normal and will not be changed.

I'm not saying it needs to change, it can just increase the confusion. But if the situation is improved e.g. by adding some explanation to the Other box (e.g. that everything should be specified using English locale, and is then parsed as such), then it might be a non-issue.

ddennedy added a commit that referenced this issue Jun 3, 2019
Fixes #745 and #713

MLT applies the system locale by default, which we can control in the
current process mainly using setlocale(). However, we need to set an
environment variable to affect the child process (variable already needed
in Windows by the xml consumer's call to getlocale).

Next, we need to inform users of Export > Other how to enter numeric
values depending on the MLT locale state of the current project. And we
need to conform encode presets' floating point numeric values.
@ddennedy ddennedy added this to the v19.06 milestone Jun 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants