Repository navigation
fix: preserve explicit zero caption grain - #3032
miguel-heygen wants to merge 1 commit into
Conversation
miga-heygen
left a comment
There was a problem hiding this comment.
Clean, correct fix.
The bug: || coerces 0 to falsy, so (dna.plate || {}).grain || 5 replaced an explicit grain: 0 with 5 — the noise filter always ran even when the theme author explicitly disabled it.
The fix: ?? preserves 0 (only null/undefined trigger the default). The noise filter string is now built conditionally — empty when grain === 0, which cleanly removes the ,noise=alls=... segment from the ffmpeg filter graph rather than passing noise=alls=0 (which would still allocate the filter for no visual effect).
Two things I verified:
- The
filter+rgba+noiseconcatenation still produces a valid filter graph in both the zero and non-zero paths. Thenullfilter name in the test output ([0:v]null[v]) is expected — it's the upstreamfiltervariable's value when no spatial filter is active. - The
P.grain || 5on line 8540 (heroless branch) is also replaced with??, so the grain value is consistent between the plate object construction and its later use.
Test coverage is proportionate: explicit-zero and missing-grain are the two boundary cases that matter. No notes.
— Miga
miga-heygen
left a comment
There was a problem hiding this comment.
No blocking issues found in my review. Stamping. — Miga
|
Continuing this contribution in #3656, rebuilt on current main with regression coverage and attribution to this PR. Keeping this PR open until the successor lands. Thanks for the original fix. |
|
Closing as implemented in #3656, now merged. The successor preserves this contribution while updating it for current main and adding regression coverage. Thanks for the original fix. |
fixes reported:1785872503.756319; no related claim remains outstanding
Source claim: https://slack.com/archives/C0BGC335AQY/p1785872503756319
Summary:
Regression coverage:
Verification: