fix: audio dac (enters a loop while playing trims)

[3djc]

In some circumstances, the audio can enter in 'loop' state, as show in video

Video_1781608013802_110.mp4

This fixes it.

While I have bot been able to reproduce on main, I would think main needs it too (I have done PR on 2.12 since I could reproduce issue and confirm fix)

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: def8226d-e8d2-42f6-93ba-d548ad6e23ce

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch 3djc/fix-h7-audiodac

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[richardclli]

Is this the same problem of PR #7274 ?

[pfeerick]

Is this the same problem of PR #7274 ?

I believe so, yes - or at least the PA01 and v12 have shown exactly same symptoms for the past several months.

[richardclli]

seems both fixes are necessary, and these 2 are for dma underrun in H7 radios, I am tracking similar problem in F4 radios, which is a regression bug from 2.10 -> 2.11

[richardclli]

My AI coder gives this comments for both PRs:

H7 DAC Audio DMA — PR #7274 vs PR #7463 Comparison

Context

Both PRs target the H5/H7/H7RS circular double-buffer DMA path in radio/src/targets/common/arm/stm32/audio_dac_driver.cpp. The H7 architecture uses a single _dma_buffer[640] split into two 320-sample halves, with DMA running in continuous circular mode. HT (Half Transfer) and TC (Transfer Complete) interrupts refill each half.

Neither PR affects the F4 path (PL18, Taranis, etc.) — they are H7-only fixes.

PR #7274 — "chore(radio): audio related DMA cleanup" (SchrodiL)

Problem solved

DMA thrashing: the old code stops DMA immediately when a single buffer half runs empty, then the audio task restarts it on the next cycle. This causes rapid stop/start jitter and audio gaps during brief buffer starvation (e.g., a flash page read that takes 1-2ms).

Solution

Adds a starvation hysteresis counter _empty_dma_halves with threshold DMA_EMPTY_HALVES_STOP_THRESHOLD = 6:

• Each half-transfer that finds no data increments the counter
• Each half-transfer that finds data resets the counter to 0
• DMA is only stopped after 6 consecutive empty halves (~30ms of silence at 32kHz)
• This absorbs transient underruns without stopping DMA

Additional changes

• dac_clear_dma_flags() helper to flush stale HT/TC flags before enabling DMA
• EN-bit busy-wait in dac_close_dma_xfer() to ensure hardware has fully disabled before flag check
• Flag clearing in dac_start_dma() before enabling interrupts

Tradeoff

During the hysteresis window, if the audio producer is genuinely starved for >30ms, the circular DMA replays stale data from the other half-buffer for up to 30ms before stopping. This means a flash erase (45-400ms) still eventually stops cleanly, but there's a brief stale-audio period.

PR #7463 — "fix: audio dac (enters a loop while playing trims)" (3djc)

Problem solved

When DMA is stopped mid-transfer and restarted, the hardware registers M0AR (memory address) and NDTR (remaining transfer count) hold partial values — they point partway through _dma_buffer. On restart, HT fires at the wrong offset relative to the actual buffer halves, causing half-tracking to desync. This manifests as trim sounds looping endlessly.

Solution

• dac_start_dma() now explicitly resets M0AR back to _dma_buffer[0] and NDTR to DMA_BUFFER_LEN on every restart
• audioConsumeCurrentBuffer() primes both halves before starting DMA (audio_update_dma_buffer(0) + audio_update_dma_buffer(1)), so the first full cycle runs valid data with correct half alignment
• DMA flags are cleared after stream disable to prevent carry-over

Additional changes

• dac_close_dma_xfer() clears pending HT/TC flags (resolves the // TODO: reset flags comment)

Side-by-side comparison

	PR #7274 (SchrodiL)	PR #7463 (3djc)
Nature	Preventative improvement	Bug fix
Fixes a confirmed bug?	No	Yes (trim sound loop)
Reproduced?	Design improvement	Confirmed with video
Core mechanism	_empty_dma_halves counter + threshold = 6	M0AR/NDTR reset + double-half prime
Effect on underrun	Absorbs brief lags (≤30ms) without stopping	No change to underrun detection
Effect on restart	None	Ensures clean half-tracking after restart
Stale audio during underrun?	Up to 30ms of stale replay	No (stop/restart is immediate)
Helps flash contention?	Yes — a 1ms flash read won't stop audio	No
DMA flag fix	dac_clear_dma_flags() helper + EN-bit wait	Direct flag clear in close + start
Conflict with other PR	Overlaps on dac_close_dma_xfer() and dac_start_dma()	Same overlap

Behavioral trace

Scenario: brief buffer lag (1-3 empty halves, e.g. flash page read)

  Time:   0ms     10ms     20ms     30ms     40ms
          │        │         │        │        │
  BOTH:   ◄── data ──► ◄── empty (x2) ──► ◄── data ──►
          audio plays     silent halves      recovers

  current main:
    ◄── data ──► [xfer ends] stop DMA [restart] ◄── data ──►
                  └── click/gap ──┘

  PR #7463 only:
    ◄── data ──► [xfer ends] stop DMA [restart cleanly] ◄── data
                  └── click/gap ──┘
        (same stop/start as main, but no desync on restart)

  PR #7274 only:
    ◄── data ──► [empty x2, _empty=2 < 6, keep running]
                 └── plays stale data from other half ──┘
                  └──→ data resumes, DMA never stopped

Scenario: long starvation (flash erase, 200ms)

  Time:   0ms     30ms     200ms
          │        │         │
  current main:
    ◄── data [stop] ◄── silence ◄── restart, recover
               ╰── 200ms silence ──╯

  PR #7463 only:
    ◄── data [stop] ◄── silence ◄── restart cleanly, recover
               ╰── 200ms silence ──╯
    (same as main but safe restart)

  PR #7274 only:
    ◄── data [empty x2] ◄── stale replay (30ms) [stop] ◄── silence [recover]
                    ╰── 30ms stale ╰── 170ms silence ──╯

Verdict

	PR #7274	PR #7463
Priority	Nice to have	Must have
Apply order	Second (after #7463)	First
Criticality	Quality improvement	Fixes confirmed bug
Merge advice	Apply both, resolve flag-clear overlap	Apply both, resolve flag-clear overlap
Effect on PL18/F4	None (H7 only)	None (H7 only)

Conclusion: They are complementary, not competing. PR #7463 ensures DMA never desyncs on restart. PR #7274 prevents unnecessary restarts in the first place through hysteresis. Both should be applied — #7463 first for the bug fix, then #7274 merged on top with the dac_close_dma_xfer() / dac_start_dma() overlap resolved.

The merged result would give:

1. No thrashing on brief underruns (PR chore(radio): audio related DMA cleanup #7274)
2. Clean restart with correct half-tracking after any stop (PR fix: audio dac (enters a loop while playing trims) #7463)
3. Double-primed buffer halves for glitch-free start (PR fix: audio dac (enters a loop while playing trims) #7463)
4. Robust DMA flag management on both start and stop (both)

[pfeerick]

Thanks for the breakdown / comparison. Obviously the AI review is incomplete / incorrect in thinking #7274 is cleanup only / not fixing the trim audio bug (#7227) , as that wasn't mentioned in the PR description itself, but they clearly target different aspects of the same issue.