feat(engine): audio effect latency reporting + lookahead tail compensation by yuto-trd · Pull Request #1998 · b-editor/beutl

yuto-trd · 2026-06-23T16:37:20Z

Description

Implements board #77 end to end: a latency reporting API plus tail-loss compensation for lookahead-bearing audio effects (the Limiter). Lookahead delays the signal by lookaheadSamples; because Composer allocates exactly ceil(Duration*rate) samples and never reads past the clip, the clip lost that many samples off its tail (stuck in the delay line). This recovers them.

Reporting

AudioNode.GetLatencySamples(int sampleRate) (virtual, default 0) reports a node's own latency; GetTotalLatencySamples aggregates the feeding path as local + max(input totals) — a linear cascade sums, a mixer fan-in takes the slowest branch. Both virtual/overridable.
AudioEffect.GetLatencySamples lets a host query latency before a graph node exists; AudioEffectGroup sums enabled children. LimiterNode/LimiterEffect override via LimiterParameters.ToLatencySamples. All entry points guard sampleRate > 0.

Compensation (node `Flush` contract)

AudioNode.Flush(AudioProcessContext): a node drains the latency it still holds as the block that follows the clip's last Process output. The real source is treated as exhausted — a leaf node returns silence — so a trimmed clip never bleeds real audio. A single-input node runs its upstream drain through its own ProcessTail, so downstream effects still shape the tail; a fan-in node (MixerNode) overrides Flush to drain and mix every branch; the base throws for an un-overridden multi-input node.
ClipNode, on the window that reaches the clip's true end (newRange.End == range.End), calls Inputs[0].Flush over clip-local [Duration, Duration+L) — contiguous with the main slice, so the cached effect never resets — and appends the drained tail into the trailing pad the window already reserves, clamped to capacity.
ClipNode.Flush remaps a parent-domain drain back to clip-local [Duration, …) so a nested clip (a SoundGroup child mixed under a group clip) keeps its cached limiter contiguous instead of resetting and dropping the tail. The parent's start is intentionally dropped, which is also why an intervening ShiftNode needs no flush override.

Why a node contract, not range expansion

A design pass (3 approaches × 3 adversarial verifiers) confirmed the board's "expand the Composer range + shift output" is incompatible with this stateful chunked pull graph: expanding each window pushes consecutive preview windows outside LimiterNode's 1-tick contiguity tolerance, triggering a spurious Reset() + double-processed overlap at every chunk boundary. Compensation therefore lives in a node Flush contract that keeps externally-observed TimeRange exactly tiling. Mid-clip windows stay byte-identical to before.

Scope: tail recovery only. Leading lookahead silence is intentionally left intact — removing it would shift the clip and break A/V sync (documented-intended LimiterEffect behavior).

Affected areas

Beutl.Engine (rendering / scene / track)
Beutl.ProjectSystem / UI / Beutl.Extensibility / Beutl.NodeGraph / Beutl.FFmpegIpc / Beutl.Api / Build-CI-docs

Breaking changes

None. All additions are additive (new virtual members with defaults, new overrides on sealed/partial classes); existing subclasses inherit the defaults unchanged. Process output is byte-identical except the clip's terminal window, which now carries the recovered tail in samples that were previously silent padding.

Test plan

Two NUnit fixtures under tests/Beutl.UnitTests/Engine/Audio/:

AudioLatencyTests: reporting — lookahead at 48k/96k, queryable before Process, matches the actually-measured impulse delay (incl. the 20 ms clamp boundary), pass-through/zero effects, group sum, cascade sum vs mixer max, uniform non-positive-rate throw.
AudioLatencyCompensationTests: Flush recovers the exact tail samples; Process+Flush concatenate to the full delayed input with no loss; a latency-free chain flushes to silence; Flush stays contiguous (real audio, not post-reset silence); downstream processing is applied to the drained tail; ClipNode terminal window appends the recovered tail; a nested ClipNode flushed in a parent's time domain remaps to clip-local and still recovers the tail; a nested clip trimmed by its parent flushes from its last processed local time (not its natural Duration); a nested flush continues after a partial tail append instead of stepping backward; MixerNode merges branch tails; a bare multi-input Flush throws; animated lookahead reports the worst-case latency; L==0 is a no-op.

Verified locally: dotnet build Engine green; dotnet test Engine.Audio latency suites 34 passed / 0 failed; dotnet format --verify-no-changes clean on touched files.

Follow-ups

