Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 41 additions & 1 deletion .github/workflows/timing-parity.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,52 @@ on:
- 'scripts/regenerate_timing_fixture.py'
- 'docs/spec/phoneme-timing-contract.toml'
- '.github/workflows/timing-parity.yml'
# The five other runtime implementations. Without these a PR that
# changes only, say, timing.rs never runs the drift check.
- 'src/rust/piper-core/src/timing.rs'
- 'src/go/piperplus/timing.go'
- 'src/wasm/openjtalk-web/src/timing.js'
- 'src/csharp/PiperPlus.Core/Inference/TimingWriter.cs'
- 'src/cpp/timing_helpers.hpp'
- 'src/cpp/piper.cpp'
# The per-runtime parity tests that `per-runtime-presence` below exists
# to stop anyone deleting. They were NOT listed, so a PR that deleted
# one skipped this workflow entirely and the guard never ran -- the one
# change it was written to catch was the one it could not see.
- 'src/python_run/tests/test_phoneme_timing_parity.py'
- 'src/rust/piper-core/tests/test_phoneme_timing_parity.rs'
- 'src/go/piperplus/timing_parity_test.go'
- 'src/cpp/tests/test_phoneme_timing_parity.cpp'
- 'src/cpp/tests/test_timing_helpers.cpp'
- 'src/csharp/PiperPlus.Core.Tests/TimingWriterParityTests.cs'
- 'src/wasm/openjtalk-web/test/js/test-phoneme-timing-parity.js'
push:
branches: [ dev ]
paths:
- 'src/python_run/piper_plus/timing.py'
- 'tests/fixtures/phoneme_timing/**'
- 'scripts/regenerate_timing_fixture.py'
- 'docs/spec/phoneme-timing-contract.toml'
- '.github/workflows/timing-parity.yml'
# The five other runtime implementations. Without these a PR that
# changes only, say, timing.rs never runs the drift check.
- 'src/rust/piper-core/src/timing.rs'
- 'src/go/piperplus/timing.go'
- 'src/wasm/openjtalk-web/src/timing.js'
- 'src/csharp/PiperPlus.Core/Inference/TimingWriter.cs'
- 'src/cpp/timing_helpers.hpp'
- 'src/cpp/piper.cpp'
# The per-runtime parity tests that `per-runtime-presence` below exists
# to stop anyone deleting. They were NOT listed, so a PR that deleted
# one skipped this workflow entirely and the guard never ran -- the one
# change it was written to catch was the one it could not see.
- 'src/python_run/tests/test_phoneme_timing_parity.py'
- 'src/rust/piper-core/tests/test_phoneme_timing_parity.rs'
- 'src/go/piperplus/timing_parity_test.go'
- 'src/cpp/tests/test_phoneme_timing_parity.cpp'
- 'src/cpp/tests/test_timing_helpers.cpp'
- 'src/csharp/PiperPlus.Core.Tests/TimingWriterParityTests.cs'
- 'src/wasm/openjtalk-web/test/js/test-phoneme-timing-parity.js'

concurrency:
group: ${{ github.workflow }}-${{ github.head_ref || github.ref }}
Expand Down Expand Up @@ -72,7 +111,7 @@ jobs:
steps:
- uses: actions/checkout@v6.1.0

