feat: absolute parameter curve editing for variance params - #2
Closed
KakaruHayate wants to merge 2 commits into
Closed
feat: absolute parameter curve editing for variance params#2KakaruHayate wants to merge 2 commits into
KakaruHayate wants to merge 2 commits into
Conversation
- Add DeltaInverse to VarianceSpec (energy/breathiness: linear, voicing: bisection) - Add variance_param_mode part property (delta/absolute dropdown) - BuildAutomationConfigs dispatches continuous delta vs piecewise absolute tracks - BuildReadbackConfigs skips readback in absolute mode (user track IS the real value) - CombineVarianceAbsolute: NaN=follow prediction, edited=DeltaInverse→Delta - SyncAutomationSubscriptions handles same-key automation object swap - Add VarianceInverseTests (linear roundtrip, voicing boundaries, bisection convergence) - Add DiffSingerVarianceMath (zero-dependency math for unit tests) - Update schema doc §14.6 (mode semantics, VoicingDeltaInverse, data isolation) - zh-CN localization for mode toggle
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughChangesVariance Parameter Modes
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PartProperties
participant DiffSingerSynthesisSession
participant CombineVarianceAbsolute
participant VarianceMath
PartProperties->>DiffSingerSynthesisSession: resolve variance mode
DiffSingerSynthesisSession->>CombineVarianceAbsolute: combine predicted and edited values
CombineVarianceAbsolute->>VarianceMath: apply DeltaInverse to absolute targets
VarianceMath-->>CombineVarianceAbsolute: return delta coefficients
CombineVarianceAbsolute-->>DiffSingerSynthesisSession: return variance input series
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
DiffSingerDeclarations.cs (1)
90-128: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Variances/InvertVoicing/VoicingDeltaDownare fully duplicated withDiffSingerVarianceMath.cs.This array and the two helper functions are byte-for-byte the same formulas as
VarianceMath.Variances/InvertVoicing/VoicingDeltaDowninDiffSingerVarianceMath.cs, just withUse/Predictgating added. The comment inDiffSingerVarianceMath.cseven says "与 DiffSingerDeclarations.Variances 同源" (same origin) — i.e. the two are meant to be kept manually in sync. Any future tweak to a Delta/DeltaInverse formula applied to one copy but not the other will silently desync production behavior from the unit-tested math, defeating the purpose ofVarianceInverseTests.cs.Consider building this array by composing
VarianceMath.Variances(addingUse/Predictper key) instead of re-declaring the formulas, so there is a single source of truth that the tests actually exercise.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@DiffSingerDeclarations.cs` around lines 90 - 128, The Variances definitions and voicing inversion helpers in DiffSingerDeclarations are duplicated from VarianceMath, creating two formula sources. Refactor Variances to compose the existing VarianceMath.Variances entries while adding the declaration-specific Use and Predict selectors, and remove the duplicated InvertVoicing and VoicingDeltaDown implementations; preserve the existing keys, metadata, gating, and prediction behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@DiffSingerDeclarations.cs`:
- Around line 481-483: Change the visibility of the static helper VarianceModeOf
in DiffSingerDeclarations from private/default to public so
DiffSingerSynthesisSession can call it through the static import, preserving its
existing behavior and signature.
In `@DiffSingerSynthesisSession.cs`:
- Around line 61-65: Remove lifetime caching of variance mode and mode-dependent
state in DiffSingerSynthesisSession, including mAbsoluteVarianceKeys and
mReadbackConfigs. In Render, derive the mode from snapshot.PartProperties, and
when part properties change rebuild the readback configurations and variance-key
set from the current properties/config so delta↔absolute toggles use the correct
combine path and tracks.
In `@Localization.cs`:
- Around line 49-51: Add the missing “Absolute” localization entry to the
translation dictionary near the existing variance parameter mode entries, using
the appropriate Chinese translation so BuildFixedAutomationConfigs track labels
resolve through L.Tr("Absolute") instead of displaying the English fallback.
In `@tests/DiffSinger.Tests/VarianceInverseTests.cs`:
- Around line 97-114: Update AllParams_Roundtrip_SampledTargets to sample valid
edit values y for each variance specification, derive targets by calling
spec.Delta(predicted, y), then apply the same edit-range clamp used by rendering
before calling spec.DeltaInverse. Assert the round-trip result against the
derived target, preserving the existing predicted inputs and skipping
specifications without DeltaInverse.
---
Nitpick comments:
In `@DiffSingerDeclarations.cs`:
- Around line 90-128: The Variances definitions and voicing inversion helpers in
DiffSingerDeclarations are duplicated from VarianceMath, creating two formula
sources. Refactor Variances to compose the existing VarianceMath.Variances
entries while adding the declaration-specific Use and Predict selectors, and
remove the duplicated InvertVoicing and VoicingDeltaDown implementations;
preserve the existing keys, metadata, gating, and prediction behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2776371f-cc63-40ee-a87d-3bbdb1f99e15
📒 Files selected for processing (7)
DiffSingerDeclarations.csDiffSingerSynthesisSession.csDiffSingerVarianceMath.csLocalization.csdocs/tunelab-voicebank-schema.mdtests/DiffSinger.Tests/DiffSinger.Tests.csprojtests/DiffSinger.Tests/VarianceInverseTests.cs
接 CodeRabbit review，另修一处会让绝对值模式上下颠倒的真 bug。 - VarianceMath.InvertVoicing：二分比较方向写反。下行 Delta(x,·) 在 y∈[0,1] 上单调**递增**（y=0 → -96dB 静音底、y=1 → 预测值），原 `> target` 令每次 求逆都收敛到区间另一端，绝对值模式下输出与用户所画曲线上下颠倒。 改 `< target` 后全域求逆最大误差 1e-4 dB。 - CombineVarianceAbsolute：不再把逆函数解出的 y 钳到编辑量程——绝对值模式下 用户轨即实参，钳 y 会让输出偏离用户画的目标值；逆函数自身已保证域安全， 结果仍按声学量程 clamp 兜底。另修 user[f] 的越界解引用（CS8602）。 - 去掉 mReadbackConfigs / mAbsoluteVarianceKeys / mConfig 三处构造期缓存， 改为按当前 part 属性现算，使 delta↔absolute 切换即时生效。 新增 AbsoluteModeOf 处理实时只读属性外观（该外观无 GetString）。 - VarianceSpec 改为包装 VarianceMath.VarianceSpec，公式单一来源： 声库面只追加 Use/Predict，数值元数据与 Delta/DeltaInverse 全部转发。 - 补 "Absolute" zh-CN 词条（轨名后缀此前回落英文）。 - 测试：LinearParams_OutOfRange_Clamped 原按 dB 刻度取 predicted，对量程 [-10,10] 的 tension 恒越界必挂——改为按各 spec 自己的量程取值，并补上 production 的声学 clamp；上行分支测试补量程内/外两例；新增 Voicing_DownwardBranch_IsMonotonicIncreasing 钉住二分方向。 - 文档记录单调方向、1.25 钳制，及 x<-72 时公式在 -96dB 以下的非单调下探 （整段在声学量程外、被 clamp 压平，不影响可达目标的求逆精度）。 54 tests pass; 插件/测试/冒烟工具 Release 全部 0 warning 0 error。
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
Implements absolute-value editing mode for DiffSinger variance parameters (energy / breathiness / voicing / tension), following ACE Studio's 包络/実参 tab pattern.
Files changed
Summary by CodeRabbit