openvinotoolkit / openvinotoolkit/model_server

readMp3() never down-mixes stereo - returns interleaved samples where the API contract says mono

Open
#4,543 0 comments 0 reactions 0 assignees View on GitHub

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

  1. Create a 2-channel MP3:
    ffmpeg -f lavfi -i "sine=frequency=440:duration=3" -ac 2 stereo.mp3
    
  2. Start a servable that accepts input_audio (audio-capable chat model).
  3. POST a chat completion containing:
    {"type":"input_audio","input_audio":{"data":"<base64 of stereo.mp3>","format":"mp3"}}
    
  4. 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

  1. OVMS version: main @ fadb3314
  2. Any audio-capable servable accepting input_audio, or any /v3/audio/transcriptions servable fed an MP3
  3. CPU
  4. N/A
  5. 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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.