From 524f0426590dda6b55ba4aadc0fe7debb5cf6b3a Mon Sep 17 00:00:00 2001 From: Kp Date: Wed, 1 Aug 2018 01:18:11 +0000 Subject: [PATCH] SDL2: MVE: fix noise from reading undefined sound data Commit 07245a0bc27e added the SDL_mixer backend for MVE audio. That commit set the length of the converted sound equal to the size of the buffer that SDL requested as a work area. No one ever touched that logic, until now. In SDL1, that choice worked fine. In SDL2, this causes garbage to play after the sound, because SDL2 considers the buffer to be defined only up to the length returned in `SDL_AudioCVT::cvt_len`. Bytes beyond that length are undefined[1], and in practice contain garbage. Fix the noise by setting the sound length equal to the length returned by SDL2, so that the undefined bytes are not treated as sound. SDL1 also maintains this length value, so no version-specific logic is required. [1]: https://wiki.libsdl.org/SDL_ConvertAudio Reported-by: andrew-strong --- d2x-rebirth/libmve/mveplay.cpp | 45 +++++++++++++++------------------- 1 file changed, 20 insertions(+), 25 deletions(-) diff --git a/d2x-rebirth/libmve/mveplay.cpp b/d2x-rebirth/libmve/mveplay.cpp index e4ce5ce8b..7c0167595 100644 --- a/d2x-rebirth/libmve/mveplay.cpp +++ b/d2x-rebirth/libmve/mveplay.cpp @@ -261,7 +261,7 @@ struct MVE_audio_clamp static int audiobuf_created = 0; static void mve_audio_callback(void *userdata, unsigned char *stream, int len); static array, TOTAL_AUDIO_BUFFERS> mve_audio_buffers; -static int mve_audio_buflens[TOTAL_AUDIO_BUFFERS]; +static array mve_audio_buflens; static int mve_audio_curbuf_curpos=0; static int mve_audio_bufhead=0; static int mve_audio_buftail=0; @@ -393,8 +393,7 @@ static int create_audiobuf_handler(unsigned char, unsigned char minor, const uns #endif mve_audio_buffers = {}; - memset(mve_audio_buflens, 0, sizeof(mve_audio_buflens)); - + mve_audio_buflens = {}; return 1; } @@ -415,17 +414,6 @@ static int play_audio_handler(unsigned char, unsigned char, const unsigned char static int audio_data_handler(unsigned char major, unsigned char, const unsigned char *data, int, void *) { - -#if DXX_USE_SDLMIXER - // MD2211: for audio conversion - SDL_AudioCVT cvt; - int clen; - int out_freq; - Uint16 out_format; - int out_channels; - // end MD2211 -#endif - static const int selected_chan=1; int chan; int nsamp; @@ -474,37 +462,44 @@ static int audio_data_handler(unsigned char major, unsigned char, const unsigned p.reset(reinterpret_cast(mve_alloc(nsamp))); memset(p.get(), 0, nsamp); /* XXX */ } - mve_audio_buflens[mve_audio_buftail] = nsamp; - mve_audio_buffers[mve_audio_buftail] = std::move(p); + unsigned buflen = nsamp; // MD2211: the following block does on-the-fly audio conversion for SDL_mixer #if DXX_USE_SDLMIXER if (!CGameArg.SndDisableSdlMixer) { // build converter: in = MVE format, out = SDL_mixer output + int out_freq; + Uint16 out_format; + int out_channels; Mix_QuerySpec(&out_freq, &out_format, &out_channels); // get current output settings + SDL_AudioCVT cvt{}; SDL_BuildAudioCVT(&cvt, mve_audio_spec->format, mve_audio_spec->channels, mve_audio_spec->freq, out_format, out_channels, out_freq); - clen = nsamp * cvt.len_mult; RAIIdmem cvtbuf; - MALLOC(cvtbuf, uint8_t[], clen); + MALLOC(cvtbuf, uint8_t[], nsamp * cvt.len_mult); cvt.buf = cvtbuf.get(); cvt.len = nsamp; // read the audio buffer into the conversion buffer - memcpy(cvt.buf, mve_audio_buffers[mve_audio_buftail].get(), nsamp); + memcpy(cvt.buf, p.get(), nsamp); // do the conversion if (SDL_ConvertAudio(&cvt)) - con_puts(CON_DEBUG,"audio conversion failed!"); - + con_printf(CON_URGENT, "%s:%u: SDL_ConvertAudio failed: nsamp=%u out_format=%i out_channels=%i out_freq=%i", __FILE__, __LINE__, nsamp, out_format, out_channels, out_freq); + else + { // copy back to the audio buffer - mve_audio_buffers[mve_audio_buftail].reset(reinterpret_cast(mve_alloc(clen))); // free the old audio buffer - mve_audio_buflens[mve_audio_buftail] = clen; - memcpy(mve_audio_buffers[mve_audio_buftail].get(), cvt.buf, clen); + const std::size_t converted_buffer_size = cvt.len_cvt; + p.reset(reinterpret_cast(mve_alloc(converted_buffer_size))); // free the old audio buffer + buflen = converted_buffer_size; + memcpy(p.get(), cvt.buf, converted_buffer_size); + } } #endif + mve_audio_buffers[mve_audio_buftail] = std::move(p); + mve_audio_buflens[mve_audio_buftail] = buflen; if (++mve_audio_buftail == TOTAL_AUDIO_BUFFERS) mve_audio_buftail = 0; @@ -775,7 +770,7 @@ void MVE_rmEndMovie(std::unique_ptr) mve_audio_canplay = 0; } mve_audio_buffers = {}; - memset(mve_audio_buflens, 0, sizeof(mve_audio_buflens)); + mve_audio_buflens = {}; mve_audio_curbuf_curpos=0; mve_audio_bufhead=0;