Clip-boundary tail loss (known limitation, documented in ClipNode.AppendFlushedTail): when a compose block ends exactly at the clip boundary the recovered tail has nowhere to go and the next block excludes the clip (half-open TimeRange.Intersects), so a few ms of lookahead tail is lost on that alignment. Fixing it needs either an oversized terminal ClipNode output (breaks the output-length contract) or a compositor that keeps a latency-bearing clip live for one trailing block — both out of scope here.
Leading-edge compensation (remove the lookahead silence at clip start) needs a Composer-level state-aware scheduler with global output offset — out of scope here; the range-expansion shortcut is verified-wrong.
Animated-lookahead drain (known limitation, documented in LimiterNode.ProcessTail): on the flush path the animated Lookahead is sampled over the post-clip drain range, not the clip samples that were actually delayed. An automation that holds a high lookahead through the clip end but keyframes back to 0 at Duration reads Read(0) (just-written silence) and still drops the tail even though GetLatencySamples reserved the worst case. Draining at the retaining lookahead is a follow-up.
Custom-leaf channel count (known limitation, documented in AudioNode.RecordProcessedChannelCount): a custom zero-input leaf that omits RecordProcessedChannelCount flushes stereo silence regardless of its real mono/multichannel layout, which can make a downstream limiter reinitialize and drop its tail. Built-in SourceNode records correctly; making the base learn the emitted layout is a follow-up.
DelayNode latency budget (known limitation, documented in DelayNode): DelayNode deliberately reports 0 latency — an echo/delay is an intentional, audible delay (render-tail-time territory), not processing latency that PDC compensates — so a wet-only delay placed after a limiter gets no extra flush budget and its delayed tail can still clip at the clip boundary. Recovering it is a separate render-tail-time concern, not latency reporting.
DelayNode/EqualizerNode Flush: only LimiterNode shapes a held tail today (the others use the pass-through ProcessTail default). A DelayNode echo tail or EqualizerNode IIR ring-out would need their own tail handling. The ClipNode drain is already capacity-bounded so a large reported cascade latency cannot overflow.
Inactive mixer branches on flush (known limitation, documented in MixerNode.Flush): the fan-in flush drains every branch unconditionally. A branch whose child clip ended before the group's terminal slice — and whose own terminal block landed exactly on its clip boundary (the chunk-alignment edge above) — still holds a stale tail that is emitted into the group pad seconds late. Draining only branches live through the terminal slice needs per-branch clip-liveness the mixer does not track today.
ResampleNode flush rate mapping (known limitation, documented in ResampleNode): Process feeds its input a SourceSampleRate context and resamples to the output rate, but there is no Flush override, so the inherited flush forwards the output-rate context upstream and does not resample the drained tail. A latency node placed upstream of the resampler would see a rate change on flush, reinitialize, and drop its tail. The built-in Sound chain resamples before the effects (so a limiter is always downstream at the output rate); this only bites a custom graph that resamples after a latency-bearing node. A rate-mirroring Flush override is the follow-up.
Referenced-scene latency (known limitation, documented in SceneSound.SceneNode): the SceneNode leaf composes a referenced scene through its own Composer but reports the zero-latency default, so an outer ClipNode reserves no drain for a lookahead limiter inside the referenced scene; a SceneSound clip that cuts the scene before an inner limiter-bearing sound ends drops that inner tail. Surfacing it needs SceneNode to aggregate its internal graph's latency and flush that graph.

Fixed issues / References

Project board: b-editor/projects/9 ("Audio graph: latency reporting/compensation API for lookahead-bearing AudioEffects")

https://claude.ai/code/session_01PJ9eDL52jpiM1t5oDRCLM2

Summary by CodeRabbit

New Features
- Added host-queryable latency reporting for audio effects, effect groups, and graph nodes (including per-node and total/cascaded latency).
- Introduced node flush support so hosts can recover held tail audio; added mixer fan-in flushing.
- Added lookahead-to-sample latency reporting for limiter effects.
Bug Fixes
- Improved flush and clip-end tail handling to preserve limiter lookahead continuity and avoid dropped/reset state.
- Added stricter behavior when flushing unsupported multi-input topologies.
Tests
- Expanded unit tests for limiter/effect latency accuracy and process/flush tail continuity across limiter, clips, and mixers.

