Fix 6 bugs: shared stdin reader, CDDB multiline, type annotation, crash fixes

- ripper: replace per-call stdin daemon threads with a shared module-level
  reader (_stdin_queue + _read_line), preventing orphan threads from stealing
  stdin input after photo uploads; all 8 input() calls in interactive_rip
  now use _read_line()
- ripper: _stream_abcde return type annotation fixed (2-tuple → 3-tuple)
- ripper: disc retry rejection now breaks gracefully instead of raising
  unhandled RuntimeError that crashed the program
- ripper: int() on disc number input wrapped in try/except
- cddb: multi-line DTITLE/TTITLE values are now concatenated instead of
  only keeping the last line (per CDDB/xmcd protocol spec)
- cli: removed unreachable dead code block in apply command
- scanner_server: upload form auto-resets after 3s for repeated uploads
- tests: _scanner_patches() updated to mock _read_line alongside
  _input_or_scan (225 tests passing)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Dieter Schlüter 2026-02-19 19:03:06 +01:00
commit 8c25bc65be
5 changed files with 100 additions and 40 deletions

View file

@ -147,7 +147,7 @@ def _read_gnudb(category: str, discid: str) -> CddbResult | None:
if line.startswith("#") or line == ".": if line.startswith("#") or line == ".":
continue continue
if line.startswith("DTITLE="): if line.startswith("DTITLE="):
dtitle = line[7:].strip() dtitle += line[7:].strip()
elif line.startswith("DYEAR="): elif line.startswith("DYEAR="):
dyear = line[6:].strip() dyear = line[6:].strip()
elif line.startswith("DGENRE="): elif line.startswith("DGENRE="):
@ -155,7 +155,11 @@ def _read_gnudb(category: str, discid: str) -> CddbResult | None:
elif line.startswith("TTITLE"): elif line.startswith("TTITLE"):
eq = line.index("=") eq = line.index("=")
idx = int(line[6:eq]) idx = int(line[6:eq])
ttitles[idx] = line[eq + 1:].strip() # Multi-Line: TTITLE0=Part1\nTTITLE0=Part2 → verketten
if idx in ttitles:
ttitles[idx] += line[eq + 1:].strip()
else:
ttitles[idx] = line[eq + 1:].strip()
# Künstler und Album aus "Artist / Title" extrahieren # Künstler und Album aus "Artist / Title" extrahieren
album_artist = "" album_artist = ""

View file

