Skip to content

fix: display original format name for ffmpeg files (#46) #62

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
systemblueteam merged 13 commits into from
Mar 27, 2026
+45 −1
Merged

fix: display original format name for ffmpeg files (#46) #62

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
7a1f640
fix: display original format name for ffmpeg files (#46)
jaimefgdev Mar 27, 2026
062e766
style: add trailing newline
jaimefgdev Mar 27, 2026
ede69af
style: apply ruff formatting
jaimefgdev Mar 27, 2026
47d69ce
fix: restore missing ffmpeg block and resolve undefined variables
jaimefgdev Mar 27, 2026
34c423d
Merge branch 'main' into fix/format-name
jaimefgdev Mar 27, 2026
d50f8dc
style: fix trailing newline for good
jaimefgdev Mar 27, 2026
c003cd7
test: add format name test and fix typing
jaimefgdev Mar 27, 2026
ab69fb9
style: add missing trailing newline
jaimefgdev Mar 27, 2026
f398dce
style: remove trailing whitespace at EOF
jaimefgdev Mar 27, 2026
a6da74e
style: remove trailing whitespace at EOF
jaimefgdev Mar 27, 2026
9660a0e
style: fix all formatting and linting issues
jaimefgdev Mar 27, 2026
ca757e0
Merge branch 'main' into fix/format-name
jaimefgdev Mar 27, 2026
dda935b
fix: manually add trailing newlines
jaimefgdev Mar 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion src/sounddiff/formats.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,15 @@
# Formats that require ffmpeg
FFMPEG_FORMATS = {".mp3", ".aac", ".m4a", ".wma", ".opus"}

# Mapping for original format display names in metadata
FORMAT_DISPLAY_NAMES: dict[str, str] = {
".mp3": "MP3",
".aac": "AAC",
".m4a": "AAC",
".wma": "WMA",
".opus": "Opus",
}


def _remove_if_exists(path: str) -> None:
"""Remove a file if it exists, silently ignoring missing files."""
Expand Down Expand Up @@ -100,6 +109,12 @@ def load_audio(path: str | Path) -> tuple[np.ndarray, AudioMetadata]:
except RuntimeError as e:
raise RuntimeError(f"Cannot read audio file: {original_filepath} ({e})") from e

if original_filepath != read_filepath:
ext = original_filepath.suffix.lower()
display_format = FORMAT_DISPLAY_NAMES.get(ext, ext.lstrip(".").upper())
else:
display_format = info.format

try:
file_size: int | None = original_filepath.stat().st_size
except OSError:
Expand All @@ -111,7 +126,7 @@ def load_audio(path: str | Path) -> tuple[np.ndarray, AudioMetadata]:
sample_rate=sample_rate,
channels=data.shape[1],
bit_depth=_subtype_to_bits(info.subtype),
format_name=info.format,
format_name=display_format,
frames=len(data),
file_size=file_size,
)
Expand Down
29 changes: 29 additions & 0 deletions tests/test_formats.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,35 @@ def test_frames_matches_data_length(self) -> None:
data, meta = load_audio(FIXTURES_DIR / "sine_a.wav")
assert meta.frames == len(data)

def test_transcoded_format_name_is_correct(self, tmp_path: Path) -> None:
"""Verifies that transcoded files show their original format name, not WAV."""
from unittest.mock import patch

# 1. Creamos un archivo MP3 falso para pasar la validación de path.exists()
fake_mp3 = tmp_path / "test.mp3"
fake_mp3.write_text("fake audio content")

# 2. Simulamos que ffmpeg existe y que la lectura del WAV temporal funciona
with (
patch("sounddiff.formats.shutil.which", return_value="ffmpeg"),
patch("sounddiff.formats.subprocess.run"),
patch("sounddiff.formats.sf.info") as mock_info,
patch("sounddiff.formats.sf.read") as mock_read,
):
# Configuramos el mock para simular lo que devolvería el WAV temporal
class MockInfo:
format = "WAV"
subtype = "PCM_16"

mock_info.return_value = MockInfo()
mock_read.return_value = (np.zeros((100, 2), dtype=np.float64), 44100)

# 3. Llamamos a tu función
_, meta = load_audio(fake_mp3)

# 4. LA COMPROBACIÓN FINAL: Debe decir MP3 y no WAV
assert meta.format_name == "MP3"
Comment on lines +59 to +86
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 27, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Test logic is sound; consider translating comments to English.

The test correctly validates the core fix by mocking the ffmpeg transcode flow and asserting that format_name reflects the original format ("MP3") rather than the intermediate WAV. Good isolation with tmp_path and proper mock setup.

The inline comments are in Spanish; for consistency with the English codebase, consider translating them:

📝 Suggested comment translations 
-        # 1. Creamos un archivo MP3 falso para pasar la validación de path.exists()
+        # 1. Create a fake MP3 file to pass path.exists() validation
         fake_mp3 = tmp_path / "test.mp3"
         fake_mp3.write_text("fake audio content")

-        # 2. Simulamos que ffmpeg existe y que la lectura del WAV temporal funciona
+        # 2. Simulate ffmpeg existing and temp WAV read succeeding
         with patch("sounddiff.formats.shutil.which", return_value="ffmpeg"), \
              ...

-            # Configuramos el mock para simular lo que devolvería el WAV temporal
+            # Configure mock to simulate what the temp WAV would return
             ...

-            # 3. Llamamos a tu función
+            # 3. Call the function under test
             _, meta = load_audio(fake_mp3)

-            # 4. LA COMPROBACIÓN FINAL: Debe decir MP3 y no WAV
+            # 4. Final assertion: must report MP3, not WAV
             assert meta.format_name == "MP3"

Copy link
Copy Markdown
Owner

@systemblueteam systemblueteam Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jaimefgdev

Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!


def test_file_size_is_populated(self) -> None:
_, meta = load_audio(FIXTURES_DIR / "sine_a.wav")
assert meta.file_size is not None
Expand Down
Loading