Lookahead-bearing audio effects (the Limiter's lookahead delay line) make the rendered clip lag and drop its tail by `lookaheadSamples`, but the audio graph had no way to report that latency. This adds a report-only API so the pipeline (and plugin hosts) can discover per-node and per-effect latency; it is the prerequisite for any future automatic compensation. - AudioNode.GetLatencySamples(sampleRate) (virtual, default 0) reports a node's own latency; GetTotalLatencySamples aggregates the feeding path as local + max(input totals), so a linear cascade sums and a mixer fan-in takes the slowest branch (both virtual/overridable for plugin authors). - AudioEffect.GetLatencySamples(sampleRate) lets a host query latency before a graph node exists; AudioEffectGroup sums its enabled children. - LimiterNode/LimiterEffect override it via a new LimiterParameters.ToLatencySamples helper that mirrors the runtime clamp. Report-only and behavior-preserving: the new methods are side-effect-free and are not called from Process/Compose, so render output is unchanged. Actual compensation (range/flush) is deferred — the board's range-expansion proposal is incompatible with the stateful chunked pull graph (it would trigger spurious LimiterNode resets and double-process overlap at every chunk boundary), so it needs a dedicated render-session layer. Refs: Project #9 "AI Review" item "Audio graph: latency reporting/compensation API for lookahead-bearing AudioEffects" Claude-Session: https://claude.ai/code/session_01PJ9eDL52jpiM1t5oDRCLM2

coderabbitai · 2026-06-23T16:37:29Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds GetLatencySamples(int sampleRate) and GetTotalLatencySamples(int sampleRate) to AudioNode and AudioEffect with Flush tail-recovery support. Introduces ProcessTail virtual hook for buffer transformation during drain. LimiterNode and LimiterEffect override these using a new LimiterParameters.ToLatencySamples helper. AudioEffectGroup sums enabled children's latencies. ClipNode appends recovered tails at clip end via Flush. Processing nodes refactored to use ProcessTail pattern. MixerNode overrides Flush for fan-in draining. Comprehensive test suites validate all latency-reporting and tail-recovery paths.

Changes

Audio Latency Query and Tail Recovery API

Layer / File(s)	Summary
Base latency and flush APIs on `AudioEffect` and `AudioNode` `src/Beutl.Engine/Audio/Effects/AudioEffect.cs`, `src/Beutl.Engine/Audio/Graph/AudioNode.cs`	Adds virtual `GetLatencySamples` (default `0`, positive-rate guard), `GetTotalLatencySamples` (recursive upstream max-branch aggregation), `Flush` (single-input passthrough-via-ProcessTail or silence allocation), `ProcessTail` hook (default pass-through), and channel-count tracking (`LastProcessedChannelCount`, `RecordProcessedChannelCount`, `CreateSilentFlush`).
`LimiterParameters.ToLatencySamples` conversion helper `src/Beutl.Engine/Audio/Effects/LimiterParameters.cs`	Static helper that validates `sampleRate`, handles non-finite `lookaheadMs`, clamps to `[MinLookaheadMs, MaxLookaheadMs]`, and returns a sample-latency value clamped to a `sampleRate`-derived ceiling.
`LimiterNode` and `LimiterEffect` latency overrides `src/Beutl.Engine/Audio/Graph/Nodes/LimiterNode.cs`, `src/Beutl.Engine/Audio/Effects/LimiterEffect.cs`	`LimiterNode` overrides `GetLatencySamples` (returns worst-case `MaxLookaheadMs` when animated, else current `Lookahead` value) and implements `ProcessTail` (shared delay-line/peak-buffer processing for both `Process` and `Flush` paths); `LimiterEffect` returns `0` when disabled, otherwise delegates to node calculation.
`AudioEffectGroup` latency aggregation `src/Beutl.Engine/Audio/Effects/AudioEffectGroup.cs`	Overrides `GetLatencySamples` to sum only enabled children's latencies, mirroring `CreateNode`'s serial cascade; caller decides whether to skip a disabled group.
`ClipNode` end-of-clip tail recovery `src/Beutl.Engine/Audio/Graph/Nodes/ClipNode.cs`	Detects when processed window reaches clip's true end and appends flushed tail (latency-held samples from the input chain) into output buffer via new `AppendFlushedTail` helper, which queries total upstream latency, flushes input, and copies drained samples into remaining capacity. `Flush` override constructs clip-local drain context at `Duration`.
Processing node refactoring to `ProcessTail` pattern `src/Beutl.Engine/Audio/Graph/Nodes/CompressorNode.cs`, `src/Beutl.Engine/Audio/Graph/Nodes/DelayNode.cs`, `src/Beutl.Engine/Audio/Graph/Nodes/EqualizerNode.cs`, `src/Beutl.Engine/Audio/Graph/Nodes/GainNode.cs`, `src/Beutl.Engine/Audio/Graph/Nodes/SourceNode.cs`	`Process` now passes the single input directly to `ProcessTail`, which assumes buffer ownership and disposal; `SourceNode` additionally records processed channel count (2) for consistent flush-silence sizing.
`MixerNode` fan-in flush override `src/Beutl.Engine/Audio/Graph/Nodes/MixerNode.cs`	Overrides `Flush` to support multi-input draining: returns silent flush when no inputs; otherwise routes through shared mixing logic in drain mode, selecting per-input `Flush` (when draining) or `Process` (during normal operation) to recover branch tails and mix with per-input gain folding.
`AudioLatencyTests` suite `tests/Beutl.UnitTests/Engine/Audio/AudioLatencyTests.cs`	15 tests covering impulse delay verification, query-before-process stability, sample-rate scaling, pass-through zero latency, effect disabled-state, group enabled-child filtering and group-state independence, linear-cascade summing, fan-in max-branch, and `ArgumentOutOfRangeException` contracts.
`AudioLatencyCompensationTests` suite `tests/Beutl.UnitTests/Engine/Audio/AudioLatencyCompensationTests.cs`	12 tests verifying tail recovery via `Flush`, process-then-flush concatenation contiguity, zero-lookahead no-op behavior, `ClipNode` terminal window tail appending, downstream processing on recovered tails, `MixerNode` flush branch merging, multi-input fan-in override requirement, animated lookahead worst-case reporting, and deterministic `RangeSineNode` test fixture.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

b-editor/beutl#1725: Introduces the limiter delay-line implementation that is extended in this PR with latency-query and tail-recovery APIs.

Poem

🐇 Hop hop, the latency is known,
Before the graph has even grown!
Lookahead samples held with care,
Max-branch paths beyond compare.
The limiter delays with grace,
While the clip tail finds its place. ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 27.14% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and concisely summarizes the main changes: adding audio effect latency reporting and lookahead tail compensation features to the audio engine.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch yuto-trd/audio-latency-report

_{Comment @coderabbitai help to get the list of available commands.}

Copilot

Pull request overview

Adds a report-only audio latency API to the Beutl audio graph/effects layer so hosts and pipelines can query per-node/effect latency (notably Limiter lookahead) as a prerequisite for future compensation.

Changes:

Introduces GetLatencySamples(int sampleRate) on AudioNode and AudioEffect (defaulting to 0) and an aggregate AudioNode.GetTotalLatencySamples(...) that folds upstream latency via local + max(inputs).
Implements limiter lookahead latency reporting via LimiterParameters.ToLatencySamples(...) and overrides on LimiterNode / LimiterEffect; adds AudioEffectGroup summation behavior mirroring CreateNode.
Adds NUnit coverage for limiter latency behavior, effect/group aggregation semantics, and non-positive sample rate guards.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tests/Beutl.UnitTests/Engine/Audio/AudioLatencyTests.cs	New fixture validating limiter latency reporting, aggregation rules, and argument validation.
src/Beutl.Engine/Audio/Graph/AudioNode.cs	Adds node latency reporting APIs and upstream aggregation method.
src/Beutl.Engine/Audio/Graph/Nodes/LimiterNode.cs	Overrides node latency reporting to expose lookahead delay.
src/Beutl.Engine/Audio/Effects/AudioEffect.cs	Adds effect-level latency reporting API for pre-graph queries.
src/Beutl.Engine/Audio/Effects/AudioEffectGroup.cs	Implements group latency as sum of enabled children (serial cascade).
src/Beutl.Engine/Audio/Effects/LimiterEffect.cs	Overrides effect latency reporting to match limiter lookahead (when enabled).
src/Beutl.Engine/Audio/Effects/LimiterParameters.cs	Adds shared helper to compute/clamp lookahead latency in samples.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Validate sampleRate in GetTotalLatencySamples itself rather than relying on the indirect throw via GetLatencySamples, so an override that folds the upstream recursion before delegating still honors the contract. Cover the aggregating entry point in the non-positive-rate test. Addresses Copilot review feedback on PR #1998. Claude-Session: https://claude.ai/code/session_01PJ9eDL52jpiM1t5oDRCLM2

Recovers the clip-tail audio that a lookahead effect (the Limiter) leaves stuck in its delay line. Adds AudioNode.Flush(context): a node drains the latency it still holds, treating the upstream source as exhausted (a leaf returns silence), so a trimmed clip never bleeds real audio. The default passes a single input's drain through unchanged; LimiterNode overrides it to push its lookahead delay line out through the same path Process uses. ClipNode, on the window that reaches the clip's true end, calls Inputs[0].Flush over the clip-local range [Duration, Duration+L) — contiguous with the main slice, so the cached effect state never resets — and appends the drained tail into the trailing pad the window already reserves. Crucially this does NOT expand the Composer's requested range: doing so pushes consecutive preview windows outside LimiterNode's 1-tick contiguity tolerance and triggers a spurious Reset + double-processed overlap at every chunk boundary (verified wrong in design review). Compensation is a node contract instead, so exact window tiling and cached DSP state are preserved. Scope: tail recovery only. Leading lookahead silence is left intact (removing it would shift the clip and break A/V sync — documented-intended behavior). Refs: Project #9 "AI Review" item "Audio graph: latency reporting/compensation API for lookahead-bearing AudioEffects" Claude-Session: https://claude.ai/code/session_01PJ9eDL52jpiM1t5oDRCLM2

coderabbitai

Actionable comments posted: 1

🤖 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 `@src/Beutl.Engine/Audio/Graph/AudioNode.cs`:
- Around line 46-50: In the Flush method of the AudioNode class, modify the
logic to throw an exception when there are multiple inputs (_inputs.Count > 1)
instead of silently returning a silent buffer. Keep the existing fallback
behavior that returns silence when there are zero inputs, but add a throw
statement for the multi-input case to force explicit merge or drain semantics in
subclasses that override Flush with multiple inputs.

🪄 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: f7b50aeb-087e-4a90-971d-93276c749796

📥 Commits

Reviewing files that changed from the base of the PR and between 70e6be8 and c7f1bda.

📒 Files selected for processing (4)

src/Beutl.Engine/Audio/Graph/AudioNode.cs
src/Beutl.Engine/Audio/Graph/Nodes/ClipNode.cs
src/Beutl.Engine/Audio/Graph/Nodes/LimiterNode.cs
tests/Beutl.UnitTests/Engine/Audio/AudioLatencyCompensationTests.cs

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c7f1bda716

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Addresses the CodeRabbit/Codex review of the Flush compensation: the default Flush bypassed the node's own processing, hardcoded stereo, dropped fan-in branches, and under-reported animated lookahead. Unify everything on a ProcessTail(input, context) seam: Process feeds it real upstream audio, Flush feeds it the drained tail, so a transforming node shapes the tail exactly like the body. The base ProcessTail is pass-through, so the zero-processing path stays byte-identical. - AudioNode: default Flush now runs the upstream drain through ProcessTail (so a downstream Equalizer/Compressor/Gain processes the recovered tail); zero-input silence matches the last processed channel count instead of hardcoded stereo; a fan-in node without a Flush override now throws rather than silently dropping tails. - Limiter/Gain/Equalizer/Compressor/Delay: Process bodies move into ProcessTail (LimiterNode's bespoke Flush is gone — it is now just a ProcessTail override). - MixerNode: overrides Flush to drain and mix every branch, recovering a lookahead tail held in any fan-in branch. - LimiterNode.GetLatencySamples: reports the worst-case lookahead when animated, so the drain reserves enough room when automation peaks near the clip end. - SourceNode records its stereo output so the flush silence matches. ClipNode's chunk-boundary-alignment tail loss (block ending exactly at the clip end) is documented as a known limitation; the only fixes are out of scope (range expansion resets the limiter; an oversized terminal buffer breaks the output contract). Refs: Project #9 "AI Review" item "Audio graph: latency reporting/compensation API for lookahead-bearing AudioEffects" Claude-Session: https://claude.ai/code/session_01PJ9eDL52jpiM1t5oDRCLM2

