Repository navigation
fix(core): parse and format option numbers in the C locale whatever the caller's - #2351
Merged
Merged
Conversation
lusoris
force-pushed
the
fix/numeric-options-c-locale
branch
from
October 6, 2026 19:29
b332699 to
ec1090a
Compare
lusoris
added a commit
that referenced
this pull request
Oct 6, 2026
…tegration branch The master fix (#2351) gained a third part after the first carry: a feature named after a fractional option (`..._0.7`) was formatted with `%g` in the caller's locale and came out as `..._0,7`, so a model never found its scores. feature_name.cpp now formats the number in the C locale on the calling thread. This takes #2351's final dict.cpp, feature_name.cpp, test_locale_handling.c (MuTest table, new feature-name and model-feature cases) and the matching changelog, AGENTS.d page, rebase note and state row. Test: test_locale_handling 10/10; fast suite 385 ok, 0 failed.
…e host and record the CUDA/HIP angle-flag corner (#2359) * test(adm): hold the ADM twins' whole decouple header to the CPU on the host and record the CUDA/HIP angle-flag corner test_adm_decouple_recip_{cuda,hip} compiled only decouple_r_s0() of the twins' header, so CodeQL reported the rest unused (cpp/unused-static-function, 16 alerts). The test now also holds decouple_r_s123(), get_best15_from32() and both angle flags to the CPU's over 800000 draws and the corners of the int16 range, and each executable has its own run_tests root. The corners expose a real difference: with every band at -32768 the CUDA and HIP scale-0 angle flag adds in int32 and wraps where the CPU's int64 sum does not; a 64-bit form puts adm_cm_aim_line_kernel_4 past its register budget, so the row stays open with the measurements and the test records the count. Metal's iadm_angle_flag_s0() had the same sums and is fixed. The two include-non-header findings of the device-source tests become declared exceptions.
…he caller's (#2351) * fix(core): parse and format option numbers in the C locale whatever the caller's A program running in a decimal-comma locale (setlocale(LC_ALL, "") under de_DE, fr_FR and others) could not use the default model vmaf_v1.0.16_3d0h: vmaf_option_set() read "0.7" with strtod() in the caller's locale, stopped at the period and refused the option, so vmaf_use_features_from_model() returned -EINVAL. The feature dictionary's number normalisation could store "0.02" as "0" or "0,02", and a feature named after a fractional option came out as "..._0,7", a name no model reads, so the model's scores were never found. opt.cpp parse_double(), dict.cpp dict_normalize_numeric() and feature_name.cpp format_double_c_locale() now run inside a thread C-locale scope, as the model reader and report writers do. State row T-OPTION-NUMBERS-CALLER-LOCALE-2026-10-06.
lusoris
force-pushed
the
fix/numeric-options-c-locale
branch
from
October 6, 2026 20:06
ec1090a to
33bbea4
Compare
6 tasks done
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A program running in a decimal-comma locale could not use the default model: option numbers were parsed and formatted in the caller's locale. Under
de_DE.UTF-8(anysetlocale(LC_ALL, "")with a comma decimal separator)vmaf_option_set()read0.7withstrtod(), stopped at the period and refused the option, sovmaf_use_features_from_model()returned-EINVALforvmaf_v1.0.16_3d0h, whose features take fractional options;vmaf_v0.6.1happened to work. The feature dictionary's number normalisation had the same fault and could store0.02as0or0,02, and a feature named after a fractional option came out as..._dlmw_0,7, a name no model reads (VMAFX_E_NOTFOUNDfor the model's scores). All three now run inside a thread C-locale scope (vmaf_thread_locale_push_c()), as the model reader and the report writers already do; the caller's locale is left as it was.Found by the RC4 GStreamer element:
gst-launch-1.0sets the user's locale, and windows over the default model then failed because the extractors' options were parsed in the streaming thread. The CLI and FFmpeg run in the C locale and never showed it.Type
fix— bug fixChecklist
make format && make lintis green locally — the commit hooks pass.python3 scripts/ci/run_meson_test.py -- -C build-cpu --suite=fast --num-processes 6-> 375 OK, 0 fail./cross-backend-diffand the worst ULP is ≤ 2. — not applicable: option parsing only..c/.cpp/.cu/.h/.hpp, it has the appropriate license header — no new file.!orBREAKING CHANGE:and the migration path is documented below. — not a breaking change.docs/adr/_index_fragments/<NNNN-slug>.md— no ADR: a defect fix that follows the existing thread-locale pattern.tidy: cpu
core/src/dict.cpp,core/src/opt.cpp,core/src/feature/feature_name.cpp,core/test/test_locale_handling.c0 findings (scripts/dev/tidy-lane.sh --only ... cpu, clang-tidy 22.1.8; a first run found a public member in the scope class and an oversizedrun_tests(), both fixed: the scope is a class with a private member, the test list aMuTesttable).Bug-status hygiene (ADR-0165)
docs/state.mdupdated —T-OPTION-NUMBERS-CALLER-LOCALE-2026-10-06in Recently closed.Netflix golden-data gate (ADR-0024)
assertAlmostEqual(...)score in the Netflix golden Python tests.make test-netflix-golden(GOLDEN_NINJA_JOBS=4): 280 passed, 3 skipped. Scores in the C locale are unchanged (the parse is the same there).Cross-backend numerical results
Not applicable: no extractor or kernel changed.
Deep-dive deliverables (ADR-0108)
AGENTS.mdinvariant note —core/src/AGENTS.d/output-writers-and-locale.mdgains the option-number rule and the two files in itspaths.changelog.d/fixed/option-numbers-c-locale.md.docs/rebase-notes.md"Option numbers parse in the C locale".Reproducer
Failing first: with
core/src/opt.cppunfixedtest_option_double_with_comma_localefails; withcore/src/dict.cppunfixedtest_dictionary_number_with_comma_localefails; withcore/src/feature/feature_name.cppunfixedtest_feature_name_with_comma_localefails;test_model_features_with_comma_localeloads the default model and registers its features underde_DE.UTF-8.Known follow-ups
None. The RC4 integration branch (#2343) carries this fix until the RC4 lanes restack onto master.