openvinotoolkit / openvinotoolkit/model_server
readMp3() never down-mixes stereo - returns interleaved samples where the API contract says mono
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 931
- Forks
- 277
- Avg merge
- 2d 13h
- Merged PRs (30d)
- 68
Description
Describe the bug
readMp3() never down-mixes stereo input, so it returns interleaved L/R samples where the API contract says mono.
src/audio/audio_utils.hpp:35 documents readMp3() as "Decode MP3 data into mono float32 PCM samples", and line 39 says the same for readWithoutResample(). The WAV path honours that contract — src/audio/audio_utils.cpp:123-134 explicitly down-mixes the channel pair:
// convert to mono, float
...
} else {
for (uint64_t i = 0; i < n; i++) {
pcmf32[i] = float(pcm16[2 * i] + pcm16[2 * i + 1]) / 65536.0f;
}
}
The MP3 path does not. src/audio/audio_utils.cpp:194 appends every decoded channel verbatim:
pcmf32.insert(pcmf32.end(), tempBuffer, tempBuffer + framesRead * mp3.channels);
So for a 2-channel file pcmf32 holds L0,R0,L1,R1,… and is twice as long as the frame count. Stereo is an explicitly supported input — src/audio/audio_utils.cpp:162-165 only rejects channels > 2 — so this is a reachable path, not a guarded edge case.
Two consequences:
1. No-resample path. readWithoutResample() feeds the chat-completions input_audio field. src/llm/io_processing/input_processors/audio_decoding_processor.cpp:70-76 copies the returned vector straight into a 1-D f32 tensor that the pipeline treats as a mono waveform:
std::vector<float> pcm = ovms::audio_utils::readWithoutResample(...);
ov::Tensor audioTensor(ov::element::f32, ov::Shape{pcm.size()});
std::memcpy(audioTensor.data<float>(), pcm.data(), pcm.size() * sizeof(float));
The model receives an interleaved L/R stream read as mono — garbage audio at 2x the expected duration.
2. Resample path. src/audio/audio_utils.cpp:209 uses pcmf32.size() (samples, not frames) as if it were a mono sample count:
size_t outputLength = (size_t)(pcmf32.size() * targetSampleRate / mp3.sampleRate);
so the output is 2x too long and resample_audio() linearly interpolates between adjacent L and R samples.
The existing tests already record the symptom without naming it — src/test/audio/audio_utils_test.cpp:173 and :190:
// For this frame, actual decoded size is 2304 samples (stereo or decoder output)
size_t expectedDecodedSize = 2304 * sizeof(float);
2304 = 1152 frames x 2 channels, i.e. one MPEG-1 Layer III joint-stereo frame returned interleaved. The WAV equivalent would have been 1152.
To Reproduce
- Create a 2-channel MP3:
ffmpeg -f lavfi -i "sine=frequency=440:duration=3" -ac 2 stereo.mp3 - Start a servable that accepts
input_audio(audio-capable chat model). - POST a chat completion containing:
{"type":"input_audio","input_audio":{"data":"<base64 of stereo.mp3>","format":"mp3"}} - Compare with the same content converted to a stereo WAV.
The MP3 request produces a nonsense transcription of roughly double the real duration; the WAV request transcribes correctly.
Expected behavior
readMp3() down-mixes 2-channel input to mono before returning, exactly as readWav() does, so the documented "mono float32 PCM samples" contract holds for both formats and for both the resample and no-resample paths.
Logs
At --log_level DEBUG, AudioDecodingProcessor: decoded audio {} samples, format='mp3' reports twice the frame count for a stereo MP3.
Configuration
- OVMS version:
main@fadb3314 - Any audio-capable servable accepting
input_audio, or any/v3/audio/transcriptionsservable fed an MP3 - CPU
- N/A
- Any 2-channel MP3
Additional context
Suggested fix: down-mix inside the decode loop (average the channel pair per frame) rather than after it, so the streaming size guard at src/audio/audio_utils.cpp:195 keeps measuring the final buffer. Note that validateAudioFileSize(...) at :172 passes mp3.channels, while the WAV call at :110 passes 1 /*will be downmixed to mono*/ — that argument should become 1 for MP3 too once the down-mix is added.
Test to add: decode a known 2-channel MP3 frame and assert result.size() == totalPCMFrameCount (currently == 2 * totalPCMFrameCount). The existing 2304 expectations in src/test/audio/audio_utils_test.cpp become 1152.
A separate, much smaller thing noticed in the same function: the overflow branch at src/audio/audio_utils.cpp:190-193 calls drmp3_uninit(&mp3) and then throws, but the enclosing catch (...) at :197-200 calls drmp3_uninit(&mp3) again before rethrowing — a double-uninit. It is currently unreachable in practice (AUDIO_BUFFER_SIZE_LIMIT is SIZE_MAX / sizeof(float)), but it disappears naturally with the fix above.
I have a patch for this and will open a PR shortly.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the MP3 decode loop in src/audio/audio_utils.cpp:162-209 and compare its channel handling with the WAV path at :123-134. Update the audio utility tests in src/test/audio/audio_utils_test.cpp:173 and :190 so stereo decoding returns one sample per frame, and verify both resampled and no-resample paths use mono-length output.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- audio-video-rtc
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 25/100