coderabbitai

Actionable comments posted: 1

🤖 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 `@src/Beutl.Engine/Audio/Graph/Nodes/LimiterNode.cs`:
- Around line 74-78: The drained buffer is leaking when ProcessTail throws an
exception during the flush operation. In the AudioNode.Flush method, wrap the
ProcessTail call in a try-finally block to ensure the drained buffer is properly
disposed regardless of whether ProcessTail succeeds or throws an exception. The
finally block should dispose of the drained buffer after ProcessTail completes,
whether successfully or via exception.

🪄 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: 8a860065-c4a2-4d25-b6e2-f7d65fbaa530

📥 Commits

Reviewing files that changed from the base of the PR and between c7f1bda and 5c0f588.

📒 Files selected for processing (10)

src/Beutl.Engine/Audio/Graph/AudioNode.cs
src/Beutl.Engine/Audio/Graph/Nodes/ClipNode.cs
src/Beutl.Engine/Audio/Graph/Nodes/CompressorNode.cs
src/Beutl.Engine/Audio/Graph/Nodes/DelayNode.cs
src/Beutl.Engine/Audio/Graph/Nodes/EqualizerNode.cs
src/Beutl.Engine/Audio/Graph/Nodes/GainNode.cs
src/Beutl.Engine/Audio/Graph/Nodes/LimiterNode.cs
src/Beutl.Engine/Audio/Graph/Nodes/MixerNode.cs
src/Beutl.Engine/Audio/Graph/Nodes/SourceNode.cs
tests/Beutl.UnitTests/Engine/Audio/AudioLatencyCompensationTests.cs

