Fix repeating sounds in MxWavePresenter (again) (#40)

* Commit

* Use audio buffer

* Add assert for buffer

* Fix naming

LEGO1/omni/include/mxwavepresenter.h

@@ -38,7 +38,6 @@ class MxWavePresenter : public MxSoundPresenter {
 	void ReadyTickle() override;      // vtable+0x18
 	void StartingTickle() override;   // vtable+0x1c
 	void StreamingTickle() override;  // vtable+0x20
-	void RepeatingTickle() override;  // added by isle-portable
 	void DoneTickle() override;       // vtable+0x2c
 	void ParseExtra() override;       // vtable+0x30
 	MxResult AddToManager() override; // vtable+0x34
@@ -89,7 +88,18 @@ class MxWavePresenter : public MxSoundPresenter {
 	static const MxU32 g_supportedFormatTag = 1;
 
 	WaveFormat* m_waveFormat; // 0x54
+
+	// [library:audio]
+	// If MxDSAction::looping is set, we keep the entire audio in memory and use `m_ab`.
+	// In (most) other cases, data is streamed through the ring buffer `m_rb`.
 	ma_pcm_rb m_rb;
+	struct {
+		ma_audio_buffer m_buffer;
+		MxU8* m_data;
+		MxU32 m_length;
+		MxU32 m_offset;
+	} m_ab;
+
 	ma_sound m_sound;
 	MxU32 m_chunkLength; // 0x5c
 	MxBool m_started;    // 0x65

LEGO1/omni/src/audio/mxwavepresenter.cpp

@@ -20,6 +20,7 @@ void MxWavePresenter::Init()
 {
 	m_waveFormat = NULL;
 	SDL_zero(m_rb);
+	SDL_zero(m_ab);
 	SDL_zero(m_sound);
 	m_chunkLength = 0;
 	m_started = FALSE;
@@ -40,6 +41,8 @@ void MxWavePresenter::Destroy(MxBool p_fromDestructor)
 {
 	ma_sound_uninit(&m_sound);
 	ma_pcm_rb_uninit(&m_rb);
+	ma_audio_buffer_uninit(&m_ab.m_buffer);
+	delete[] m_ab.m_data;
 
 	if (m_waveFormat) {
 		delete[] ((MxU8*) m_waveFormat);
@@ -55,31 +58,39 @@ void MxWavePresenter::Destroy(MxBool p_fromDestructor)
 // FUNCTION: LEGO1 0x100b1bd0
 MxBool MxWavePresenter::WriteToSoundBuffer(void* p_audioPtr, MxU32 p_length)
 {
-	ma_uint32 requestedFrames =
+	if (m_action->IsLooping()) {
-		ma_calculate_buffer_size_in_frames_from_milliseconds(g_millisecondsPerChunk, m_waveFormat->m_samplesPerSec);
+		assert(m_ab.m_offset + p_length <= m_ab.m_length);
-	ma_uint32 acquiredFrames = requestedFrames;
+		memcpy(m_ab.m_data + m_ab.m_offset, p_audioPtr, p_length);
-	void* bufferOut;
+		m_ab.m_offset += p_length;
-
+		return TRUE;
-	ma_pcm_rb_acquire_write(&m_rb, &acquiredFrames, &bufferOut);
-
-	// [library:audio] If there isn't enough space in the buffer for a full chunk, try again later.
-	if (acquiredFrames != requestedFrames) {
-		ma_pcm_rb_commit_write(&m_rb, 0);
-		return FALSE;
 	}
+	else {
+		ma_uint32 requestedFrames =
+			ma_calculate_buffer_size_in_frames_from_milliseconds(g_millisecondsPerChunk, m_waveFormat->m_samplesPerSec);
+		ma_uint32 acquiredFrames = requestedFrames;
+		void* bufferOut;
 
-	ma_uint32 acquiredBytes = acquiredFrames * ma_get_bytes_per_frame(m_rb.format, m_rb.channels);
+		ma_pcm_rb_acquire_write(&m_rb, &acquiredFrames, &bufferOut);
-	assert(p_length <= acquiredBytes);
 
-	memcpy(bufferOut, p_audioPtr, p_length);
+		// [library:audio] If there isn't enough space in the buffer for a full chunk, try again later.
+		if (acquiredFrames != requestedFrames) {
+			ma_pcm_rb_commit_write(&m_rb, 0);
+			return FALSE;
+		}
 
-	// [library:audio] Pad with silence data if we don't have a full chunk.
+		ma_uint32 acquiredBytes = acquiredFrames * ma_get_bytes_per_frame(m_rb.format, m_rb.channels);
-	if (p_length < acquiredBytes) {
+		assert(p_length <= acquiredBytes);
-		memset((ma_uint8*) bufferOut + p_length, m_silenceData, acquiredBytes - p_length);
+
+		memcpy(bufferOut, p_audioPtr, p_length);
+
+		// [library:audio] Pad with silence data if we don't have a full chunk.
+		if (p_length < acquiredBytes) {
+			memset((ma_uint8*) bufferOut + p_length, m_silenceData, acquiredBytes - p_length);
+		}
+
+		ma_pcm_rb_commit_write(&m_rb, acquiredFrames);
+		return TRUE;
 	}
-
-	ma_pcm_rb_commit_write(&m_rb, acquiredFrames);
-	return TRUE;
 }
 
 // FUNCTION: LEGO1 0x100b1cf0
@@ -116,31 +127,45 @@ void MxWavePresenter::StartingTickle()
 			m_silenceData = 0;
 		}
 
-		// [library:audio]
+		ma_format format = m_waveFormat->m_bitsPerSample == 16 ? ma_format_s16 : ma_format_u8;
-		// If we have a repeating action (MxDSAction::c_looping set), we must make sure the ring buffer
+		ma_uint32 channels = m_waveFormat->m_channels;
-		// is large enough to contain the entire sound at once. The size must be a multiple of `g_millisecondsPerChunk`
+		ma_uint32 sampleRate = m_waveFormat->m_samplesPerSec;
-		if (ma_pcm_rb_init(
-				m_waveFormat->m_bitsPerSample == 16 ? ma_format_s16 : ma_format_u8,
-				m_waveFormat->m_channels,
-				ma_calculate_buffer_size_in_frames_from_milliseconds(
-					m_action->GetFlags() & MxDSAction::c_looping
-						? (m_action->GetDuration() / m_action->GetLoopCount() + g_millisecondsPerChunk - 1) /
-							  g_millisecondsPerChunk * g_millisecondsPerChunk
-						: g_rbSizeInMilliseconds,
-					m_waveFormat->m_samplesPerSec
-				),
-				NULL,
-				NULL,
-				&m_rb
-			) != MA_SUCCESS) {
-			goto done;
-		}
 
-		ma_pcm_rb_set_sample_rate(&m_rb, m_waveFormat->m_samplesPerSec);
+		if (m_action->IsLooping()) {
+			ma_uint32 sizeInFrames = ma_calculate_buffer_size_in_frames_from_milliseconds(
+				m_action->GetDuration() / m_action->GetLoopCount(),
+				sampleRate
+			);
+
+			m_ab.m_length = ma_get_bytes_per_frame(format, channels) * sizeInFrames;
+			m_ab.m_data = new MxU8[m_ab.m_length];
+
+			ma_audio_buffer_config config =
+				ma_audio_buffer_config_init(format, channels, sizeInFrames, m_ab.m_data, NULL);
+			config.sampleRate = sampleRate;
+
+			if (ma_audio_buffer_init(&config, &m_ab.m_buffer) != MA_SUCCESS) {
+				goto done;
+			}
+		}
+		else {
+			if (ma_pcm_rb_init(
+					format,
+					channels,
+					ma_calculate_buffer_size_in_frames_from_milliseconds(g_rbSizeInMilliseconds, sampleRate),
+					NULL,
+					NULL,
+					&m_rb
+				) != MA_SUCCESS) {
+				goto done;
+			}
+
+			ma_pcm_rb_set_sample_rate(&m_rb, sampleRate);
+		}
 
 		if (ma_sound_init_from_data_source(
 				MSoundManager()->GetEngine(),
-				&m_rb,
+				m_action->IsLooping() ? (ma_data_source*) &m_ab.m_buffer : (ma_data_source*) &m_rb,
 				m_is3d ? 0 : MA_SOUND_FLAG_NO_SPATIALIZATION,
 				NULL,
 				&m_sound
@@ -148,7 +173,7 @@ void MxWavePresenter::StartingTickle()
 			goto done;
 		}
 
-		ma_sound_set_looping(&m_sound, MA_TRUE);
+		ma_sound_set_looping(&m_sound, m_action->IsLooping() ? m_action->GetLoopCount() > 1 : MA_TRUE);
 
 		SetVolume(((MxDSSound*) m_action)->GetVolume());
 		ProgressTickleState(e_streaming);
@@ -188,20 +213,11 @@ void MxWavePresenter::StreamingTickle()
 	}
 }
 
-void MxWavePresenter::RepeatingTickle()
-{
-	if (IsEnabled() && !m_currentChunk) {
-		if (m_action->GetElapsedTime() >= m_action->GetStartTime() + m_action->GetDuration()) {
-			ProgressTickleState(e_freezing);
-		}
-	}
-}
-
 // FUNCTION: LEGO1 0x100b20c0
 void MxWavePresenter::DoneTickle()
 {
 	if (!ma_sound_get_engine(&m_sound) || m_action->GetFlags() & MxDSAction::c_bit7 ||
-		ma_pcm_rb_pointer_distance(&m_rb) == 0) {
+		m_action->GetFlags() & MxDSAction::c_looping || ma_pcm_rb_pointer_distance(&m_rb) == 0) {
 		MxMediaPresenter::DoneTickle();
 	}
 }
@@ -209,29 +225,9 @@ void MxWavePresenter::DoneTickle()
 // FUNCTION: LEGO1 0x100b2130
 void MxWavePresenter::LoopChunk(MxStreamChunk* p_chunk)
 {
-	// [library:audio]
+	WriteToSoundBuffer(p_chunk->GetData(), p_chunk->GetLength());
-	// The original code writes all the chunks directly into the buffer. However, since we are using
-	// a ring buffer instead, we cannot do that. Instead, we use the original code's `m_loopingChunks`
-	// to store permanent copies of all the chunks. (`MxSoundPresenter::LoopChunk`)
-	// These will then be used to write them all at once to the ring buffer when necessary.
-	MxSoundPresenter::LoopChunk(p_chunk);
-
-	assert(m_action->GetFlags() & MxDSAction::c_looping);
-
-	// [library:audio]
-	// We don't currently support a loop count greater than 1 for repeating actions.
-	// However, there don't seem to be any such actions in the game.
-	assert(m_action->GetLoopCount() == 1);
-
-	// [library:audio]
-	// So far there are no known cases where the sound is initially enabled if it's set to repeat.
-	// While we can technically support this (see branch below), this should be tested.
-	assert(!IsEnabled());
-
 	if (IsEnabled()) {
-		WriteToSoundBuffer(p_chunk->GetData(), p_chunk->GetLength());
 		m_subscriber->FreeDataChunk(p_chunk);
-		m_currentChunk = NULL;
 	}
 }
 
@@ -260,15 +256,7 @@ MxResult MxWavePresenter::PutData()
 			}
 
 			assert(!ma_sound_is_playing(&m_sound));
-
+			ma_sound_seek_to_pcm_frame(&m_sound, 0);
-			// [library:audio]
-			// We push all the repeating chunks at once into the buffer.
-			// This should never fail, since the buffer is ensured to be large enough to contain the entire sound.
-			while (m_loopingChunkCursor->Next(m_currentChunk)) {
-				assert(WriteToSoundBuffer(m_currentChunk->GetData(), m_currentChunk->GetLength()));
-			}
-
-			m_currentChunk = NULL;
 
 			if (ma_sound_start(&m_sound) == MA_SUCCESS) {
 				m_started = TRUE;
@@ -361,7 +349,16 @@ void MxWavePresenter::Resume()
 {
 	if (m_paused) {
 		if (ma_sound_get_engine(&m_sound) && m_started) {
-			ma_sound_start(&m_sound);
+			switch (m_currentTickleState) {
+			case e_streaming:
+			case e_repeating:
+				ma_sound_start(&m_sound);
+				break;
+			case e_done:
+				if (!ma_sound_at_end(&m_sound)) {
+					ma_sound_start(&m_sound);
+				}
+			}
 		}
 
 		m_paused = FALSE;