From 6497e8c47ae3efe8034b17bfb8b58f9b87c709fd Mon Sep 17 00:00:00 2001 From: Artem Akymenko Date: Sun, 26 Jul 2026 06:08:43 +0000 Subject: [PATCH] refactor: metadata + markers unified in shared layer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - MarkerCollector: SRP extraction from executor (observation, not execution) - Outro marker: executor now records outro as chapter marker - Voice format: chapter voices [{provider, voice}], chunk voice {provider, voice} - _finalize(): build_metadata_payload() + metadata.json + record_override_usage() - ffmetadata: voices list → comma-separated string - EPUB3: ChunkOverlay.voice = dict, _render_chunk_inline handles dict format - 8 new tests: multi-speaker, ffmetadata format, EPUB3 format - Updated existing tests for new voice format --- abogen/application/conversion_executor.py | 153 ++++++++++-- abogen/application/conversion_service.py | 33 +++ abogen/epub3/exporter.py | 11 +- abogen/infrastructure/exporters.py | 11 +- tests/test_conversion_executor_unified.py | 273 ++++++++++++++++++++++ tests/test_exporters.py | 4 +- tests/test_ffmetadata.py | 4 +- 7 files changed, 456 insertions(+), 33 deletions(-) diff --git a/abogen/application/conversion_executor.py b/abogen/application/conversion_executor.py index 059629d..8242ace 100644 --- a/abogen/application/conversion_executor.py +++ b/abogen/application/conversion_executor.py @@ -10,7 +10,7 @@ from __future__ import annotations import time from contextlib import ExitStack -from typing import Any, Callable, Dict, List, Optional, Tuple +from typing import Any, Callable, Dict, List, Optional, Set, Tuple from abogen.application.conversion_models import ( ConversionPlan, @@ -40,6 +40,109 @@ from abogen.domain.output_paths import sanitize_filename_for_chapter from abogen.infrastructure.subtitle_writer import make_subtitle_writer +# ─── MarkerCollector ─── + + +class MarkerCollector: + """Observes execution events and accumulates chapter/chunk markers. + + Separates marker collection from synthesis logic. + """ + + def __init__(self) -> None: + self._chapter_markers: List[Dict[str, Any]] = [] + self._chunk_markers: List[Dict[str, Any]] = [] + self._current_chapter_voices: Set[Tuple[str, str]] = set() + self._current_chapter_index: int = 0 + self._current_chapter_title: str = "" + self._current_chapter_start: float = 0.0 + + def on_chapter_start( + self, index: int, title: str, start_time: float + ) -> None: + """Record chapter start.""" + self._current_chapter_index = index + self._current_chapter_title = title + self._current_chapter_start = start_time + self._current_chapter_voices.clear() + + def on_segment( + self, + provider: str, + voice: Any, + voice_spec: str, + speaker_id: str = "narrator", + ) -> None: + """Record a voice used in this chapter (for multi-speaker tracking).""" + self._current_chapter_voices.add((provider, voice_spec)) + + def on_chunk( + self, + chunk_id: str, + chapter_index: int, + chunk_index: int, + start: float, + end: float, + speaker_id: str, + provider: str, + voice_spec: str, + level: str, + characters: int, + ) -> None: + """Record a chunk marker.""" + self._chunk_markers.append({ + "id": chunk_id, + "chapter_index": chapter_index, + "chunk_index": chunk_index, + "start": start, + "end": end, + "speaker_id": speaker_id, + "voice": {"provider": provider, "voice": voice_spec}, + "level": level, + "characters": characters, + }) + + def on_chapter_end(self, end_time: float) -> None: + """Record chapter end and build chapter marker.""" + voices = [ + {"provider": p, "voice": v} + for p, v in sorted(self._current_chapter_voices) + ] + self._chapter_markers.append({ + "chapter_index": self._current_chapter_index, + "index": self._current_chapter_index + 1, + "title": self._current_chapter_title, + "start": self._current_chapter_start, + "end": end_time, + "voices": voices, + }) + + def on_outro( + self, + start_time: float, + end_time: float, + provider: str, + voice_spec: str, + ) -> None: + """Record outro chapter marker.""" + self._chapter_markers.append({ + "chapter_index": len(self._chapter_markers), + "index": len(self._chapter_markers) + 1, + "title": "Outro", + "start": start_time, + "end": end_time, + "voices": [{"provider": provider, "voice": voice_spec}], + }) + + @property + def chapter_markers(self) -> List[Dict[str, Any]]: + return self._chapter_markers + + @property + def chunk_markers(self) -> List[Dict[str, Any]]: + return self._chunk_markers + + def execute_conversion( plan: ConversionPlan, events: ConversionEvents, @@ -67,6 +170,7 @@ def execute_conversion( """ request = plan.request result = ConversionResult(metadata=plan.metadata) + collector = MarkerCollector() # Determine cancellation checker if check_cancelled is None: @@ -192,6 +296,9 @@ def execute_conversion( ) chapter_backend = pipeline_provider.get(chapter_provider, request.language, request.use_gpu) + # Record chapter start for markers + collector.on_chapter_start(chapter_idx - 1, chapter.title, stats.current_time) + # Per-chapter sink chapter_sink: Optional[AudioSink] = None chapter_path = None @@ -285,8 +392,6 @@ def execute_conversion( pending_heading_strip = True # Process body segments - chapter_chunk_markers: List[Dict[str, Any]] = [] - chapter_body_start = stats.current_time for seg_idx, segment in enumerate(chapter.segments): check_cancelled() @@ -317,6 +422,9 @@ def execute_conversion( seg_speed = chapter_speed seg_backend = chapter_backend + # Track voice for chapter marker + collector.on_segment(seg_provider, seg_voice, segment.voice_spec) + seg_start_time = stats.current_time local_segments, accumulated_tokens = synthesize_text( text=seg_text, @@ -353,17 +461,18 @@ def execute_conversion( # Record chunk marker if segment.source in ("chunk", "voice_marker"): - chapter_chunk_markers.append({ - "id": segment.chunk_id, - "chapter_index": chapter_idx - 1, - "chunk_index": segment.chunk_index or seg_idx, - "start": seg_start_time, - "end": stats.current_time, - "speaker_id": segment.speaker_id, - "voice": segment.voice_spec, - "level": segment.level or (request.chapter_chunk.chunk_level if request.chapter_chunk else "paragraph"), - "characters": len(segment.text), - }) + collector.on_chunk( + chunk_id=segment.chunk_id or "", + chapter_index=chapter_idx - 1, + chunk_index=segment.chunk_index or seg_idx, + start=seg_start_time, + end=stats.current_time, + speaker_id=segment.speaker_id or "narrator", + provider=seg_provider, + voice_spec=segment.voice_spec, + level=segment.level or (request.chapter_chunk.chunk_level if request.chapter_chunk else "paragraph"), + characters=len(segment.text), + ) # Silence between chapters if chapter_idx < len(plan.chapters) and request.silence_between_chapters > 0: @@ -382,15 +491,8 @@ def execute_conversion( if chapter_subtitle_writer: chapter_subtitle_writer.close() - # Add chapter marker - result.chapter_markers.append({ - "chapter_index": chapter_idx - 1, - "title": chapter.title, - "start": chapter_body_start, - "end": stats.current_time, - }) - - result.chunk_markers.extend(chapter_chunk_markers) + # Record chapter end for markers + collector.on_chapter_end(stats.current_time) # Process outro if plan.outro and plan.outro.enabled and merge_chapters: @@ -409,6 +511,7 @@ def execute_conversion( stats=stats, ) + outro_start = stats.current_time synthesize_text( text=plan.outro.text, params=synth, @@ -418,9 +521,13 @@ def execute_conversion( chapter_sink=None, preview_callback=lambda text: events.log(f" {text[:80]}"), ) + # Record outro marker + collector.on_outro(outro_start, stats.current_time, outro_provider, plan.outro.voice_spec) events.log("Outro synthesized.") # Set result metadata + result.chapter_markers = collector.chapter_markers + result.chunk_markers = collector.chunk_markers result.total_chapters = len(plan.chapters) result.total_segments = sum(len(ch.segments) for ch in plan.chapters) result.total_characters = total_characters diff --git a/abogen/application/conversion_service.py b/abogen/application/conversion_service.py index 1166805..a8a454a 100644 --- a/abogen/application/conversion_service.py +++ b/abogen/application/conversion_service.py @@ -166,6 +166,39 @@ def _finalize( else: events.log("Skipped EPUB 3 generation: audio output unavailable.", level="warning") + # Build metadata payload and write metadata.json + if plan.output_layout and plan.output_layout.metadata_dir: + from abogen.domain.metadata_helpers import build_metadata_payload + + metadata_payload = build_metadata_payload( + metadata=result.metadata, + chapter_markers=result.chapter_markers, + chunk_markers=result.chunk_markers, + chunk_level=request.chapter_chunk.chunk_level if request.chapter_chunk else None, + speaker_mode=request.chapter_chunk.speaker_mode if request.chapter_chunk else None, + speakers=request.chapter_chunk.speakers if request.chapter_chunk else None, + generate_epub3=bool(request.epub3_export), + ) + + metadata_dir = plan.output_layout.metadata_dir + metadata_dir.mkdir(parents=True, exist_ok=True) + metadata_file = metadata_dir / "metadata.json" + + import json + + metadata_file.write_text(json.dumps(metadata_payload, indent=2), encoding="utf-8") + result.artifacts["metadata"] = metadata_file + events.log(f"Metadata written to {metadata_file}") + + # Record override usage + if result.usage_counter: + try: + from abogen.normalization_settings import record_override_usage + + record_override_usage(result.usage_counter) + except Exception as exc: + events.log(f"Failed to record override usage: {exc}", level="debug") + def _prepare_tts_context( request: ConversionRequest, diff --git a/abogen/epub3/exporter.py b/abogen/epub3/exporter.py index 834c783..edb8ed5 100644 --- a/abogen/epub3/exporter.py +++ b/abogen/epub3/exporter.py @@ -22,7 +22,7 @@ class ChunkOverlay: start: Optional[float] end: Optional[float] speaker_id: str - voice: Optional[str] + voice: Optional[Dict[str, str]] level: Optional[str] = None group_id: Optional[str] = None @@ -273,7 +273,7 @@ class EPUB3PackageBuilder: start=_safe_float(marker.get("start")), end=_safe_float(marker.get("end")), speaker_id=speaker_id, - voice=str(voice) if voice else None, + voice=voice if isinstance(voice, dict) else None, level=str(level) if level else None, group_id=normalized_group_id, ) @@ -696,7 +696,12 @@ def _group_chunks_for_render(chunks: Sequence[ChunkOverlay]) -> List[Tuple[Optio def _render_chunk_inline(chunk: ChunkOverlay) -> str: escaped_id = html.escape(chunk.id) speaker_attr = f" data-speaker=\"{html.escape(chunk.speaker_id)}\"" if chunk.speaker_id else "" - voice_attr = f" data-voice=\"{html.escape(chunk.voice)}\"" if chunk.voice else "" + voice_str = None + if chunk.voice and isinstance(chunk.voice, dict): + name = chunk.voice.get("voice", "") + provider = chunk.voice.get("provider", "") + voice_str = f"{name}@{provider}" if name and provider else name or None + voice_attr = f" data-voice=\"{html.escape(voice_str)}\"" if voice_str else "" level_attr = f" data-level=\"{html.escape(chunk.level)}\"" if chunk.level else "" raw_text = chunk.text or "" escaped_text = html.escape(raw_text) diff --git a/abogen/infrastructure/exporters.py b/abogen/infrastructure/exporters.py index 5a32e96..a561ce3 100644 --- a/abogen/infrastructure/exporters.py +++ b/abogen/infrastructure/exporters.py @@ -84,9 +84,14 @@ class ExportService: title = chapter.get("title") if title: lines.append(f"title={self._escape_ffmetadata_value(title)}") - voice = chapter.get("voice") - if voice: - lines.append(f"voice={self._escape_ffmetadata_value(voice)}") + voices = chapter.get("voices") + if voices and isinstance(voices, list): + voice_str = ", ".join( + f"{v.get('voice', '')}@{v.get('provider', '')}" + for v in voices if v.get("voice") + ) + if voice_str: + lines.append(f"voice={self._escape_ffmetadata_value(voice_str)}") return "\n".join(lines) + "\n" diff --git a/tests/test_conversion_executor_unified.py b/tests/test_conversion_executor_unified.py index cf02bba..8156290 100644 --- a/tests/test_conversion_executor_unified.py +++ b/tests/test_conversion_executor_unified.py @@ -622,3 +622,276 @@ class TestHeadingDedup: # Both segments should be synthesized (heading + 2 body segments) assert result.total_segments >= 2 + + +class TestMarkerCollector: + """Tests for MarkerCollector.""" + + def test_chapter_marker_has_voices_list(self): + """Chapter markers should have 'voices' as list of dicts.""" + with tempfile.TemporaryDirectory() as tmpdir: + req = ConversionRequest(direct_text="Hello", voice="M1") + plan = ConversionPlan( + request=req, + metadata={}, + chapters=[ + ChapterPlan( + index=1, + title="Chapter 1", + original_title="Chapter 1", + body_text="Body text", + segments=[ + SegmentPlan( + text="Body text", + voice_spec="M1", + kind="body", + source="chapter", + ), + ], + voice_spec="M1", + ) + ], + output_layout=OutputLayout( + parent_dir=Path(tmpdir), + audio_dir=Path(tmpdir), + ), + ) + + events = FakeEvents() + pipeline = FakePipelineProvider() + resolver = FakeVoiceResolver() + tts_context = TTSContext() + + result = execute_conversion(plan, events, pipeline, resolver, tts_context) + + assert len(result.chapter_markers) == 1 + marker = result.chapter_markers[0] + assert "voices" in marker + assert isinstance(marker["voices"], list) + assert len(marker["voices"]) == 1 + assert marker["voices"][0]["provider"] == "kokoro" + assert marker["voices"][0]["voice"] == "M1" + + def test_outro_marker_recorded(self): + """Outro should be recorded as a chapter marker.""" + from abogen.application.conversion_models import IntroOutroSpec + + with tempfile.TemporaryDirectory() as tmpdir: + req = ConversionRequest(direct_text="Hello", voice="M1") + plan = ConversionPlan( + request=req, + metadata={}, + chapters=[ + ChapterPlan( + index=1, + title="Chapter 1", + original_title="Chapter 1", + body_text="Body text", + segments=[ + SegmentPlan( + text="Body text", + voice_spec="M1", + kind="body", + source="chapter", + ), + ], + voice_spec="M1", + ) + ], + outro=IntroOutroSpec( + enabled=True, + text="Thanks for listening", + voice_spec="M1", + kind="outro", + ), + output_layout=OutputLayout( + parent_dir=Path(tmpdir), + audio_dir=Path(tmpdir), + ), + ) + + events = FakeEvents() + pipeline = FakePipelineProvider() + resolver = FakeVoiceResolver() + tts_context = TTSContext() + + result = execute_conversion(plan, events, pipeline, resolver, tts_context) + + # Should have chapter marker + outro marker + assert len(result.chapter_markers) == 2 + outro_marker = result.chapter_markers[1] + assert outro_marker["title"] == "Outro" + assert "start" in outro_marker + assert "end" in outro_marker + assert outro_marker["end"] > outro_marker["start"] + + def test_chunk_marker_voice_is_dict(self): + """Chunk markers should have 'voice' as dict with provider.""" + with tempfile.TemporaryDirectory() as tmpdir: + req = ConversionRequest(direct_text="Hello", voice="M1") + plan = ConversionPlan( + request=req, + metadata={}, + chapters=[ + ChapterPlan( + index=1, + title="Chapter 1", + original_title="Chapter 1", + body_text="Body text", + segments=[ + SegmentPlan( + text="Body text", + voice_spec="M1", + kind="body", + source="chunk", + chunk_id="chunk_001", + chunk_index=0, + speaker_id="narrator", + level="paragraph", + ), + ], + voice_spec="M1", + ) + ], + output_layout=OutputLayout( + parent_dir=Path(tmpdir), + audio_dir=Path(tmpdir), + ), + ) + + events = FakeEvents() + pipeline = FakePipelineProvider() + resolver = FakeVoiceResolver() + tts_context = TTSContext() + + result = execute_conversion(plan, events, pipeline, resolver, tts_context) + + assert len(result.chunk_markers) == 1 + chunk = result.chunk_markers[0] + assert isinstance(chunk["voice"], dict) + assert chunk["voice"]["provider"] == "kokoro" + assert chunk["voice"]["voice"] == "M1" + + def test_multi_speaker_collects_unique_voices(self): + """Multi-speaker chapters should collect all unique voices.""" + with tempfile.TemporaryDirectory() as tmpdir: + req = ConversionRequest(direct_text="Hello", voice="M1") + plan = ConversionPlan( + request=req, + metadata={}, + chapters=[ + ChapterPlan( + index=1, + title="Chapter 1", + original_title="Chapter 1", + body_text="Body text", + segments=[ + SegmentPlan( + text="Narrator speaks", + voice_spec="M1", + kind="body", + source="chapter", + ), + SegmentPlan( + text="Character speaks", + voice_spec="F1", + kind="body", + source="chapter", + ), + ], + voice_spec="M1", + ) + ], + output_layout=OutputLayout( + parent_dir=Path(tmpdir), + audio_dir=Path(tmpdir), + ), + ) + + events = FakeEvents() + pipeline = FakePipelineProvider() + resolver = FakeVoiceResolver() + tts_context = TTSContext() + + result = execute_conversion(plan, events, pipeline, resolver, tts_context) + + marker = result.chapter_markers[0] + assert len(marker["voices"]) == 2 + voice_specs = {v["voice"] for v in marker["voices"]} + assert "M1" in voice_specs + assert "F1" in voice_specs + + +class TestFfmetadataVoiceFormat: + """Tests for ffmetadata rendering with new voice format.""" + + def test_render_ffmetadata_with_voices_list(self): + """ffmetadata should render voices list as comma-separated string.""" + from abogen.infrastructure.exporters import ExportService + + svc = ExportService() + chapters = [ + { + "title": "Chapter 1", + "start": 0.0, + "end": 60.0, + "voices": [ + {"provider": "kokoro", "voice": "M1"}, + {"provider": "kokoro", "voice": "F1"}, + ], + } + ] + content = svc.render_ffmetadata({}, chapters) + assert "voice=M1@kokoro, F1@kokoro" in content + + def test_render_ffmetadata_with_empty_voices(self): + """ffmetadata should handle empty voices list.""" + from abogen.infrastructure.exporters import ExportService + + svc = ExportService() + chapters = [ + { + "title": "Chapter 1", + "start": 0.0, + "end": 60.0, + "voices": [], + } + ] + content = svc.render_ffmetadata({}, chapters) + assert "voice=" not in content + + +class TestEpub3VoiceFormat: + """Tests for EPUB3 voice handling with new format.""" + + def test_chunk_overlay_voice_is_dict(self): + """ChunkOverlay should accept voice as dict.""" + from abogen.epub3.exporter import ChunkOverlay + + overlay = ChunkOverlay( + id="test", + text="hello", + original_text=None, + start=0.0, + end=1.0, + speaker_id="narrator", + voice={"provider": "kokoro", "voice": "M1"}, + ) + assert isinstance(overlay.voice, dict) + assert overlay.voice["provider"] == "kokoro" + + def test_render_chunk_inline_with_voice_dict(self): + """_render_chunk_inline should render voice dict as data-voice attribute.""" + from abogen.epub3.exporter import ChunkOverlay, _render_chunk_inline + + overlay = ChunkOverlay( + id="chunk_001", + text="Hello world", + original_text=None, + start=0.0, + end=1.0, + speaker_id="narrator", + voice={"provider": "kokoro", "voice": "M1"}, + ) + html = _render_chunk_inline(overlay) + assert 'data-voice="M1@kokoro"' in html diff --git a/tests/test_exporters.py b/tests/test_exporters.py index 398cc73..fd65c98 100644 --- a/tests/test_exporters.py +++ b/tests/test_exporters.py @@ -69,9 +69,9 @@ class TestRenderFfmetadata: assert "title=Ch 1" in result def test_renders_voice_in_chapter(self): - chapters = [{"start": 0.0, "end": 5.0, "voice": "af_heart"}] + chapters = [{"start": 0.0, "end": 5.0, "voices": [{"provider": "kokoro", "voice": "af_heart"}]}] result = self.svc.render_ffmetadata({}, chapters) - assert "voice=af_heart" in result + assert "voice=af_heart@kokoro" in result def test_skips_chapters_without_times(self): chapters = [{"title": "No times"}] diff --git a/tests/test_ffmetadata.py b/tests/test_ffmetadata.py index c96e9af..2d3f04a 100644 --- a/tests/test_ffmetadata.py +++ b/tests/test_ffmetadata.py @@ -14,7 +14,7 @@ def test_render_ffmetadata_includes_chapters(tmp_path): "publisher": "ACME=Corp", } chapters = [ - {"start": 0.0, "end": 5.0, "title": "Intro", "voice": "voice_a"}, + {"start": 0.0, "end": 5.0, "title": "Intro", "voices": [{"provider": "kokoro", "voice": "voice_a"}]}, {"start": 5.0, "end": 12.345, "title": "Chapter 2"}, ] @@ -28,7 +28,7 @@ def test_render_ffmetadata_includes_chapters(tmp_path): assert rendered.count("[CHAPTER]") == 2 assert "START=0" in rendered assert "END=5000" in rendered - assert "voice=voice_a" in rendered + assert "voice=voice_a@kokoro" in rendered audio_path = tmp_path / "book.m4b" metadata_path = svc.write_ffmetadata_file(audio_path, metadata, chapters)