✅ Files skipped from review due to trivial changes (1)

src/Beutl.Engine/Audio/Graph/Nodes/EqualizerNode.cs

🚧 Files skipped from review as they are similar to previous changes (1)

src/Beutl.Engine/Audio/Graph/Nodes/ClipNode.cs

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5c0f588207

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

The single-input Flush path disposed the drained buffer only after a successful ProcessTail, leaking it on a throw. Wrap ProcessTail in try/catch and dispose the drain on failure; Dispose is idempotent, so a transforming node that already consumed the buffer is unaffected. Addresses CodeRabbit review on PR #1998. Claude-Session: https://claude.ai/code/session_01PJ9eDL52jpiM1t5oDRCLM2

LimiterNode reports the animated worst-case lookahead, but the effect-level GetLatencySamples a host queries before graph construction still returned the CurrentValue snapshot, so it could under-reserve and reintroduce the tail loss. Mirror the node: an animated Lookahead reports MaxLookaheadMs. Addresses Codex review on PR #1998. Claude-Session: https://claude.ai/code/session_01PJ9eDL52jpiM1t5oDRCLM2

coderabbitai · 2026-06-23T21:54:34Z

Caution

Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted.

Error details

{}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 583146a1e9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

A SoundGroup mixes child clips and then flushes them through the group's own clip in the group's time domain. A nested ClipNode inherited the base single-input Flush, which forwarded the parent's drain context unchanged, so the child's cached lookahead limiter saw a discontinuity, reset its delay line, and dropped the very tail being drained. Override ClipNode.Flush to rebuild the drain at clip-local Duration — where the clip's last Process slice ended — mirroring AppendFlushedTail. The parent's start is intentionally dropped, which is also why an intervening ShiftNode needs no flush override of its own. Document the remaining best-effort gaps (animated-lookahead drain, custom-leaf channel count, DelayNode latency budget) inline as known limitations / follow-ups rather than expanding the PR further.