@ -293,10 +293,6 @@ def apply(
if output_dir is None: if output_dir is None:
in_place = True in_place = True
if not in_place and output_dir is None:
typer.echo("Fehler: output_dir oder --in-place angeben.", err=True)
raise typer.Exit(1)
# Prüfe Track-Anzahl pro Disc # Prüfe Track-Anzahl pro Disc
_check_disc_counts_or_exit(album, input_dir, album_json) _check_disc_counts_or_exit(album, input_dir, album_json)

View file

@ -144,32 +144,71 @@ def _get_vision_result(
return None return None
# ---------------------------------------------------------------------------
# Shared stdin reader — ein einziger Thread liest von stdin, alle Aufrufe
# von _read_line() und _input_or_scan() nutzen dieselbe Queue. Damit gibt
# es keine verwaisten Threads, die nach einem Foto-Upload weiterhin auf
# sys.stdin blockieren und nachfolgende Eingaben "stehlen".
# ---------------------------------------------------------------------------
_stdin_queue: _queue_module.Queue[str] = _queue_module.Queue()
_stdin_reader_lock = threading.Lock()
_stdin_reader_started = False
def _ensure_stdin_reader() -> None:
"""Startet den gemeinsamen stdin-Reader-Thread (einmalig, idempotent)."""
global _stdin_reader_started
with _stdin_reader_lock:
if _stdin_reader_started:
return
_stdin_reader_started = True
def _reader() -> None:
while True:
try:
line = sys.stdin.readline()
if not line: # EOF
_stdin_queue.put("")
break
_stdin_queue.put(line.rstrip("\n"))
except (EOFError, OSError):
_stdin_queue.put("")
break
threading.Thread(target=_reader, daemon=True).start()
def _read_line(prompt: str = "") -> str:
"""Liest eine Zeile von stdin über den Shared Reader.
Ersetzt input() überall in interactive_rip, damit keine konkurrierenden
Threads auf stdin warten.
"""
_ensure_stdin_reader()
if prompt:
print(prompt, end="", flush=True)
return _stdin_queue.get()
def _input_or_scan( def _input_or_scan(
prompt: str, prompt: str,
scanner: ScannerServer | None, scanner: ScannerServer | None,
) -> tuple[str, Path | None]: ) -> tuple[str, Path | None]:
"""Kombiniertes input() + Scanner-Queue: wartet gleichzeitig auf Tastatur und Foto. """Kombiniertes stdin + Scanner-Queue: wartet gleichzeitig auf Tastatur und Foto.
Nutzt den Shared stdin-Reader kein eigener Thread pro Aufruf.
Returns: Returns:
(eingegebener Text, None) wenn der User Enter drückt (eingegebener Text, None) wenn der User Enter drückt
("", photo_path) wenn ein Foto hochgeladen wird ("", photo_path) wenn ein Foto hochgeladen wird
""" """
if scanner is None: if scanner is None:
return _clean_input(input(prompt)), None return _clean_input(_read_line(prompt)), None
_ensure_stdin_reader()
print(prompt, end="", flush=True) print(prompt, end="", flush=True)
stdin_q: _queue_module.Queue = _queue_module.Queue()
def _read_stdin() -> None:
try:
val = sys.stdin.readline().rstrip("\n")
except EOFError:
val = ""
stdin_q.put(val)
threading.Thread(target=_read_stdin, daemon=True).start()
while True: while True:
photo = scanner.get_photo(timeout=0) photo = scanner.get_photo(timeout=0)
if photo is not None: if photo is not None:
@ -177,7 +216,7 @@ def _input_or_scan(
return "", photo return "", photo
try: try:
val = stdin_q.get(timeout=0.1) val = _stdin_queue.get(timeout=0.1)
return _clean_input(val), None return _clean_input(val), None
except _queue_module.Empty: except _queue_module.Empty:
continue continue
@ -252,7 +291,7 @@ def _parse_grab_tracks(grab_data: list[tuple[int, str]]) -> list[TrackInfo]:
def _stream_abcde( def _stream_abcde(
process: subprocess.Popen, process: subprocess.Popen,
use_cddb: bool, use_cddb: bool,
) -> tuple[list[TrackInfo] | None, int]: ) -> tuple[list[TrackInfo] | None, str, int]:
"""Stream abcde output live, show meaningful progress, collect CDDB data. """Stream abcde output live, show meaningful progress, collect CDDB data.
Filters abcde/cdparanoia output into three layers: Filters abcde/cdparanoia output into three layers:
@ -659,12 +698,12 @@ def interactive_rip(config: RipperConfig) -> None:
if detected_ean: if detected_ean:
print(f" Erkannter Barcode: {detected_ean}") print(f" Erkannter Barcode: {detected_ean}")
confirm = _clean_input( confirm = _clean_input(
input(" Korrekt? (Enter = ja, neuer Wert = tippen): ") _read_line(" Korrekt? (Enter = ja, neuer Wert = tippen): ")
) )
ean = confirm if confirm else detected_ean ean = confirm if confirm else detected_ean
else: else:
print(" Kein Barcode erkannt.") print(" Kein Barcode erkannt.")
ean = _clean_input(input(" EAN manuell eingeben (Enter = überspringen): ")) ean = _clean_input(_read_line(" EAN manuell eingeben (Enter = überspringen): "))
mb_album: AlbumModel | None = None mb_album: AlbumModel | None = None
mb_mbid: str | None = None mb_mbid: str | None = None
@ -759,9 +798,10 @@ def interactive_rip(config: RipperConfig) -> None:
except RuntimeError as e: except RuntimeError as e:
print(f"\n ✗ Error: {e}") print(f"\n ✗ Error: {e}")
raw_retry = input(" Nochmal versuchen? (j/n): ") raw_retry = _read_line(" Nochmal versuchen? (j/n): ")
if _clean_input(raw_retry).lower() not in ("j", "ja", "y", "yes"): if _clean_input(raw_retry).lower() not in ("j", "ja", "y", "yes"):
raise # Abbruch: Ausnahme nach oben weitergeben print(" Disc übersprungen.")
break # while-break → nächste Disc im for-loop
# Vision-LLM-Ergebnis abholen (läuft parallel zum Ripping) # Vision-LLM-Ergebnis abholen (läuft parallel zum Ripping)
final_album = mb_album final_album = mb_album
@ -842,8 +882,13 @@ def interactive_rip(config: RipperConfig) -> None:
print(f"\n Album: {album_name}") print(f"\n Album: {album_name}")
print(f" CD Drive: {config.device}") print(f" CD Drive: {config.device}")
raw_disc = input(f" CD number [{disc_counter}]: ") raw_disc = _read_line(f" CD number [{disc_counter}]: ")
disc_num = int(_clean_input(raw_disc)) if _clean_input(raw_disc) else disc_counter cleaned_disc = _clean_input(raw_disc)
try:
disc_num = int(cleaned_disc) if cleaned_disc else disc_counter
except ValueError:
print(f" Ungültige Nummer '{cleaned_disc}', verwende {disc_counter}")
disc_num = disc_counter
disc_dir = ( disc_dir = (
config.output_dir config.output_dir
@ -873,7 +918,7 @@ def interactive_rip(config: RipperConfig) -> None:
for t in tracks: for t in tracks:
print(f" {t.track_number:2d}. {t.title} [{t.artist}]") print(f" {t.track_number:2d}. {t.title} [{t.artist}]")
raw_ok = input("\n Treffer korrekt? (j/n) [j]: ") raw_ok = _read_line("\n Treffer korrekt? (j/n) [j]: ")
if _clean_input(raw_ok).lower() not in ("n", "no", "nein"): if _clean_input(raw_ok).lower() not in ("n", "no", "nein"):
print(" Umbenennen ...", flush=True) print(" Umbenennen ...", flush=True)
_rename_files(disc_dir, tracks, config.audio_format) _rename_files(disc_dir, tracks, config.audio_format)
@ -899,13 +944,13 @@ def interactive_rip(config: RipperConfig) -> None:
except RuntimeError as e: except RuntimeError as e:
print(f"\n ✗ Error: {e}") print(f"\n ✗ Error: {e}")
raw_retry = input(" Try again? (y/n): ") raw_retry = _read_line(" Try again? (y/n): ")
if _clean_input(raw_retry).lower() != "y": if _clean_input(raw_retry).lower() != "y":
print(" Aborting disc.") print(" Aborting disc.")
break break
continue continue
raw_next = input("\n Next CD for this album? (y/n): ") raw_next = _read_line("\n Next CD for this album? (y/n): ")
if _clean_input(raw_next).lower() != "y": if _clean_input(raw_next).lower() != "y":
break break
@ -993,7 +1038,7 @@ def interactive_rip(config: RipperConfig) -> None:
processed_albums.append((album_root, json_path, len(final_album.discs))) processed_albums.append((album_root, json_path, len(final_album.discs)))
scanner.stop() scanner.stop()
raw_album = input("\nNext album? (y/n): ") raw_album = _read_line("\nNext album? (y/n): ")
if _clean_input(raw_album).lower() != "y": if _clean_input(raw_album).lower() != "y":
break break

View file

@ -110,6 +110,13 @@ _UPLOAD_HTML = """\
if (data.status === 'ok') { if (data.status === 'ok') {
status.className = 'ok'; status.className = 'ok';
status.textContent = '\u2713 Erfolgreich hochgeladen! KI analysiert das Cover\u2026'; status.textContent = '\u2713 Erfolgreich hochgeladen! KI analysiert das Cover\u2026';
setTimeout(() => {
input.value = '';
preview.style.display = 'none';
btn.style.display = 'none';
btn.disabled = false;
status.style.display = 'none';
}, 3000);
} else { } else {
throw new Error(data.message || 'Unbekannter Fehler'); throw new Error(data.message || 'Unbekannter Fehler');
} }

View file

@ -358,7 +358,7 @@ class TestInteractiveRipEanFirst:
config = RipperConfig(output_dir=tmp_path) config = RipperConfig(output_dir=tmp_path)
sp = self._scanner_patches() sp = self._scanner_patches()
with ( with (
sp[0], sp[1], sp[0], sp[1], sp[2],
patch("musiksammlung.ripper.rip_disc", return_value=(tmp_path, None, _CDDB_TRACKS)), patch("musiksammlung.ripper.rip_disc", return_value=(tmp_path, None, _CDDB_TRACKS)),
patch("builtins.input", side_effect=iter(inputs)), patch("builtins.input", side_effect=iter(inputs)),
patch("musiksammlung.ripper.lookup_by_barcode", return_value=(_MB_ALBUM, "fake-mbid")), patch("musiksammlung.ripper.lookup_by_barcode", return_value=(_MB_ALBUM, "fake-mbid")),
@ -386,7 +386,7 @@ class TestInteractiveRipEanFirst:
config = RipperConfig(output_dir=tmp_path) config = RipperConfig(output_dir=tmp_path)
sp = self._scanner_patches() sp = self._scanner_patches()
with ( with (
sp[0], sp[1], sp[0], sp[1], sp[2],
patch("musiksammlung.ripper.rip_disc", return_value=(tmp_path, None, _CDDB_TRACKS)), patch("musiksammlung.ripper.rip_disc", return_value=(tmp_path, None, _CDDB_TRACKS)),
patch("builtins.input", side_effect=iter(inputs)), patch("builtins.input", side_effect=iter(inputs)),
patch("musiksammlung.ripper.lookup_by_barcode", patch("musiksammlung.ripper.lookup_by_barcode",
@ -414,7 +414,7 @@ class TestInteractiveRipEanFirst:
config = RipperConfig(output_dir=tmp_path) config = RipperConfig(output_dir=tmp_path)
sp = self._scanner_patches() sp = self._scanner_patches()
with ( with (
sp[0], sp[1], sp[0], sp[1], sp[2],
patch("musiksammlung.ripper.rip_disc", return_value=(tmp_path, None, _CDDB_TRACKS)), patch("musiksammlung.ripper.rip_disc", return_value=(tmp_path, None, _CDDB_TRACKS)),
patch("builtins.input", side_effect=iter(inputs)), patch("builtins.input", side_effect=iter(inputs)),
patch("musiksammlung.ripper.lookup_by_barcode", patch("musiksammlung.ripper.lookup_by_barcode",
@ -440,7 +440,7 @@ class TestInteractiveRipEanFirst:
sp = self._scanner_patches() sp = self._scanner_patches()
with ( with (
sp[0], sp[1], sp[0], sp[1], sp[2],
patch("musiksammlung.ripper.rip_disc", return_value=(disc_dir, None, _CDDB_TRACKS)), patch("musiksammlung.ripper.rip_disc", return_value=(disc_dir, None, _CDDB_TRACKS)),
patch("builtins.input", side_effect=iter(inputs)), patch("builtins.input", side_effect=iter(inputs)),
patch("musiksammlung.ripper.lookup_by_barcode", return_value=(_MB_ALBUM, "fake-mbid")), patch("musiksammlung.ripper.lookup_by_barcode", return_value=(_MB_ALBUM, "fake-mbid")),
@ -459,7 +459,11 @@ class TestInteractiveRipEanFirst:
@staticmethod @staticmethod
def _scanner_patches(): def _scanner_patches():
"""Patches für ScannerServer und _input_or_scan (alle interactive_rip-Tests).""" """Patches für ScannerServer, _input_or_scan und _read_line.
_input_or_scan und _read_line leiten beide an builtins.input weiter,
das in jedem Test separat mit einer Eingabeliste gemockt wird.
"""
mock_scanner = MagicMock() mock_scanner = MagicMock()
mock_scanner.url.return_value = "http://127.0.0.1:8765" mock_scanner.url.return_value = "http://127.0.0.1:8765"
mock_scanner.get_photo.return_value = None mock_scanner.get_photo.return_value = None
@ -469,6 +473,10 @@ class TestInteractiveRipEanFirst:
"musiksammlung.ripper._input_or_scan", "musiksammlung.ripper._input_or_scan",
side_effect=lambda prompt, scanner: (input(prompt), None), side_effect=lambda prompt, scanner: (input(prompt), None),
), ),
patch(
"musiksammlung.ripper._read_line",
side_effect=lambda prompt="": input(prompt),
),
] ]
@staticmethod @staticmethod
@ -491,7 +499,7 @@ class TestInteractiveRipEanFirst:
config = RipperConfig(output_dir=tmp_path) config = RipperConfig(output_dir=tmp_path)
patches = self._fallback_patches(inputs) patches = self._fallback_patches(inputs)
with ( with (
patches[0], patches[1], patches[2], patches[0], patches[1], patches[2], patches[3],
patch("musiksammlung.ripper.rip_disc", return_value=(tmp_path, None, _CDDB_TRACKS)), patch("musiksammlung.ripper.rip_disc", return_value=(tmp_path, None, _CDDB_TRACKS)),
patch("musiksammlung.ripper.lookup_by_barcode") as mock_lookup, patch("musiksammlung.ripper.lookup_by_barcode") as mock_lookup,
): ):
@ -514,7 +522,7 @@ class TestInteractiveRipEanFirst:
config = RipperConfig(output_dir=tmp_path) config = RipperConfig(output_dir=tmp_path)
patches = self._fallback_patches(inputs) patches = self._fallback_patches(inputs)
with ( with (
patches[0], patches[1], patches[2], patches[0], patches[1], patches[2], patches[3],
patch("musiksammlung.ripper.rip_disc", return_value=(tmp_path, None, _CDDB_TRACKS)), patch("musiksammlung.ripper.rip_disc", return_value=(tmp_path, None, _CDDB_TRACKS)),
patch( patch(
"musiksammlung.ripper.lookup_by_barcode", "musiksammlung.ripper.lookup_by_barcode",
@ -539,7 +547,7 @@ class TestInteractiveRipEanFirst:
config = RipperConfig(output_dir=tmp_path) config = RipperConfig(output_dir=tmp_path)
patches = self._fallback_patches(inputs) patches = self._fallback_patches(inputs)
with ( with (
patches[0], patches[1], patches[2], patches[0], patches[1], patches[2], patches[3],
patch("musiksammlung.ripper.rip_disc", return_value=(tmp_path, None, _CDDB_TRACKS)), patch("musiksammlung.ripper.rip_disc", return_value=(tmp_path, None, _CDDB_TRACKS)),
patch("musiksammlung.ripper.lookup_by_barcode"), patch("musiksammlung.ripper.lookup_by_barcode"),
): ):
@ -565,7 +573,7 @@ class TestInteractiveRipEanFirst:
config = RipperConfig(output_dir=tmp_path) config = RipperConfig(output_dir=tmp_path)
patches = self._fallback_patches(inputs) patches = self._fallback_patches(inputs)
with ( with (
patches[0], patches[1], patches[2], patches[0], patches[1], patches[2], patches[3],
patch("musiksammlung.ripper.rip_disc", return_value=(tmp_path, None, _CDDB_TRACKS)), patch("musiksammlung.ripper.rip_disc", return_value=(tmp_path, None, _CDDB_TRACKS)),
patch("musiksammlung.ripper.lookup_by_barcode"), patch("musiksammlung.ripper.lookup_by_barcode"),
): ):