Skip to content

Fix wfdb_to_wav failing for every record - #632

Open
jkkronk wants to merge 2 commits into
MIT-LCP:mainfrom
jkkronk:fix/wfdb-to-wav
Open

jkkronk wants to merge 2 commits into
MIT-LCP:mainfrom
jkkronk:fix/wfdb-to-wav

Conversation

@jkkronk

@jkkronk jkkronk commented Nov 14, 2026

Copy link
Copy Markdown

wfdb_to_wav fails for every record (#410):

>>> from wfdb.io.convert.wav import wfdb_to_wav
>>> wfdb_to_wav("sample-data/100")
AttributeError: 'NoneType' object has no attribute 'astype'

As diagnosed in the issue, it reads the record with smooth_frames=False and then calls record.adc(), which converts p_signal. Since 4.0.0, rdrecord with smooth_frames=False returns the expanded samples in e_p_signal and leaves p_signal as None. This PR reads the record with smooth_frames=True, which is also what a .wav file needs: one sample per frame for each signal.

That makes the write_header=True code reachable, so the second commit fixes a typo there (samps_per_fram instead of samps_per_frame): for a record with more than one sample per frame, the header kept the input's samples per frame, and the converted record could not be read back.

The tests convert sample-data/100 (11-bit) and sample-data/test01_00s_frame (two samples per frame), read the .wav file with the standard library's wave module and compare it with (d_signal - adc_zero) << (16 - adc_res), the rule mit2wav uses. A second test reads back the record written with write_header=True. Both fail on main and pass here. For signals with more than one sample per frame, the frame means come from rdrecord, which truncates them (#335), so they can differ by one unit from mit2wav built with WFDB 10.7.

The silent file mentioned in the issue

The reporter's .wav file had no sound after changing this line. This PR does not change that, so it says "Refs" rather than "Fixes". With a resolution of 0 in the header the shift is 16 bits, and above 16 bits it is negative, which NumPy evaluates to 0. Either way every sample is 0, and the bits-per-sample field of the .wav header is set to that resolution although two bytes are written per sample. Reproduced with:

  • a record written with wrsamp(..., fmt=["32"]): all samples are 0, and the header says 32 bits per sample
  • sample-data/flacformats: the 24-bit signal is all 0
  • sample-data/v102s (resolution 0): all samples are 0, and the header says 0 bits per sample, which wave rejects

mit2wav behaves the same: the WFDB library keeps an explicit resolution of 0 (it only fills in a default when the field is missing, although header(5) says that 0 also means the 12-bit default), and a negative shift is undefined in C. So there is no behaviour to copy, and a fix needs a decision: scale the samples down to 16 bits, assume a resolution, or raise an error. I could not download the reporter's VOICED records here, so I don't know which case they hit. For 9 to 16 bits the output follows the rule above exactly in the sample records I checked; with 8 bits the samples are right too, but the header says 8 bits per sample, so wave and libsndfile read each byte as a sample.

Also left alone:

  • channels is never passed to rdrecord.
  • With write_header=True, the header is written to the current directory under the input record's name, so next to the input record it replaces the input header (mit2wav refuses this). It also names the signal file <record>.wav even if output_filename is different, and it keeps the checksums of the input record, which do not match the shifted samples.
  • Multi-segment records, records without an ADC resolution and records with a non-integer sampling frequency still fail, for other reasons.

uv run pytest -m "not network" (350 passed), ruff format --check, ruff check and towncrier build --draft pass. A changelog entry is included.

Refs #410

jkkronk and others added 2 commits November 13, 2026 21:04
wfdb_to_wav read the record with smooth_frames=False and then called
record.adc(), which converts p_signal. Since version 4.0.0, rdrecord
with smooth_frames=False always returns the expanded samples in
e_p_signal and leaves p_signal unset, so the conversion failed for
every record (MIT-LCP#410).

Read the record with smooth_frames=True instead, as suggested in the
issue. A .wav file holds one sample per frame for each signal, so this
is also what is needed for signals with more than one sample per frame.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CaYn6ePikBune9dYf3oiPf
With write_header=True, wfdb_to_wav set samps_per_fram instead of
samps_per_frame, so the header kept the samples per frame of the input
record. Now that a record with more than one sample per frame can be
converted, its header described the .wav file wrongly, and reading the
converted record back failed with "Samples were not loaded correctly".
The .wav file holds one sample per frame for each signal, which is also
what mit2wav writes in its header.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CaYn6ePikBune9dYf3oiPf
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