coderabbitai

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

tests/Beutl.UnitTests/Engine/Audio/AudioLatencyCompensationTests.cs (2)
153-182: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

This test passes trivially and does not exercise AppendFlushedTail.

The window Context(TimeSpan.Zero, clipSamples) covers the clip exactly, so newBuffer.SampleCount == clipSamples, writeOffset == clipSamples, and capacity == 0 — AppendFlushedTail returns early without draining (the same capacity == 0 situation the nested-clip test explicitly documents at Lines 288-289). The asserted indices [clipSamples - L, clipSamples) fall inside the main delayed-limiter slice, which is real non-zero sine regardless of recovery, so the assertion holds even if tail recovery is a complete no-op. To actually validate the recovery path, extend the window past the clip end so trailing capacity exists and assert on [clipSamples, clipSamples + L).
💚 Proposed fix to exercise the drain path
-        // A window that exactly covers the whole clip: it reaches the clip's true end, so ClipNode
-        // drains the limiter tail into the trailing samples.
-        using var output = clip.Process(Context(TimeSpan.Zero, clipSamples));
-
-        var data = output.GetChannelData(0);
-        bool tailNonZero = false;
-        for (int i = clipSamples - L; i < clipSamples; i++)
+        // A window that extends L samples past the clip end leaves trailing room, so AppendFlushedTail
+        // drains the limiter's held tail into samples [clipSamples, clipSamples + L).
+        using var output = clip.Process(Context(TimeSpan.Zero, clipSamples + L));
+
+        var data = output.GetChannelData(0);
+        bool tailNonZero = false;
+        for (int i = clipSamples; i < clipSamples + L; i++)
         {
             if (MathF.Abs(data[i]) > 1e-5f) { tailNonZero = true; break; }
         }
🤖 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 `@tests/Beutl.UnitTests/Engine/Audio/AudioLatencyCompensationTests.cs` around
lines 153 - 182, The test ClipNode_TerminalWindow_AppendsRecoveredTail does not
actually exercise the AppendFlushedTail recovery path because the Context window
exactly covers the clip with no trailing capacity, causing AppendFlushedTail to
return early. To fix this, extend the Context window past the clip end to create
trailing capacity (so the window sample count exceeds clipSamples), and then
update the assertion loop indices to check the newly recovered tail samples in
the range beyond the original clip end (approximately clipSamples to clipSamples
+ L) instead of checking indices within the clip that already contain valid sine
data from the delayed limiter output.
185-208: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

ClipNode_ZeroLookahead_IsNoOp is tautological — it can never fail.

clipA/clipB use identical configurations and the identical Context(TimeSpan.Zero, clipSamples), and RangeSineNode is deterministic, so a[i] == b[i] always holds regardless of behavior. The comment claims the reference is "A mid-clip window (does not reach the end)," but clipB uses the same terminal window as clipA. Either make the reference genuinely bypass the drain (e.g., a window that stops short of the clip end, comparing the overlapping region) or compare against the raw source output so the assertion has discriminating power.
🤖 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 `@tests/Beutl.UnitTests/Engine/Audio/AudioLatencyCompensationTests.cs` around
lines 185 - 208, The test ClipNode_ZeroLookahead_IsNoOp is tautological because
clipA and clipB use identical configurations and both process with
Context(TimeSpan.Zero, clipSamples), so a[i] will always equal b[i] regardless
of the drain behavior. Fix this by modifying the reference (clipB) to use a
different window that genuinely represents a mid-clip region that does not reach
the end, such as Context(TimeSpan.Zero, clipSamples - someOffset), so the
overlapping region comparison can actually detect if the drain perturbs samples,
or alternatively compare against the raw sourceB output directly instead of a
processed clipB output.

🤖 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.

Outside diff comments:
In `@tests/Beutl.UnitTests/Engine/Audio/AudioLatencyCompensationTests.cs`:
- Around line 153-182: The test ClipNode_TerminalWindow_AppendsRecoveredTail
does not actually exercise the AppendFlushedTail recovery path because the
Context window exactly covers the clip with no trailing capacity, causing
AppendFlushedTail to return early. To fix this, extend the Context window past
the clip end to create trailing capacity (so the window sample count exceeds
clipSamples), and then update the assertion loop indices to check the newly
recovered tail samples in the range beyond the original clip end (approximately
clipSamples to clipSamples + L) instead of checking indices within the clip that
already contain valid sine data from the delayed limiter output.
- Around line 185-208: The test ClipNode_ZeroLookahead_IsNoOp is tautological
because clipA and clipB use identical configurations and both process with
Context(TimeSpan.Zero, clipSamples), so a[i] will always equal b[i] regardless
of the drain behavior. Fix this by modifying the reference (clipB) to use a
different window that genuinely represents a mid-clip region that does not reach
the end, such as Context(TimeSpan.Zero, clipSamples - someOffset), so the
overlapping region comparison can actually detect if the drain perturbs samples,
or alternatively compare against the raw sourceB output directly instead of a
processed clipB output.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: d48db3cc-c99a-4a33-a44d-1f11c1033a3a

