From 8c25bc65be400f188b843ee69f2df1f526ce079a Mon Sep 17 00:00:00 2001 From: dschlueter Date: Thu, 19 Feb 2026 19:03:06 +0100 Subject: [PATCH] Fix 6 bugs: shared stdin reader, CDDB multiline, type annotation, crash fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 --- src/musiksammlung/cddb.py | 8 ++- src/musiksammlung/cli.py | 4 -- src/musiksammlung/ripper.py | 95 +++++++++++++++++++++-------- src/musiksammlung/scanner_server.py | 7 +++ tests/test_ripper.py | 26 +++++--- 5 files changed, 100 insertions(+), 40 deletions(-) diff --git a/src/musiksammlung/cddb.py b/src/musiksammlung/cddb.py index 00d105f..be68b0f 100644 --- a/src/musiksammlung/cddb.py +++ b/src/musiksammlung/cddb.py @@ -147,7 +147,7 @@ def _read_gnudb(category: str, discid: str) -> CddbResult | None: if line.startswith("#") or line == ".": continue if line.startswith("DTITLE="): - dtitle = line[7:].strip() + dtitle += line[7:].strip() elif line.startswith("DYEAR="): dyear = line[6:].strip() elif line.startswith("DGENRE="): @@ -155,7 +155,11 @@ def _read_gnudb(category: str, discid: str) -> CddbResult | None: elif line.startswith("TTITLE"): eq = line.index("=") 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 album_artist = "" diff --git a/src/musiksammlung/cli.py b/src/musiksammlung/cli.py index b9fbaff..d32b9a8 100644 --- a/src/musiksammlung/cli.py +++ b/src/musiksammlung/cli.py @@ -293,10 +293,6 @@ def apply( if output_dir is None: 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 _check_disc_counts_or_exit(album, input_dir, album_json) diff --git a/src/musiksammlung/ripper.py b/src/musiksammlung/ripper.py index 541edee..d8e3611 100644 --- a/src/musiksammlung/ripper.py +++ b/src/musiksammlung/ripper.py @@ -144,32 +144,71 @@ def _get_vision_result( 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( prompt: str, scanner: ScannerServer | 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: (eingegebener Text, None) — wenn der User Enter drückt ("", photo_path) — wenn ein Foto hochgeladen wird """ 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) - 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: photo = scanner.get_photo(timeout=0) if photo is not None: @@ -177,7 +216,7 @@ def _input_or_scan( return "", photo try: - val = stdin_q.get(timeout=0.1) + val = _stdin_queue.get(timeout=0.1) return _clean_input(val), None except _queue_module.Empty: continue @@ -252,7 +291,7 @@ def _parse_grab_tracks(grab_data: list[tuple[int, str]]) -> list[TrackInfo]: def _stream_abcde( process: subprocess.Popen, use_cddb: bool, -) -> tuple[list[TrackInfo] | None, int]: +) -> tuple[list[TrackInfo] | None, str, int]: """Stream abcde output live, show meaningful progress, collect CDDB data. Filters abcde/cdparanoia output into three layers: @@ -659,12 +698,12 @@ def interactive_rip(config: RipperConfig) -> None: if detected_ean: print(f" Erkannter Barcode: {detected_ean}") 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 else: 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_mbid: str | None = None @@ -759,9 +798,10 @@ def interactive_rip(config: RipperConfig) -> None: except RuntimeError as 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"): - 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) final_album = mb_album @@ -842,8 +882,13 @@ def interactive_rip(config: RipperConfig) -> None: print(f"\n Album: {album_name}") print(f" CD Drive: {config.device}") - raw_disc = input(f" CD number [{disc_counter}]: ") - disc_num = int(_clean_input(raw_disc)) if _clean_input(raw_disc) else disc_counter + raw_disc = _read_line(f" CD number [{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 = ( config.output_dir @@ -873,7 +918,7 @@ def interactive_rip(config: RipperConfig) -> None: for t in tracks: 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"): print(" Umbenennen ...", flush=True) _rename_files(disc_dir, tracks, config.audio_format) @@ -899,13 +944,13 @@ def interactive_rip(config: RipperConfig) -> None: except RuntimeError as 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": print(" Aborting disc.") break 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": break @@ -993,7 +1038,7 @@ def interactive_rip(config: RipperConfig) -> None: processed_albums.append((album_root, json_path, len(final_album.discs))) scanner.stop() - raw_album = input("\nNext album? (y/n): ") + raw_album = _read_line("\nNext album? (y/n): ") if _clean_input(raw_album).lower() != "y": break diff --git a/src/musiksammlung/scanner_server.py b/src/musiksammlung/scanner_server.py index e03f3f6..918f6ec 100644 --- a/src/musiksammlung/scanner_server.py +++ b/src/musiksammlung/scanner_server.py @@ -110,6 +110,13 @@ _UPLOAD_HTML = """\ if (data.status === 'ok') { status.className = 'ok'; 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 { throw new Error(data.message || 'Unbekannter Fehler'); } diff --git a/tests/test_ripper.py b/tests/test_ripper.py index 7b9ff25..5d4a29a 100644 --- a/tests/test_ripper.py +++ b/tests/test_ripper.py @@ -358,7 +358,7 @@ class TestInteractiveRipEanFirst: config = RipperConfig(output_dir=tmp_path) sp = self._scanner_patches() with ( - sp[0], sp[1], + sp[0], sp[1], sp[2], patch("musiksammlung.ripper.rip_disc", return_value=(tmp_path, None, _CDDB_TRACKS)), patch("builtins.input", side_effect=iter(inputs)), patch("musiksammlung.ripper.lookup_by_barcode", return_value=(_MB_ALBUM, "fake-mbid")), @@ -386,7 +386,7 @@ class TestInteractiveRipEanFirst: config = RipperConfig(output_dir=tmp_path) sp = self._scanner_patches() with ( - sp[0], sp[1], + sp[0], sp[1], sp[2], patch("musiksammlung.ripper.rip_disc", return_value=(tmp_path, None, _CDDB_TRACKS)), patch("builtins.input", side_effect=iter(inputs)), patch("musiksammlung.ripper.lookup_by_barcode", @@ -414,7 +414,7 @@ class TestInteractiveRipEanFirst: config = RipperConfig(output_dir=tmp_path) sp = self._scanner_patches() with ( - sp[0], sp[1], + sp[0], sp[1], sp[2], patch("musiksammlung.ripper.rip_disc", return_value=(tmp_path, None, _CDDB_TRACKS)), patch("builtins.input", side_effect=iter(inputs)), patch("musiksammlung.ripper.lookup_by_barcode", @@ -440,7 +440,7 @@ class TestInteractiveRipEanFirst: sp = self._scanner_patches() with ( - sp[0], sp[1], + sp[0], sp[1], sp[2], patch("musiksammlung.ripper.rip_disc", return_value=(disc_dir, None, _CDDB_TRACKS)), patch("builtins.input", side_effect=iter(inputs)), patch("musiksammlung.ripper.lookup_by_barcode", return_value=(_MB_ALBUM, "fake-mbid")), @@ -459,7 +459,11 @@ class TestInteractiveRipEanFirst: @staticmethod 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.url.return_value = "http://127.0.0.1:8765" mock_scanner.get_photo.return_value = None @@ -469,6 +473,10 @@ class TestInteractiveRipEanFirst: "musiksammlung.ripper._input_or_scan", side_effect=lambda prompt, scanner: (input(prompt), None), ), + patch( + "musiksammlung.ripper._read_line", + side_effect=lambda prompt="": input(prompt), + ), ] @staticmethod @@ -491,7 +499,7 @@ class TestInteractiveRipEanFirst: config = RipperConfig(output_dir=tmp_path) patches = self._fallback_patches(inputs) 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.lookup_by_barcode") as mock_lookup, ): @@ -514,7 +522,7 @@ class TestInteractiveRipEanFirst: config = RipperConfig(output_dir=tmp_path) patches = self._fallback_patches(inputs) 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.lookup_by_barcode", @@ -539,7 +547,7 @@ class TestInteractiveRipEanFirst: config = RipperConfig(output_dir=tmp_path) patches = self._fallback_patches(inputs) 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.lookup_by_barcode"), ): @@ -565,7 +573,7 @@ class TestInteractiveRipEanFirst: config = RipperConfig(output_dir=tmp_path) patches = self._fallback_patches(inputs) 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.lookup_by_barcode"), ):