diff --git a/prep_wake_samples.py b/prep_wake_samples.py index 527f9f6..e99bf77 100644 --- a/prep_wake_samples.py +++ b/prep_wake_samples.py @@ -28,6 +28,7 @@ import subprocess import sys import tempfile +from collections import Counter from math import gcd from pathlib import Path @@ -92,6 +93,31 @@ def _write_clip(out_dir: Path, stem: str, idx: int, audio: np.ndarray) -> Path: return path +def _write_unique( + out_dir: Path, stem: str, idx: int, audio: np.ndarray, seen: set[int] +) -> Path: + """Write a clip, refusing to clobber one this same run already wrote. + + Rerunning the script into a populated dir is a supported refresh, so a + leftover from a *prior* run is fine to overwrite. What must never happen is + two inputs in *one* run mapping to the same file and the second silently + replacing the first. `seen` holds the inode of every clip written this run; + the filesystem resolves a case-insensitive collision (a/Computah.wav and + b/computah.wav) to that same inode, so this catches it where a path-string + compare would not, and stays quiet on a case-sensitive FS where the two are + genuinely different files. + """ + path = out_dir / f"{stem}_{idx:03d}.wav" + if path.exists() and path.stat().st_ino in seen: + raise FileExistsError( + f"two inputs map to {path.name} in one run " + f"(same basename, or a case-insensitive filesystem collision)" + ) + _write_clip(out_dir, stem, idx, audio) + seen.add(path.stat().st_ino) + return path + + # --------------------------------------------------------------------------- # # Silence-based segmentation # --------------------------------------------------------------------------- # @@ -206,6 +232,83 @@ class — pointing a whole mixed folder at --label positive would mislabel the return uniq +def _unique_stems(files: list[Path]) -> list[str]: + """A distinct output stem per input file, in input order. + + Clip names are derived from the input stem, so two inputs that share a + basename (a/computah.wav and b/computah.wav) would write the same clip + names and the second file would silently overwrite the first. A stem that + is already unique across the inputs is kept as-is. Within a colliding group + one file keeps the bare stem and the rest get the lowest numeric suffix not + otherwise taken. Every input stem is reserved up front so a disambiguated + "computah-1" can never land on a real "computah-1" input -- including one + that is itself part of another colliding group. + + One member of the group keeps the bare stem so that refreshing a samples dir + stays a refresh: adding a second computah.wav renames only the new file and + leaves the original's clips to be overwritten in place. Suffixing every + member instead would orphan the previous run's computah_###.wav, and + eval_wake_threshold reads every clip in the directory, so those orphans would + quietly train and score as duplicates. + + Which member keeps the bare stem is decided by resolved path, not by input + order, so the mapping depends only on WHICH files are present. Passing the + same two files in the other order must not move the bare stem to the other + file -- that would strand the first run's clips just as surely. + + A clip's final name is "-N_.wav" when disambiguated (the "-N" + from here, the "_" segment index from _write_clip) or "_.wav" + when the stem was already unique. + """ + counts = Counter(f.stem for f in files) + used = {f.stem for f in files} + assigned: dict[int, str] = {} + groups: dict[str, list[int]] = {} + for i, f in enumerate(files): + if counts[f.stem] == 1: + assigned[i] = f.stem + else: + groups.setdefault(f.stem, []).append(i) + + # Sorted group order, and resolved-path order within a group, so every + # assignment below is a function of the input set alone. + for stem, idxs in sorted(groups.items()): + ranked = sorted(idxs, key=lambda j: str(files[j].resolve())) + for rank, i in enumerate(ranked): + if rank == 0: + assigned[i] = stem + continue + n = 1 + while f"{stem}-{n}" in used: + n += 1 + name = f"{stem}-{n}" + used.add(name) + assigned[i] = name + return [assigned[i] for i in range(len(files))] + + +def _stale_clips(out_dir: Path, written: set[Path]) -> list[Path]: + """Audio files in `out_dir` this run did not write, sorted; empty if absent. + + Mirrors `eval_wake_threshold._clips()`, precondition included: a directory + that does not exist holds nothing. The precondition lives here, with the + read, because the earlier inline copy dropped it and a run that writes no + clips never creates out_dir (only `_write_clip` does), so the scan died on + a missing directory. + + The mirror is close but not exact: this module's `AUDIO_EXTS` is the wider + set, so the warning can name a file eval would skip. #82 tracks giving both + modules one definition instead of two copies. + """ + if not out_dir.is_dir(): + return [] + return sorted( + p + for p in out_dir.iterdir() + if p.suffix.lower() in AUDIO_EXTS and p not in written + ) + + def process( inputs: list[Path], out_dir: Path, @@ -215,35 +318,66 @@ def process( max_dur_s: float, ) -> int: """Process every input file; return the total clip count written.""" + # Checked before any audio is decoded, so an occupied --output is named once + # rather than surfacing as whichever exception the run happens to reach first: + # mkdir() raises FileExistsError once a clip is ready to write, and a run that + # writes nothing gets as far as the stale scan below. + if out_dir.exists() and not out_dir.is_dir(): + print( + f"error: --output {out_dir} exists and is not a directory; " + f"point --output at a directory, or move that file", + file=sys.stderr, + ) + return 0 + files = _inputs(inputs) if not files: joined = ", ".join(str(p) for p in inputs) print(f"no audio files found at {joined}", file=sys.stderr) return 0 - total = 0 + stems = _unique_stems(files) + written: set[Path] = set() + seen_inodes: set[int] = set() all_durs: list[float] = [] - for f in files: + for f, stem in zip(files, stems): audio = load_mono_16k(f) dur = len(audio) / TARGET_SR if label == "background": # Continuous negative audio: keep it whole, just normalized. - _write_clip(out_dir, f.stem, 0, audio) - total += 1 + written.add(_write_unique(out_dir, stem, 0, audio, seen_inodes)) print(f" {f.name}: {dur:.1f}s background -> 1 file") continue clips, stats = segment_on_silence(audio, min_gap_s, min_dur_s, max_dur_s) for i, clip in enumerate(clips): - _write_clip(out_dir, f.stem, i, clip) + written.add(_write_unique(out_dir, stem, i, clip, seen_inodes)) all_durs.append(len(clip) / TARGET_SR) - total += stats["kept"] note = "" if stats["too_short"] or stats["too_long"]: note = f" (dropped {stats['too_short']} short, {stats['too_long']} long)" print(f" {f.name}: {dur:.1f}s -> {stats['kept']} clips{note}") + # Count files actually on disk, not segments requested, so the report is + # honest even if two inputs ever resolve to the same clip name. + total = len(written) print(f"\nwrote {total} {label} clip(s) to {out_dir}") + + # Clips this run did not write are left from an earlier run over a different + # input set. Training and eval read every clip in the directory, so they + # would score as extra data nobody asked for. Deleting them here would be + # too eager (the dir is the user's), so say so and name the fix. + stale = _stale_clips(out_dir, written) + if stale: + shown = ", ".join(p.name for p in stale[:3]) + if len(stale) > 3: + shown += f", +{len(stale) - 3} more" + print( + f"warning: {len(stale)} clip(s) in {out_dir} are left from an earlier " + f"run ({shown}). Training and eval read every clip in this directory, " + f"so delete them or clear {out_dir} and rerun.", + file=sys.stderr, + ) if all_durs: a = np.array(all_durs) print( diff --git a/test_prep_wake_samples.py b/test_prep_wake_samples.py index 0e2b191..d852d31 100644 --- a/test_prep_wake_samples.py +++ b/test_prep_wake_samples.py @@ -143,6 +143,237 @@ def test_empty_input_no_crash(d: Path) -> None: check(total == 0, f"empty input returns 0 without crashing (got {total})") +def test_zero_clips_no_crash(d: Path) -> None: + """Audio that yields no clips must report cleanly, not raise. + + Distinct from test_empty_input_no_crash: the input file IS found, so this + runs past the no-files early return and reaches the stale-clip scan. Only + _write_clip() creates out_dir, so a run that writes nothing never creates + it and the scan reads a directory that is not there. + """ + audio = make_bursts(n=3, burst_s=0.6, gap_s=1.0, sr=prep.TARGET_SR) + src = d / "all_dropped.wav" + sf.write(src, audio, prep.TARGET_SR, subtype="PCM_16") + out = d / "out_dropped" + # A --min-dur longer than every burst, so all three segments drop as short. + total = prep.process([src], out, "positive", 0.3, 5.0, 10.0) + check(total == 0, f"all-dropped input returns 0 without crashing (got {total})") + check(not out.exists(), "a run that writes no clips leaves no output dir") + + +def test_output_path_not_a_directory(d: Path) -> None: + """An --output naming an existing file reports it, rather than raising. + + The mistake used to surface as one of two exceptions depending on whether a + clip was ready to write (FileExistsError from mkdir) or not + (NotADirectoryError from the stale scan), both after decoding every input. + """ + audio = make_bursts(n=2, burst_s=0.6, gap_s=1.0, sr=prep.TARGET_SR) + src = d / "into_a_file.wav" + sf.write(src, audio, prep.TARGET_SR, subtype="PCM_16") + occupied = d / "not_a_dir" + occupied.write_text("keep me") + total = prep.process([src], occupied, "positive", 0.3, 0.2, 3.0) + check(total == 0, f"an occupied --output returns 0, not a traceback (got {total})") + check(occupied.read_text() == "keep me", "the file at --output is left untouched") + + +def test_duplicate_basename_no_overwrite(d: Path) -> None: + """Two inputs sharing a basename must not overwrite each other's clips. + + Clip names come from the input stem, so a/computah.wav and b/computah.wav + once wrote the same names and the second file silently clobbered the first + while the reported total still counted both. The two files carry different + burst counts (2 and 3) so any overwrite shows up as fewer files than clips. + """ + a = d / "a" + b = d / "b" + a.mkdir() + b.mkdir() + sf.write( + a / "computah.wav", + make_bursts(2, 0.5, 0.8, prep.TARGET_SR), + prep.TARGET_SR, + subtype="PCM_16", + ) + sf.write( + b / "computah.wav", + make_bursts(3, 0.5, 0.8, prep.TARGET_SR), + prep.TARGET_SR, + subtype="PCM_16", + ) + + out = d / "dup_positive" + total = prep.process( + [a / "computah.wav", b / "computah.wav"], out, "positive", 0.3, 0.2, 3.0 + ) + files = sorted(out.glob("*.wav")) + check( + total == 5 and len(files) == 5, + f"5 clips from two same-basename inputs, none overwritten " + f"(total={total}, files={len(files)})", + ) + check( + len({f.name for f in files}) == len(files), + f"every clip name is distinct ({[f.name for f in files]})", + ) + + # The background path derives names the same way, so it shares the bug. + out_bg = d / "dup_background" + total_bg = prep.process( + [a / "computah.wav", b / "computah.wav"], out_bg, "background", 0.3, 0.2, 3.0 + ) + bg_files = sorted(out_bg.glob("*.wav")) + check( + total_bg == 2 and len(bg_files) == 2, + f"two same-basename background files stay two files " + f"(total={total_bg}, files={len(bg_files)})", + ) + + +def test_rerun_refreshes_populated_dir(d: Path) -> None: + """Rerunning into a dir that already holds a prior run's clips must work. + + The docs tell people to run prep_wake_samples straight into samples/, so a + refresh overwrites last run's clips rather than aborting. Running process + twice into the same out_dir must succeed both times and leave the same file + count -- a prior-run leftover is an intended overwrite, not a collision. + """ + src = d / "rerun_src" + src.mkdir() + sf.write( + src / "take.wav", + make_bursts(3, 0.5, 0.8, prep.TARGET_SR), + prep.TARGET_SR, + subtype="PCM_16", + ) + out = d / "rerun_out" + first = prep.process([src / "take.wav"], out, "positive", 0.3, 0.2, 3.0) + second = prep.process([src / "take.wav"], out, "positive", 0.3, 0.2, 3.0) + files = sorted(out.glob("*.wav")) + check( + first == second == 3 and len(files) == 3, + f"rerun into a populated dir refreshes cleanly " + f"(first={first}, second={second}, files={len(files)})", + ) + + +def test_in_run_collision_raises(d: Path) -> None: + """Two clips in one run mapping to the same file must fail loud. + + _unique_stems keeps in-run basenames distinct, but this is the backstop for + a case-insensitive filesystem where distinct-cased stems resolve to one + inode. Writing the same name twice with a shared seen-set (as one run would) + must raise instead of silently dropping the first clip. + """ + out = d / "in_run_collision" + audio = make_bursts(1, 0.5, 0.8, prep.TARGET_SR) + seen: set[int] = set() + first = prep._write_unique(out, "dup", 0, audio, seen) + check(first.exists(), f"first clip of the run lands on disk ({first.name})") + try: + prep._write_unique(out, "dup", 0, audio, seen) + except FileExistsError: + check(True, "a second clip mapping to the same file in one run raises") + else: + check(False, "an in-run collision should have raised FileExistsError") + + +def test_refresh_after_adding_collider_leaves_no_orphans(d: Path) -> None: + """Adding a second same-basename input must not orphan the first run's clips. + + Run one input, then rerun with a second file of the same basename into the + same dir. If every colliding input got suffixed, the first run's + computah_###.wav would be stranded next to an identical computah-1_###.wav -- + and eval_wake_threshold._clips() reads every clip in the directory, so the + stale pair would silently double-count. The first input keeps the bare stem, + so its clips are overwritten in place and only the new file is suffixed. + """ + a = d / "refresh_a" + b = d / "refresh_b" + a.mkdir() + b.mkdir() + sf.write( + a / "computah.wav", + make_bursts(2, 0.5, 0.8, prep.TARGET_SR), + prep.TARGET_SR, + subtype="PCM_16", + ) + sf.write( + b / "computah.wav", + make_bursts(3, 0.5, 0.8, prep.TARGET_SR), + prep.TARGET_SR, + subtype="PCM_16", + ) + + out = d / "refresh_out" + prep.process([a / "computah.wav"], out, "positive", 0.3, 0.2, 3.0) + prep.process( + [a / "computah.wav", b / "computah.wav"], out, "positive", 0.3, 0.2, 3.0 + ) + + files = sorted(p.name for p in out.glob("*.wav")) + # 2 clips from a (bare stem, overwritten in place) + 3 from b (suffixed). + check( + len(files) == 5, + f"refresh with an added collider leaves no orphaned clips ({files})", + ) + check( + any(n.startswith("computah_") for n in files), + f"the first colliding input keeps the bare stem ({files})", + ) + + +def test_stem_assignment_ignores_input_order(d: Path) -> None: + """The same colliding files must map to the same stems in any order. + + If the bare stem went to whichever file was listed first, rerunning with the + inputs reordered would hand it to the other file: the previous run's + computah_###.wav would stop being rewritten and would linger as a duplicate + that eval_wake_threshold still reads. Ranking by resolved path makes the + mapping a function of which files are present, not how they were passed. + """ + a = d / "order_a" + b = d / "order_b" + a.mkdir() + b.mkdir() + for folder in (a, b): + sf.write( + folder / "computah.wav", + make_bursts(2, 0.5, 0.8, prep.TARGET_SR), + prep.TARGET_SR, + subtype="PCM_16", + ) + + forward = prep._unique_stems([a / "computah.wav", b / "computah.wav"]) + reverse = prep._unique_stems([b / "computah.wav", a / "computah.wav"]) + # Same file -> same stem regardless of position, so reverse is forward flipped. + check( + forward == list(reversed(reverse)), + f"stem assignment is order-independent (forward={forward}, reverse={reverse})", + ) + check( + sorted(forward) == ["computah", "computah-1"], + f"one file keeps the bare stem, the other is suffixed ({sorted(forward)})", + ) + + # Two colliding groups where one group's bare stem is the name the other + # group's suffix search wants. Reserving only singletons hands "computah-1" + # to both, and the run later dies on a bogus same-basename FileExistsError. + two_groups = prep._unique_stems( + [ + a / "computah.wav", + b / "computah.wav", + a / "computah-1.wav", + b / "computah-1.wav", + ] + ) + check( + len(set(two_groups)) == len(two_groups), + f"overlapping collider groups still get distinct stems ({two_groups})", + ) + + def main() -> int: test_segment_count() with tempfile.TemporaryDirectory(prefix="prep-wake-") as tmp: @@ -152,6 +383,13 @@ def main() -> int: test_background_kept_whole(d) test_explicit_files_only(d) test_empty_input_no_crash(d) + test_zero_clips_no_crash(d) + test_output_path_not_a_directory(d) + test_duplicate_basename_no_overwrite(d) + test_rerun_refreshes_populated_dir(d) + test_in_run_collision_raises(d) + test_refresh_after_adding_collider_leaves_no_orphans(d) + test_stem_assignment_ignores_input_order(d) n_pass = sum(1 for ok, _ in results if ok) print(f"=== {n_pass}/{len(results)} checks passed ===") return 0 if n_pass == len(results) else 1