📥 Commits

Reviewing files that changed from the base of the PR and between 583146a and 3ff93c5.

📒 Files selected for processing (5)

src/Beutl.Engine/Audio/Graph/AudioNode.cs
src/Beutl.Engine/Audio/Graph/Nodes/ClipNode.cs
src/Beutl.Engine/Audio/Graph/Nodes/DelayNode.cs
src/Beutl.Engine/Audio/Graph/Nodes/LimiterNode.cs
tests/Beutl.UnitTests/Engine/Audio/AudioLatencyCompensationTests.cs

🚧 Files skipped from review as they are similar to previous changes (3)

src/Beutl.Engine/Audio/Graph/Nodes/DelayNode.cs
src/Beutl.Engine/Audio/Graph/Nodes/LimiterNode.cs
src/Beutl.Engine/Audio/Graph/AudioNode.cs

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3ff93c5e26

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

ClipNode.Flush drained from the clip's natural Duration, which is wrong when a parent (a SoundGroup window) trims the clip before its own end: the cached effects last processed up to the trim boundary, so draining from Duration skips past them, trips the limiter's discontinuity guard, and drops the tail held at the boundary. Track the clip-local end of the last processed window and drain from there instead. Document the remaining best-effort gaps inline as known limitations: MixerNode.Flush drains every branch unconditionally (a child that ended early on the chunk-alignment edge can emit a stale tail late), and SceneNode reports zero latency for a referenced scene (an inner limiter tail is dropped at a SceneSound cut).

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f054212294

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

When a nested clip's terminal window had trailing pad smaller than the reported latency, AppendFlushedTail drained only part of the held tail and advanced the upstream chain past Duration, but _lastProcessedLocalEnd stayed at Duration. A later parent flush of the same child then restarted the drain at the stale time, stepping the cached effects backward, which tripped the discontinuity guard, reset them, and dropped the rest of the tail. Record the advanced drain position after the append so Flush continues from it. Document the ResampleNode flush gap (no rate-mirroring Flush override) as a known limitation; it only bites a custom graph that resamples after a latency-bearing node, since the built-in Sound chain resamples first.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ef513eb5f9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

