From 874714a40d71f602b6067fc3d802fbd9bd759240 Mon Sep 17 00:00:00 2001 From: Christian Semmler Date: Sun, 15 Dec 2024 16:29:41 -0700 Subject: [PATCH 1/2] Fix repeating sounds in `MxWavePresenter` (#35) --- LEGO1/omni/include/mxwavepresenter.h | 1 + LEGO1/omni/src/audio/mxwavepresenter.cpp | 62 ++++++++++++++++++------ 2 files changed, 49 insertions(+), 14 deletions(-) diff --git a/LEGO1/omni/include/mxwavepresenter.h b/LEGO1/omni/include/mxwavepresenter.h index 0e5e5536..21b16454 100644 --- a/LEGO1/omni/include/mxwavepresenter.h +++ b/LEGO1/omni/include/mxwavepresenter.h @@ -38,6 +38,7 @@ 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 diff --git a/LEGO1/omni/src/audio/mxwavepresenter.cpp b/LEGO1/omni/src/audio/mxwavepresenter.cpp index 71d1adf7..35ea954e 100644 --- a/LEGO1/omni/src/audio/mxwavepresenter.cpp +++ b/LEGO1/omni/src/audio/mxwavepresenter.cpp @@ -108,18 +108,6 @@ void MxWavePresenter::StartingTickle() assert(m_waveFormat->m_formatTag == g_supportedFormatTag); assert(m_waveFormat->m_bitsPerSample == 8 || m_waveFormat->m_bitsPerSample == 16); - // [library:audio] - // The original game supported a looping/repeating action mode which apparently - // went unused in the retail version of the game (at least for audio). - // It is only ever "used" on startup to load two dummy sounds which are - // initially disabled and never play: IsleScript::c_TransitionSound1 and IsleScript::c_TransitionSound2 - // If MxDSAction::c_looping was set, MxWavePresenter kept the entire sound track - // in a buffer, presumably to allow random seeking and looping. This functionality - // has most likely been superseded by the looping mechanism implemented in the streaming layer. - // Since this has gone unused and to reduce complexity, we don't allow this anymore; - // except for the two dummy sounds, which must be !IsEnabled() - assert(!(m_action->GetFlags() & MxDSAction::c_looping) || !IsEnabled()); - if (m_waveFormat->m_bitsPerSample == 8) { m_silenceData = 0x7F; } @@ -128,11 +116,17 @@ void MxWavePresenter::StartingTickle() m_silenceData = 0; } + // [library:audio] + // If we have a repeating action (MxDSAction::c_looping set), we must make sure the ring buffer + // is large enough to contain the entire sound at once. The size must be a multiple of `g_millisecondsPerChunk` 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( - g_rbSizeInMilliseconds, + 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, @@ -194,6 +188,15 @@ 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() { @@ -206,9 +209,29 @@ void MxWavePresenter::DoneTickle() // FUNCTION: LEGO1 0x100b2130 void MxWavePresenter::LoopChunk(MxStreamChunk* p_chunk) { - WriteToSoundBuffer(p_chunk->GetData(), p_chunk->GetLength()); + // [library:audio] + // 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; } } @@ -236,6 +259,17 @@ MxResult MxWavePresenter::PutData() break; } + assert(!ma_sound_is_playing(&m_sound)); + + // [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; } From cfa3769abff4757add2c53cc6cb51654a0ad1f31 Mon Sep 17 00:00:00 2001 From: Christian Semmler Date: Mon, 16 Dec 2024 14:13:52 -0700 Subject: [PATCH 2/2] Use macros for `Seek` modes (#1235) * Use macros for `Seek` modes * Fix syntax * Use `OF_READ` * Add names to skip.yml * Revert "Add names to skip.yml" This reverts commit 28b6f577dc0c47070064070f4655a15dab3bda25. --- LEGO1/lego/legoomni/src/entity/legoworldpresenter.cpp | 4 ++-- LEGO1/modeldb/modeldb.cpp | 10 ++++++++-- LEGO1/omni/include/mxdsfile.h | 2 +- LEGO1/omni/include/mxdssource.h | 2 +- LEGO1/omni/src/common/mxutilities.cpp | 4 ++-- LEGO1/omni/src/stream/mxdiskstreamprovider.cpp | 6 +++--- LEGO1/omni/src/stream/mxdsfile.cpp | 2 +- LEGO1/omni/src/stream/mxramstreamprovider.cpp | 4 ++-- 8 files changed, 20 insertions(+), 14 deletions(-) diff --git a/LEGO1/lego/legoomni/src/entity/legoworldpresenter.cpp b/LEGO1/lego/legoomni/src/entity/legoworldpresenter.cpp index cbf711ae..c4b8c52e 100644 --- a/LEGO1/lego/legoomni/src/entity/legoworldpresenter.cpp +++ b/LEGO1/lego/legoomni/src/entity/legoworldpresenter.cpp @@ -319,7 +319,7 @@ MxResult LegoWorldPresenter::FUN_10067360(ModelDbPart& p_part, FILE* p_wdbFile) MxResult result; MxU8* buff = new MxU8[p_part.m_partDataLength]; - fseek(p_wdbFile, p_part.m_partDataOffset, 0); + fseek(p_wdbFile, p_part.m_partDataOffset, SEEK_SET); if (fread(buff, p_part.m_partDataLength, 1, p_wdbFile) != 1) { return FAILURE; } @@ -344,7 +344,7 @@ MxResult LegoWorldPresenter::FUN_100674b0(ModelDbModel& p_model, FILE* p_wdbFile { MxU8* buff = new MxU8[p_model.m_unk0x04]; - fseek(p_wdbFile, p_model.m_unk0x08, 0); + fseek(p_wdbFile, p_model.m_unk0x08, SEEK_SET); if (fread(buff, p_model.m_unk0x04, 1, p_wdbFile) != 1) { return FAILURE; } diff --git a/LEGO1/modeldb/modeldb.cpp b/LEGO1/modeldb/modeldb.cpp index 2ac3f978..c56e62b7 100644 --- a/LEGO1/modeldb/modeldb.cpp +++ b/LEGO1/modeldb/modeldb.cpp @@ -52,8 +52,11 @@ MxResult ModelDbModel::Read(FILE* p_file) if (fread(&m_up, sizeof(*m_up), 3, p_file) != 3) { return FAILURE; } + if (fread(&m_unk0x34, sizeof(m_unk0x34), 1, p_file) != 1) { + return FAILURE; + } - return fread(&m_unk0x34, sizeof(m_unk0x34), 1, p_file) == 1 ? SUCCESS : FAILURE; + return SUCCESS; } // FUNCTION: LEGO1 0x10027850 @@ -76,8 +79,11 @@ MxResult ModelDbPart::Read(FILE* p_file) if (fread(&m_partDataLength, sizeof(m_partDataLength), 1, p_file) != 1) { return FAILURE; } + if (fread(&m_partDataOffset, sizeof(m_partDataOffset), 1, p_file) != 1) { + return FAILURE; + } - return fread(&m_partDataOffset, sizeof(m_partDataOffset), 1, p_file) == 1 ? SUCCESS : FAILURE; + return SUCCESS; } // FUNCTION: LEGO1 0x10027910 diff --git a/LEGO1/omni/include/mxdsfile.h b/LEGO1/omni/include/mxdsfile.h index 09772b68..f15139f9 100644 --- a/LEGO1/omni/include/mxdsfile.h +++ b/LEGO1/omni/include/mxdsfile.h @@ -43,7 +43,7 @@ class MxDSFile : public MxDSSource { MxResult Open(MxULong) override; // vtable+0x14 MxResult Close() override; // vtable+0x18 MxResult Read(unsigned char*, MxULong) override; // vtable+0x20 - MxResult Seek(MxLong, int) override; // vtable+0x24 + MxResult Seek(MxLong, MxS32) override; // vtable+0x24 MxULong GetBufferSize() override; // vtable+0x28 MxULong GetStreamBuffersNum() override; // vtable+0x2c diff --git a/LEGO1/omni/include/mxdssource.h b/LEGO1/omni/include/mxdssource.h index 352ee2c5..06f5e556 100644 --- a/LEGO1/omni/include/mxdssource.h +++ b/LEGO1/omni/include/mxdssource.h @@ -31,7 +31,7 @@ class MxDSSource : public MxCore { virtual MxLong Close() = 0; // vtable+0x18 virtual MxResult ReadToBuffer(MxDSBuffer* p_buffer); // vtable+0x1c virtual MxResult Read(unsigned char*, MxULong) = 0; // vtable+0x20 - virtual MxLong Seek(MxLong, int) = 0; // vtable+0x24 + virtual MxLong Seek(MxLong, MxS32) = 0; // vtable+0x24 virtual MxULong GetBufferSize() = 0; // vtable+0x28 virtual MxULong GetStreamBuffersNum() = 0; // vtable+0x2c virtual MxLong GetLengthInDWords(); // vtable+0x30 diff --git a/LEGO1/omni/src/common/mxutilities.cpp b/LEGO1/omni/src/common/mxutilities.cpp index fe45327e..c943bde5 100644 --- a/LEGO1/omni/src/common/mxutilities.cpp +++ b/LEGO1/omni/src/common/mxutilities.cpp @@ -12,7 +12,7 @@ #include // GLOBAL: LEGO1 0x101020e8 -void (*g_omniUserMessage)(const char*, int) = NULL; +void (*g_omniUserMessage)(const char*, MxS32) = NULL; // FUNCTION: LEGO1 0x100b6e10 MxBool GetRectIntersection( @@ -166,7 +166,7 @@ MxDSObject* CreateStreamObject(MxDSFile* p_file, MxS16 p_ofs) MxU8* buf; _MMCKINFO tmpChunk; - if (p_file->Seek(((MxLong*) p_file->GetBuffer())[p_ofs], 0)) { + if (p_file->Seek(((MxLong*) p_file->GetBuffer())[p_ofs], SEEK_SET)) { return NULL; } diff --git a/LEGO1/omni/src/stream/mxdiskstreamprovider.cpp b/LEGO1/omni/src/stream/mxdiskstreamprovider.cpp index 0e100ef9..1f7dba15 100644 --- a/LEGO1/omni/src/stream/mxdiskstreamprovider.cpp +++ b/LEGO1/omni/src/stream/mxdiskstreamprovider.cpp @@ -92,11 +92,11 @@ MxResult MxDiskStreamProvider::SetResourceToGet(MxStreamController* p_resource) m_pFile = new MxDSFile(path.GetData(), 0); if (m_pFile != NULL) { - if (m_pFile->Open(0) != 0) { + if (m_pFile->Open(OF_READ) != 0) { path = MxString(MxOmni::GetCD()) + p_resource->GetAtom().GetInternal() + ".si"; m_pFile->SetFileName(path.GetData()); - if (m_pFile->Open(0) != 0) { + if (m_pFile->Open(OF_READ) != 0) { goto done; } } @@ -245,7 +245,7 @@ void MxDiskStreamProvider::PerformWork() buffer = streamingAction->GetUnknowna0(); if (m_pFile->GetPosition() == streamingAction->GetBufferOffset() || - m_pFile->Seek(streamingAction->GetBufferOffset(), 0) == 0) { + m_pFile->Seek(streamingAction->GetBufferOffset(), SEEK_SET) == 0) { buffer->SetUnknown14(m_pFile->GetPosition()); if (m_pFile->ReadToBuffer(buffer) == SUCCESS) { diff --git a/LEGO1/omni/src/stream/mxdsfile.cpp b/LEGO1/omni/src/stream/mxdsfile.cpp index 0f3b5227..54195f92 100644 --- a/LEGO1/omni/src/stream/mxdsfile.cpp +++ b/LEGO1/omni/src/stream/mxdsfile.cpp @@ -41,7 +41,7 @@ MxResult MxDSFile::Open(MxULong p_uStyle) Close(); } else { - Seek(0, 0); + Seek(0, SEEK_SET); } return result; diff --git a/LEGO1/omni/src/stream/mxramstreamprovider.cpp b/LEGO1/omni/src/stream/mxramstreamprovider.cpp index 55d282b3..d655c16f 100644 --- a/LEGO1/omni/src/stream/mxramstreamprovider.cpp +++ b/LEGO1/omni/src/stream/mxramstreamprovider.cpp @@ -68,11 +68,11 @@ MxResult MxRAMStreamProvider::SetResourceToGet(MxStreamController* p_resource) m_pFile = new MxDSFile(path.GetData(), 0); if (m_pFile != NULL) { - if (m_pFile->Open(0) != 0) { + if (m_pFile->Open(OF_READ) != 0) { path = MxString(MxOmni::GetCD()) + p_resource->GetAtom().GetInternal() + ".si"; m_pFile->SetFileName(path.GetData()); - if (m_pFile->Open(0) != 0) { + if (m_pFile->Open(OF_READ) != 0) { goto done; } }