Skip to content

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

Description

@Daksha1611

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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions