Skip to content

Commit b062240

Browse files
committed
fix(sound-files): add explicit -f mp3 for atomic-write compatibility (#999)
Commit 74c1a9f introduced atomic tempfile writes ($target.converting) to preserve uploads on conversion failure. This broke MP3 output because ffmpeg 8.0 cannot infer the muxer from the .converting extension — all other formats already had explicit -f flags, MP3 was the only one without. Every MP3 conversion since 14 Apr has been silently failing (exit 234), affecting custom sound uploads, MOH files, and module sounds. Also: strengthen test_10_convert_uploaded_audio_file to verify ALL 7 Asterisk formats (wav, mp3, ulaw, alaw, gsm, g722, sln) exist on disk after conversion, not just the MP3 path returned by the API.
1 parent 547e882 commit b062240

2 files changed

Lines changed: 53 additions & 6 deletions

File tree

src/Core/System/Configs/SoundFilesConf.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -606,7 +606,7 @@ public static function convertAudioFile(string $sourceFile, array $targetFormats
606606
'container' => 'wav',
607607
],
608608
'mp3' => [
609-
'options' => "-codec:a libmp3lame -b:a $mp3Bitrate",
609+
'options' => "-codec:a libmp3lame -b:a $mp3Bitrate -f mp3",
610610
'container' => 'mp3',
611611
],
612612
'ulaw' => [

tests/api/test_11_sound_files_prepare.py

Lines changed: 52 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,13 @@ def test_09_upload_real_audio_file(self, api_client):
356356
print(f" Merged file path: {merged_file_path}")
357357

358358
def test_10_convert_uploaded_audio_file(self, api_client):
359-
"""Test POST /sound-files:convertAudioFile - Convert uploaded file to system format"""
359+
"""Test POST /sound-files:convertAudioFile - Convert uploaded file to system format
360+
361+
Verifies that conversion produces ALL Asterisk-compatible formats (wav, mp3,
362+
ulaw, alaw, gsm, g722, sln) plus that each output file exists and is non-empty.
363+
Regression: commit 74c1a9f4e introduced atomic .converting tempfiles that broke
364+
MP3 output because ffmpeg couldn't infer the format from the .converting extension.
365+
"""
360366
if not hasattr(self.__class__, 'uploaded_file_path') or not self.uploaded_file_path:
361367
pytest.skip("No uploaded file from previous test")
362368

@@ -376,12 +382,53 @@ def test_10_convert_uploaded_audio_file(self, api_client):
376382
data = response.get('data', {})
377383
print(f" Convert response data: {data}")
378384

379-
# ConvertAudioFile returns array with MP3 path as first element
385+
# Verify ALL expected Asterisk formats exist on disk via bash command
386+
# The API returns only the MP3 path, but we need to verify all formats
380387
if isinstance(data, list) and len(data) > 0:
381-
converted_path = data[0]
388+
converted_path = data[0] # MP3 path
382389
self.__class__.converted_file_path = converted_path
383-
print(f"✓ Audio file converted to MP3")
384-
print(f" Converted path: {converted_path}")
390+
391+
# Derive base path (without extension) to check all formats
392+
import os
393+
base_path = os.path.splitext(converted_path)[0]
394+
expected_formats = ['wav', 'mp3', 'ulaw', 'alaw', 'gsm', 'g722', 'sln']
395+
396+
# Verify each format via executeBashCommand
397+
check_cmd = ' && '.join(
398+
f'test -s "{base_path}.{fmt}" && echo "{fmt}:OK" || echo "{fmt}:MISSING"'
399+
for fmt in expected_formats
400+
)
401+
check_response = api_client.post(
402+
'system:executeBashCommand',
403+
{'command': check_cmd, 'timeout': 10}
404+
)
405+
406+
if check_response.get('result'):
407+
output = check_response.get('data', {}).get('output', '')
408+
missing_formats = []
409+
for line in output.strip().split('\n'):
410+
line = line.strip()
411+
if ':MISSING' in line:
412+
fmt_name = line.split(':')[0]
413+
missing_formats.append(fmt_name)
414+
elif ':OK' in line:
415+
fmt_name = line.split(':')[0]
416+
print(f" ✓ {fmt_name} format exists and non-empty")
417+
418+
assert not missing_formats, (
419+
f"Conversion produced incomplete output. Missing formats: {missing_formats}. "
420+
f"Base path: {base_path}. This may indicate ffmpeg cannot determine the "
421+
f"output format from the .converting temp extension (needs explicit -f flag)."
422+
)
423+
else:
424+
pytest.fail(
425+
"Cannot verify converted formats: system:executeBashCommand "
426+
"unavailable. Format verification is required to catch "
427+
"ffmpeg atomic-write regressions."
428+
)
429+
430+
print(f"✓ Audio file converted to all {len(expected_formats)} formats")
431+
print(f" MP3 path: {converted_path}")
385432

386433
# Now create a sound file record manually with retry on database lock
387434
import time

0 commit comments

Comments
 (0)