ClipNode.Flush drained the input chain but never recorded the new clip-local end, so a parent flushing the same nested clip across multiple blocks (its tail capacity below the child's latency) replayed the drain from the old local end, tripping the cached effects' discontinuity guard and emitting the remaining tail as silence. Record drainContext.TimeRange.End after the upstream flush, mirroring AppendFlushedTail. Also harden two ClipNode tail tests that passed trivially: - TerminalWindow_AppendsRecoveredTail now extends the window past the clip end so AppendFlushedTail actually drains, asserting on the recovered [clipSamples, clipSamples+L) region instead of the main slice (which held real sine regardless of recovery). - ZeroLookahead_IsNoOp now uses a reference window that stops short of the clip end, giving the overlap comparison discriminating power.

On the Flush path the limiter ran the drained tail through ProcessAnimated, which re-samples Lookahead over the post-clip drain range. When automation holds a high lookahead through the clip end but keyframes back to 0 at Duration, the drain read delay offset 0 (the flush silence) and dropped the held tail, even though GetLatencySamples had reserved the worst-case room. Thread a `draining` flag through AudioNode.ProcessTail (Process passes false, Flush passes true). LimiterNode now retains the coefficients of the clip's terminal sample and, while draining an animated chain, reads the delay line at that frozen lookahead instead of the decayed automation value, so the held samples are recovered at the offset they were buffered. Static and non-draining paths are unchanged. The other ProcessTail overrides (Gain/EQ/Compressor/Delay) ignore the flag.

MixerNode.Flush drained every input branch unconditionally. A SoundGroup child that ended before the group's terminal slice — and whose own terminal block landed on its clip boundary (the chunk-alignment edge where ClipNode cannot self-recover) — still holds a stale tail, which the mixer then mixed into the group pad seconds late. Add per-branch clip-liveness: MixerNode.BranchEndTimes carries each branch's group-local clip end (parallel to Inputs; empty keeps the old drain-all behavior). On flush, a branch whose clip ended before the drain block is skipped; if every branch is dead the mixer flushes silence. SoundGroup populates BranchEndTimes in connect order as it wires children to the mixer.

When a sound ended exactly on a compose-window boundary, its ClipNode's terminal window was full (capacity 0), so AppendFlushedTail could not recover the limiter tail, and the next window excluded the sound entirely (it dropped out of frame.Objects), so the held tail was lost. The Composer now records the entries active each window. When a sound is active in the previous window but not the current one — and the windows are contiguous (not a seek) — it flushes that sound's cached output graph and mixes the drained tail into the start of the current window, where it belongs ([windowStart, windowStart + latency)). The drain is a no-op for sounds that already self-recovered mid-window, so it never double-emits. Adds a file-free LimiterTailSound test harness and an end-to-end test that composes a limiter-bearing sound across the boundary.

github-actions · 2026-06-25T11:01:56Z

No TODO comments were found.

github-actions · 2026-06-25T11:05:57Z

Package	Line Rate	Branch Rate	Complexity	Health
Beutl.Api	11%	5%	1170	➖
Beutl.Configuration	41%	22%	350	➖
Beutl.Controls	0%	0%	5513	➖
Beutl.Core	62%	56%	3069	➖
Beutl.Editor	78%	70%	1742	➖
Beutl.Editor.Components	1%	0%	8552	➖
Beutl.Embedding.MediaFoundation	6%	8%	1376	➖
Beutl.Engine	63%	52%	18215	➖
Beutl.Engine.SourceGenerators	59%	44%	540	➖
Beutl.Extensibility	38%	59%	112	➖
Beutl.Extensions.AVFoundation	5%	12%	246	➖
Beutl.Extensions.FFmpeg	7%	8%	861	➖
Beutl.FFmpegIpc	25%	33%	825	➖
Beutl.Language	7%	50%	1349	➖
Beutl.NodeGraph	24%	15%	2474	➖
Beutl.ProjectSystem	58%	43%	1061	➖
Beutl.Threading	100%	90%	137	✔
Beutl.Utilities	94%	87%	358	✔
Summary	35% (43951 / 124948)	31% (10966 / 35594)	47950	➖

Minimum allowed line rate is 0%

Copilot AI review requested due to automatic review settings June 23, 2026 16:37

Copilot started reviewing on behalf of yuto-trd June 23, 2026 16:37 View session

Copilot AI reviewed Jun 23, 2026

View reviewed changes

Comment thread src/Beutl.Engine/Audio/Graph/AudioNode.cs

Comment thread tests/Beutl.UnitTests/Engine/Audio/AudioLatencyTests.cs

yuto-trd added 2 commits June 24, 2026 01:48

yuto-trd changed the title ~~feat(engine): report audio effect/node latency for lookahead effects~~ feat(engine): audio effect latency reporting + lookahead tail compensation Jun 23, 2026

coderabbitai Bot reviewed Jun 23, 2026

View reviewed changes

Comment thread src/Beutl.Engine/Audio/Graph/AudioNode.cs

chatgpt-codex-connector Bot reviewed Jun 23, 2026

View reviewed changes

coderabbitai Bot reviewed Jun 23, 2026

View reviewed changes

Comment thread src/Beutl.Engine/Audio/Graph/Nodes/LimiterNode.cs

chatgpt-codex-connector Bot reviewed Jun 23, 2026

View reviewed changes

Comment thread src/Beutl.Engine/Audio/Effects/LimiterEffect.cs Outdated

yuto-trd force-pushed the yuto-trd/audio-latency-report branch from ac81c39 to b5c71df Compare June 23, 2026 21:50

chatgpt-codex-connector Bot reviewed Jun 23, 2026

View reviewed changes

Comment thread src/Beutl.Engine/Audio/Graph/Nodes/LimiterNode.cs Outdated

Comment thread src/Beutl.Engine/Audio/Graph/AudioNode.cs

Comment thread src/Beutl.Engine/Audio/Graph/AudioNode.cs

Comment thread src/Beutl.Engine/Audio/Graph/Nodes/DelayNode.cs Outdated

coderabbitai Bot reviewed Jun 24, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Jun 24, 2026

View reviewed changes

Comment thread src/Beutl.Engine/Audio/Graph/Nodes/ClipNode.cs Outdated

Comment thread src/Beutl.Engine/Audio/Graph/Nodes/MixerNode.cs

Comment thread src/Beutl.Engine/Audio/Graph/AudioNode.cs

chatgpt-codex-connector Bot reviewed Jun 25, 2026

View reviewed changes

Comment thread src/Beutl.Engine/Audio/Graph/AudioNode.cs

Comment thread src/Beutl.Engine/Audio/Graph/Nodes/ClipNode.cs

chatgpt-codex-connector Bot reviewed Jun 25, 2026

View reviewed changes

Comment thread src/Beutl.Engine/Audio/Graph/Nodes/ClipNode.cs Outdated

yuto-trd added 4 commits June 25, 2026 16:46

Uh oh!

Uh oh!

Conversation

yuto-trd commented Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Reporting

Compensation (node Flush contract)

Why a node contract, not range expansion

Affected areas

Breaking changes

Test plan

Follow-ups

Fixed issues / References

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

❌ Failed checks (1 warning)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

coderabbitai Bot commented Jun 23, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

yuto-trd commented Jun 23, 2026 •

edited

Loading

Compensation (node `Flush` contract)

coderabbitai Bot commented Jun 23, 2026 •

edited

Loading