From d1d9c4fcc0d6b76764ae8923cadd0f6eb3915a6c Mon Sep 17 00:00:00 2001 From: nyanpasu64 Date: Wed, 4 Jun 2025 07:09:27 -0700 Subject: [PATCH 1/2] Fix debug assert error after failing to export WAV file We would fail to clear m_pWaveFile if we failed to start rendering. On the next render attempt, this would cause the assertion `ASSERT(m_pWaveFile == nullptr)` to fail. Manually clear m_pWaveFile to prevent this from happening. If mmioOpen fails, hmmioOut is nullptr and we don't need to call CloseFile(). --- Source/SoundGen.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Source/SoundGen.cpp b/Source/SoundGen.cpp index bcb1ad12..0d0dc3ea 100644 --- a/Source/SoundGen.cpp +++ b/Source/SoundGen.cpp @@ -2098,6 +2098,11 @@ bool CSoundGen::RenderToFile(LPTSTR pFile, render_end_t SongEndType, int SongEnd if (!m_pWaveFile || !m_pWaveFile->OpenFile(pFile, theApp.GetSettings()->Sound.iSampleRate, 16, 1)) { AfxMessageBox(IDS_FILE_OPEN_ERROR); + + // When writing to a locked file, hmmioOut is nullptr so we don't need to call + // m_pWaveFile->CloseFile(). + m_pWaveFile.reset(); + return false; } else { From 3aa6fdceee7c946429116a967697a605f4f3cc7d Mon Sep 17 00:00:00 2001 From: nyanpasu64 Date: Wed, 4 Jun 2025 07:09:27 -0700 Subject: [PATCH 2/2] Add logging to catch WAV export crashes The modulo logic is fiddly, but I confirmed it works with various log item counts (by reducing LOG_SIZE to 4, pushing items with and without overflow, and verifying the tail output is correct). --- Source/SoundGen.cpp | 142 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 141 insertions(+), 1 deletion(-) diff --git a/Source/SoundGen.cpp b/Source/SoundGen.cpp index 0d0dc3ea..d7520305 100644 --- a/Source/SoundGen.cpp +++ b/Source/SoundGen.cpp @@ -50,6 +50,9 @@ #include "MIDI.h" #include "ChannelFactory.h" // // // test #include "DetuneTable.h" // // // +#include +#include +#include #include #include @@ -71,6 +74,110 @@ // Enable audio dithering //#define DITHERING +struct LogEntry { + DWORD child_tid; + const char /*'static*/* event; +}; + +namespace { +class Log { + static constexpr size_t LOG_SIZE = 256; + +public: + explicit Log() {} + + void log(const char /*'static*/* event) { + auto lock = std::unique_lock(_mtx); + return log_(event); + } + +private: + void log_(const char /*'static*/* event) { + const auto tid = GetCurrentThreadId(); + const auto child_tid = (tid == theApp.m_nThreadID) + ? 0 + : tid; + + _log[wrap(_end2)] = LogEntry{ + /*.child_tid = */ child_tid, + /*.event = */ event, + }; + _end2++; + if (_end2 - _begin2 > LOG_SIZE) { + _begin2 = _end2 - LOG_SIZE; + if (_begin2 >= LOG_SIZE) { + _begin2 -= LOG_SIZE; + _end2 -= LOG_SIZE; + } + } + } + +public: + void dump(const char /*'static*/* event = nullptr) { + // there should be no possiblity of deadlock since we never block while owning _mtx. + auto lock = std::unique_lock(_mtx); + + if (event) { + log_(event); + } + + // the children yearn for the ~~mines~~ std::format + char fname[99]; + while (true) { + snprintf(fname, sizeof(fname), "wavlog-%d.txt", _generation); + if (!std::filesystem::exists(fname)) { + break; + } + _generation++; + } + + auto f = fopen(fname, "w"); + if (!f) { + lock.unlock(); + MessageBoxA(nullptr, + "WAV export error, failed to dump logs.", + "WAV export error", + MB_OK | MB_ICONERROR); + return; + } + + std::string s; + for (unsigned index2 = _begin2; index2 != _end2; index2++) { + LogEntry const& entry = _log[wrap(index2)]; + + // pray it works. if not, nothing we can do. + fprintf(f, "(%d) %s\n", entry.child_tid, entry.event); + } + // nothing we can do with an error. + fclose(f); + + _generation++; + + log_("dump()"); + MessageBoxA(nullptr, + "WAV export error, please report and attach wavlog-*.txt", + "WAV export error", + MB_OK | MB_ICONERROR); + } + +private: + unsigned wrap(unsigned index) const { + return index % LOG_SIZE; + } + + // fields +private: + std::mutex _mtx; + std::array _log{}; + unsigned _begin2 = 0; + unsigned _end2 = 0; + + int _generation = 0; +}; + +Log LOGGER; +} + // The depth of each vibrato level const double CSoundGen::NEW_VIBRATO_DEPTH[] = { 1.0, 1.5, 2.5, 4.0, 5.0, 7.0, 10.0, 12.0, 14.0, 17.0, 22.0, 30.0, 44.0, 64.0, 96.0, 128.0 @@ -811,6 +918,10 @@ bool CSoundGen::PostGuiMessage(GuiMessageId message, WPARAM wParam, LPARAM lPara // Called from main thread ASSERT(GetCurrentThreadId() == theApp.m_nThreadID); + if (message == WM_USER_STOP_RENDER) { + LOGGER.log("CSoundGen::GuiPostMessage(WM_USER_STOP_RENDER)"); + } + return m_MessageQueue.try_push(GuiMessage{ message, wParam, @@ -1132,6 +1243,9 @@ void CSoundGen::FillBuffer(int16_t const * pBuffer, uint32_t Size) if (m_bRendering) { // Output to file + if (!m_pWaveFile) { + LOGGER.dump("CSoundGen::FillBuffer: ASSERT(m_pWaveFile) failed"); + } ASSERT(m_pWaveFile); // // // // This code needs to be changed if we add stereo support. m_pWaveFile->WriteWave((char *) pBuffer, 2 * Size); @@ -2064,6 +2178,8 @@ void CSoundGen::EvaluateGlobalEffects(stChanNote *NoteData, int EffColumns) bool CSoundGen::RenderToFile(LPTSTR pFile, render_end_t SongEndType, int SongEndParam, int Track) { + LOGGER.log("{ CSoundGen::RenderToFile"); + // Called from main thread ASSERT(GetCurrentThreadId() == theApp.m_nThreadID); ASSERT(m_pDocument != NULL); @@ -2091,8 +2207,15 @@ bool CSoundGen::RenderToFile(LPTSTR pFile, render_end_t SongEndType, int SongEnd m_iRenderRowCount = m_iRenderEndParam; } + if (m_bRendering) { + LOGGER.dump("ASSERT(!m_bRendering) failed"); + } ASSERT(!m_bRendering); + if (m_pWaveFile) { + LOGGER.dump("ASSERT(m_pWaveFile == nullptr) failed"); + } ASSERT(m_pWaveFile == nullptr); + LOGGER.log("m_pWaveFile = std::make_unique();"); m_pWaveFile = std::make_unique(); // Unfortunately, destructor doesn't cleanup object. Only CloseFile() does. if (!m_pWaveFile || @@ -2103,31 +2226,43 @@ bool CSoundGen::RenderToFile(LPTSTR pFile, render_end_t SongEndType, int SongEnd // m_pWaveFile->CloseFile(). m_pWaveFile.reset(); + LOGGER.log("} RenderToFile error"); return false; } else { m_bRequestRenderStart = true; + LOGGER.log("CSoundGen::GuiPostMessage(WM_USER_START_RENDER)"); PostGuiMessage(WM_USER_START_RENDER, 0, 0); } + LOGGER.log("} CSoundGen::RenderToFile"); return true; } void CSoundGen::StopRendering() { + LOGGER.log("{ CSoundGen::StopRendering"); + // Called from player thread ASSERT(std::this_thread::get_id() == m_audioThreadID); + if (!m_bRendering) { + LOGGER.dump("ASSERT(m_bRendering) failed"); + } ASSERT(m_bRendering); auto l = Lock(); - if (!IsRendering()) + if (!IsRendering()) { + LOGGER.log("} !IsRendering(), CSoundGen::StopRendering"); return; + } m_bPlaying = false; m_bRendering = false; m_bStoppingRender = false; // // // m_bRequestRenderStop = false; // // // + m_iDelayedStart = 0; + m_iDelayedEnd = 0; m_iPlayFrame = 0; m_iPlayRow = 0; m_pWaveFile->CloseFile(); // // // @@ -2136,6 +2271,7 @@ void CSoundGen::StopRendering() ResetBuffer(); ResetAPU(); // // // HaltPlayer(); + LOGGER.log("} CSoundGen::StopRendering"); } void CSoundGen::GetRenderStat(int &Frame, int &Time, bool &Done, int &FramesToRender, int &Row, int &RowCount) const @@ -2356,6 +2492,7 @@ void CSoundGen::OnIdle() if (m_iDelayedStart > 0) { --m_iDelayedStart; if (!m_iDelayedStart) { + LOGGER.log("!m_iDelayedStart -> WM_USER_PLAY"); PostSelfMessage(WM_USER_PLAY, MODE_PLAY_START, m_iRenderTrack); } } @@ -2531,7 +2668,9 @@ void CSoundGen::OnResetPlayer(WPARAM wParam, LPARAM lParam) void CSoundGen::OnStartRender(WPARAM wParam, LPARAM lParam) { + LOGGER.log("{ CSoundGen::OnStartRender"); auto l = Lock(); + LOGGER.log("{} Lock()"); ResetBuffer(); m_bRequestRenderStart = false; m_bRequestRenderStop = false; @@ -2539,6 +2678,7 @@ void CSoundGen::OnStartRender(WPARAM wParam, LPARAM lParam) m_bRendering = true; m_iDelayedStart = 5; // Wait 5 frames until player starts m_iDelayedEnd = 5; + LOGGER.log("} CSoundGen::OnStartRender"); } void CSoundGen::OnStopRender(WPARAM wParam, LPARAM lParam)