- name: Assert all 6 per-runtime parity test files exist
- name: Assert all per-runtime parity test files exist
run: |
set -e
missing=0
Expand All @@ -88,6 +127,7 @@ jobs:
check src/rust/piper-core/tests/test_phoneme_timing_parity.rs
check src/go/piperplus/timing_parity_test.go
check src/cpp/tests/test_phoneme_timing_parity.cpp
check src/cpp/tests/test_timing_helpers.cpp
check src/csharp/PiperPlus.Core.Tests/TimingWriterParityTests.cs
check src/wasm/openjtalk-web/test/js/test-phoneme-timing-parity.js
if [ "$missing" -ne 0 ]; then
Expand Down
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- C++: phoneme timing の cursor walk を `src/cpp/timing_helpers.hpp` に切り出し、**本番コードに初めてテストを通した**。`test_phoneme_timing_parity.cpp` は cross-runtime golden fixture を読みながら、spec のアルゴリズムを**自身の翻訳単位で再実装**していた (コメントに `To stay self-contained (no onnxruntime/espeak link), this test re-implements the spec algorithm` と明記)。他 5 ランタイムはいずれも本番関数を呼んでいる (Python `durations_to_timing` / Rust `durations_to_timing` / Go `DurationsToTiming` / JS `durationsToTiming` / C# `TimingWriter.CalculateTiming`) ため、**C++ だけが出荷される挙動について何も検査していなかった**。新設した `test_timing_helpers.cpp` (16 ケース) が本番実体を直接検査し、変異 5 種 (PAD/BOS/EOS skip の削除 / skip 時に cursor を進めない / accumulator を float に戻す / 促音 overlap 比率の変更 / frame の truncate を round に) がすべて検出されることを実測確認した
- C++: timing cursor の accumulator を `float` から `double` にした。値の格納は `float` のまま (`PhonemeInfo` は `SynthesisResult` 経由で shared library から公開されており、幅を変えると `ABI diff (libpiper_plus.so head vs base)` gate が検出する struct レイアウト変更になる) なので、丸めは **1 エントリにつき 1 回**になり、発話全体にわたる累積がなくなる。実測: ceil 後相当の 1153 frame で float32 累積は float64 基準から **2.1e-3 ms** ずれており、同 parity テストが宣言する許容誤差 `kAbsTolMs = 1e-6` を大きく超えていた (レプリカが `double` だったため露見していなかった)
- ci: `timing-parity.yml` の `paths:` を 5 件から 18 件に拡張した。`per-runtime-presence` job は「parity テストを削除したら fail する — that is intentional」と宣言しているのに、**守る対象 6 ファイルが 1 つも trigger に入っておらず**、parity テストを削除するだけの PR では workflow 自体が起動しなかった。他 5 ランタイムの timing 実装も未登録で、`timing.rs` だけを変更する PR では drift check が走らなかった。同型の穴が他 7 workflow にもあることを走査で確認し #704 に起票した
- tests: timing golden fixture に判別力のあるケースを 3 件追加した (`fractional_durations` / `fractional_just_above_integer` / `long_utterance_accumulation`)。従来の 7 ケースは**全て整数 durations** (最大 10 frame / 最長 11 音素) で、実際の ONNX `durations` 出力が常に小数であるにもかかわらず代表していなかった。`ceil(整数) == 整数` なので、frame 量子化を変えても fixture は 1 バイトも変わらない。**実測**: Go に `math.Ceil` を入れる変異は旧 fixture では `ok` で素通りし、新 fixture では `--- FAIL` になる。Rust でも同様に検出される。併せて `scripts/regenerate_timing_fixture.py` に「小数ケース 2 件以上 / 最長ケース 50 エントリ以上」の anti-vacuity guard を入れ、ケースが後から整数化・短縮されたら `--check` が fail するようにした (変異 2 種で 2/2 検出を確認)

- Rust / Go: phoneme timing の音素名を実際の音素に解決するようにした (#656)。 両ランタイムの CLI は `ph_0`, `ph_1`, ... (Rust) / `p0`, `p1`, ... (Go) という**連番プレースホルダ**を出力しており、 timing 出力からどの音素がどの区間かを識別できず lip-sync / 字幕用途で使えなかった。 `docs/spec/phoneme-timing-contract.toml` の `[reverse_map]` (first-wins 衝突解決 / PUA `U+XXXX` fallback / 明示マッピング優先) を canonical (`src/python_run/piper_plus/timing.py`) と JS mirror に合わせて実装した — Rust `build_phoneme_id_reverse_map` + `phoneme_ids_to_tokens`、 Go `BuildPhonemeIDReverseMap` + `PhonemeIDsToTokens`。 逆引きマップに無い ID は `<id>` 形式で出し、 どの ID が未知なのかが分かるようにした (`ph_N` は何も伝えない)。 **反復順**: Python の dict と JS の object は挿入順を保つため first-wins の「最初」が一意だが、 Rust の `HashMap` と Go の `map` は反復順が非決定的なため**キーをソートしてから**反復する。 ID 衝突の無いマップでは順序が結果に影響しないため canonical と完全一致する (in-tree fixture は 173 キーで衝突 0 件); 衝突のあるマップでは異なる勝者を選び得ることをコメントに明記した
- Rust / Go: 合成に実際に渡した phoneme ID 列を結果として返すようにした (`SynthesisResult::phoneme_ids` / `SynthesisResult.PhonemeIDs`)。 両ランタイムは `durations` を original 長へ**先頭から** `truncate` していたが、 padded layout は `[BOS, frontPad.., body.., backPad.., EOS]` なので body が `frontPad` 個ずれ、 index 対応で音素名を引き当てると**発話音素が pad の duration を受け取る** (#689 と同型)。 `truncate` をやめ padding 後の長さのまま返し、 対応付け用の ID 列を併せて返す形にした。 呼び出し元が `phonemize_to_ids(text)` を再実行して対応付けるのは誤りで、 Rust では Strategy C が 10 文字以下のテキストを SSML でラップするため合成に使う音素列がテキスト由来のものと別物になる (実測: `"Sol"` で ID 11 個に対し durations 111 個)。 CLI は長さが一致しない場合のみプレースホルダにフォールバックし警告を出す — 誤った音素名を出すほうが害が大きいため
- C#: 逆引きマップの spec 違反 2 件を修正した (#656)。 (1) `reverse.TryAdd(ids[0], display)` が**キーの最初の ID しか登録せず**、 1 音素が複数 ID を持つモデルで残りの ID が `"?"` にフォールバックしていた — contract の例は `{"b": [6, 7]}` → `{6: 'b', 7: 'b'}` を要求する。 (2) `OpenJTalkToPiperMapping.CharToToken` に無い PUA 文字が**生の PUA 文字のまま**出力されており、 `[reverse_map.pua_handling]` が要求する `U+XXXX` 形式になっていなかった (他 5 ランタイムはすべて `U+XXXX`)。 修正前に回帰テスト 2 件が fail することを実測確認済み
Expand Down
99 changes: 99 additions & 0 deletions scripts/regenerate_timing_fixture.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,51 @@
"hop_length": 256,
},
},
{
"name": "fractional_durations",
"description": (
"Non-integer frame counts — what a real ONNX `durations` output "
"actually looks like. Every other case here is integer-valued, "
"and ceil(int) == int, so an implementation that forgot to apply "
"the frame quantisation the decoder uses would match all of them "
"(issue #653). This case is the one that can tell them apart."
),
"inputs": {
"durations": [2.5, 3.25, 1.75, 4.125],
"phoneme_tokens": ["a", "b", "c", "d"],
"sample_rate": 22050,
"hop_length": 256,
},
},
{
"name": "fractional_just_above_integer",
"description": (
"Durations a hair above an integer. Distinguishes truncation from "
"rounding from ceiling: trunc(3.0001)=3, round(3.0001)=3, "
"ceil(3.0001)=4. A single .5 case cannot separate round from ceil."
),
"inputs": {
"durations": [3.0001, 7.0001],
"phoneme_tokens": ["x", "y"],
"sample_rate": 22050,
"hop_length": 256,
},
},
{
"name": "long_utterance_accumulation",
"description": (
"60 phonemes of fractional duration. The other cases top out at "
"11 entries / 50 frames, where a float32 cursor drifts only ~5e-5 "
"ms from float64 and slips under every runtime's tolerance. This "
"length makes the accumulator width observable."
),
"inputs": {
"durations": [round(2.0 + (i % 7) * 0.375, 6) for i in range(60)],
"phoneme_tokens": [f"p{i % 10}" for i in range(60)],
"sample_rate": 22050,
"hop_length": 256,
},
},
{
"name": "all_zero_durations",
"description": "All-zero durations → contiguous zero-length boundaries",
Expand Down Expand Up @@ -140,6 +185,49 @@ def build_fixture() -> dict:
}


# Minimum discriminating power the case list must retain.
#
# Every original case used integer frame counts, and `ceil(int) == int`, so a
# runtime that applied the decoder's frame quantisation (or failed to) matched
# all of them. Measured: applying `math.Ceil` to the Go implementation passed
# the integer-only fixture and fails the current one. The long case exists for
# the same reason at a different axis -- 11 entries is too short for a float32
# cursor to drift past any runtime's tolerance.
#
# These are asserted rather than left to reviewer vigilance because the failure
# mode is silent: a "simplified" case list still produces a green parity matrix.
MIN_FRACTIONAL_CASES = 2
MIN_LONG_CASE_ENTRIES = 50


def _validate_discriminating_power(cases: list[dict]) -> list[str]:
"""Return human-readable reasons the case list cannot detect known defects."""
problems: list[str] = []

fractional = [
c
for c in cases
if any(float(d) != int(float(d)) for d in c["inputs"]["durations"])
]
if len(fractional) < MIN_FRACTIONAL_CASES:
problems.append(
f"only {len(fractional)} case(s) use non-integer durations "
f"(need >= {MIN_FRACTIONAL_CASES}). Real ONNX `durations` output is "
"always fractional; with integer-only cases a frame-quantisation "
"change is invisible to every runtime's parity test (issue #653)."
)

longest = max((len(c["inputs"]["durations"]) for c in cases), default=0)
if longest < MIN_LONG_CASE_ENTRIES:
problems.append(
f"longest case has {longest} entries (need >= {MIN_LONG_CASE_ENTRIES}). "
"Short cases keep cursor accumulation error below every runtime's "
"tolerance, so a narrower accumulator goes undetected."
)

return problems


def _serialize(fixture: dict) -> str:
return json.dumps(fixture, indent=2, ensure_ascii=False) + "\n"

Expand All @@ -155,6 +243,17 @@ def main() -> int:
)
args = parser.parse_args()

problems = _validate_discriminating_power(CASES)
if problems:
print(
"ERROR: the fixture case list cannot detect the defects it is "
"meant to pin:",
file=sys.stderr,
)
for problem in problems:
print(f" - {problem}", file=sys.stderr)
return 1

serialized = _serialize(build_fixture())
rel = FIXTURE_PATH.relative_to(REPO_ROOT)

Expand Down
Loading
Loading