Skip to content

WavWriter: fix SaveFile final flushing + add comprehensive unit test coverage#696

Open
chattob wants to merge 7 commits intoelectro-smith:masterfrom
chattob:fix/wavwriter-savefile-tail
Open

WavWriter: fix SaveFile final flushing + add comprehensive unit test coverage#696
chattob wants to merge 7 commits intoelectro-smith:masterfrom
chattob:fix/wavwriter-savefile-tail

Conversation

@chattob
Copy link

@chattob chattob commented Mar 17, 2026

Summary

This PR fixes WavWriter finalization bugs and adds comprehensive unit tests.

What Was Fixed

SaveFile() final flush logic at buffer boundaries

On master, SaveFile() does not flush any pending full half-buffer before finalizing. Instead, it writes wptr_ * bytes_per_sample bytes starting at the beginning of the transfer buffer.

That is only correct while recording is still in the first half of the buffer. Once recording has advanced into the second half the final WAV tail repeats earlier samples from the first half.

The fix changes finalization to:

  • flush any pending full half-buffer first
  • then write only the true remaining tail
  • select that tail from the correct half of the buffer
  • handle the exact-half-boundary case without writing the same half twice

Uninitialized writer state

Init() and OpenFile() now reset the writer’s internal state and clear the transfer buffer before a new recording starts.
This is more a preventive measure.

Missing WAV type dependency in header

src/util/WavWriter.h now includes util/wav_format.h, so WAV_FormatTypeDef and the WAV constants are always available.

Test Coverage Added

  • Header/size and empty-file behavior
  • Length accessor consistency with actual saved payload

Save-path state transitions

  • partial first-half tail
  • pending FLUSH0
  • FLUSH0 + second-half tail
  • pending FLUSH1
  • FLUSH1 + new first-half tail

Representative format cases

  • 32-bit mono + stereo
  • 16-bit mono
  • 16-bit stereo crossing from the first half into the second half

Validation

  • Built and ran util_WavWriter tests via CMake/CTest. The new tests expose the old finalization regressions and pass with the fix.
  • Tested on a real application. Did not encounter bugs anymore.

@github-actions
Copy link

Test Results

163 tests  +13   163 ✅ +13   0s ⏱️ ±0s
  1 suites ± 0     0 💤 ± 0 
  1 files   ± 0     0 ❌ ± 0 

Results for commit ccfcc99. ± Comparison against base commit 9498417.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant