From bcac1e7955584165760da0ed8d62f8a076a2bbcb Mon Sep 17 00:00:00 2001 From: kaetemi Date: Wed, 1 Apr 2020 12:46:21 +0800 Subject: [PATCH 1/7] Fix silently lost nlassert, ryzom/ryzomcore#596 --- nel/include/nel/misc/debug.h | 8 +++++++- nel/src/misc/co_task.cpp | 4 ++++ nel/src/misc/debug.cpp | 9 +++++++++ nel/src/misc/win_thread.cpp | 3 +++ 4 files changed, 23 insertions(+), 1 deletion(-) diff --git a/nel/include/nel/misc/debug.h b/nel/include/nel/misc/debug.h index a8c23eced..1d533c149 100644 --- a/nel/include/nel/misc/debug.h +++ b/nel/include/nel/misc/debug.h @@ -89,6 +89,9 @@ void createDebug (const char *logPath = NULL, bool logInFile = true, bool eraseL /// Do not call this, unless you know what you're trying to do (it kills debug)! void destroyDebug(); +/// Attach exception handler, for new threads and fibers +void attachExceptionHandler(); + // call this if you want to change the dir of the log.log file void changeLogDirectory(const std::string &dir); @@ -352,7 +355,7 @@ void setCrashAlreadyReported(bool state); * Same as nlassertex(false,exp); */ -// removed because we always check assert (even in release mode) #if defined (NL_OS_WINDOWS) && defined (NL_DEBUG) +#if defined(NL_DEBUG) /* Debug break is only useful in debug builds */ #if defined(NL_OS_WINDOWS) #define NLMISC_BREAKPOINT __debugbreak() #elif defined(NL_OS_UNIX) && defined(NL_COMP_GCC) @@ -360,6 +363,9 @@ void setCrashAlreadyReported(bool state); #else #define NLMISC_BREAKPOINT abort() #endif +#else +#define NLMISC_BREAKPOINT do { } while (0) +#endif // Internal, don't use it (make smaller assert code) extern bool _assert_stop(bool &ignoreNextTime, sint line, const char *file, const char *funcName, const char *exp); diff --git a/nel/src/misc/co_task.cpp b/nel/src/misc/co_task.cpp index 524cc69c6..4548fbe86 100644 --- a/nel/src/misc/co_task.cpp +++ b/nel/src/misc/co_task.cpp @@ -143,6 +143,9 @@ namespace NLMISC NL_CT_DEBUG("CoTask : task %p start func called", task); + // Attach exception handler + attachExceptionHandler(); + try { // run the task @@ -151,6 +154,7 @@ namespace NLMISC catch(...) { nlwarning("CCoTask::startFunc : the task has generated an unhandled exeption and will terminate"); + NLMISC_BREAKPOINT; } task->_Finished = true; diff --git a/nel/src/misc/debug.cpp b/nel/src/misc/debug.cpp index cc3203a14..1f7cf38db 100644 --- a/nel/src/misc/debug.cpp +++ b/nel/src/misc/debug.cpp @@ -1157,6 +1157,15 @@ void destroyDebug() } } +void attachExceptionHandler() +{ +#ifndef NL_COMP_MINGW +# ifdef NL_OS_WINDOWS + _set_se_translator(exceptionTranslator); +# endif // NL_OS_WINDOWS +#endif //!NL_COMP_MINGW +} + void createDebug (const char *logPath, bool logInFile, bool eraseLastLog) { // Do some basic compiler time check on type size diff --git a/nel/src/misc/win_thread.cpp b/nel/src/misc/win_thread.cpp index a75b2165f..0de191d55 100644 --- a/nel/src/misc/win_thread.cpp +++ b/nel/src/misc/win_thread.cpp @@ -67,6 +67,9 @@ static unsigned long __stdcall ProxyFunc (void *arg) // Set the thread pointer in TLS memory nlverify (TlsSetValue (TLSThreadPointer, (void*)parent) != 0); + // Attach exception handler + attachExceptionHandler(); + // Run the thread parent->Runnable->run(); From 83210ce17bff7a3fe80d585f4bdf968a036e21cc Mon Sep 17 00:00:00 2001 From: kaetemi Date: Wed, 1 Apr 2020 14:11:50 +0800 Subject: [PATCH 2/7] Fix stuck loading screen when quitting scenario, fix kaetemi/ryzomclassic#83 --- .../src/interface_v3/inventory_manager.cpp | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/ryzom/client/src/interface_v3/inventory_manager.cpp b/ryzom/client/src/interface_v3/inventory_manager.cpp index 816f676e2..0bb20c844 100644 --- a/ryzom/client/src/interface_v3/inventory_manager.cpp +++ b/ryzom/client/src/interface_v3/inventory_manager.cpp @@ -3648,13 +3648,23 @@ void CInventoryManager::onTradeChangeSession() // *************************************************************************** void CInventoryManager::updateItemInfoQueue() { + if (!ConnectionReadySent) // CONNECTION:READY not yet sent, so we cannot send requests yet! + { + // Caused by CNetworkConnection::reinit, and NLMISC::CCDBNodeBranch::resetData + // TODO: Item sheets are effectively being set to 0, any way to detect this in particular? + // Slots with sheet 0 won't have any info to request anyway! + // Remove this check in favour of the assert lower down when properly implemented. + nlwarning("Update item info queue (%i), but connection not ready", (int)_ItemInfoWaiters.size()); + return; + } + + // CONNECTION:READY not yet sent, so we cannot send requests yet! + nlassert(ConnectionReadySent || !_ItemInfoWaiters.size()); + // For All waiters, look if one need update. TItemInfoWaiters::iterator it; for(it= _ItemInfoWaiters.begin();it!=_ItemInfoWaiters.end();it++) { - /* \todo yoyo remove: temp patch to be sure that the client does not send messages before the - CONNECTION:READY is sent - */ nlassert(ConnectionReadySent); IItemInfoWaiter *waiter=*it; From 87cd9f19a4a4e76e5a57ec1d879a90e3cc2c8d25 Mon Sep 17 00:00:00 2001 From: kaetemi Date: Wed, 1 Apr 2020 14:38:11 +0800 Subject: [PATCH 3/7] Remove duplicate assert --- ryzom/client/src/interface_v3/inventory_manager.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/ryzom/client/src/interface_v3/inventory_manager.cpp b/ryzom/client/src/interface_v3/inventory_manager.cpp index 0bb20c844..f84e38234 100644 --- a/ryzom/client/src/interface_v3/inventory_manager.cpp +++ b/ryzom/client/src/interface_v3/inventory_manager.cpp @@ -3665,8 +3665,6 @@ void CInventoryManager::updateItemInfoQueue() TItemInfoWaiters::iterator it; for(it= _ItemInfoWaiters.begin();it!=_ItemInfoWaiters.end();it++) { - nlassert(ConnectionReadySent); - IItemInfoWaiter *waiter=*it; uint itemSlotId= waiter->ItemSlotId; TItemInfoMap::iterator it= _ItemInfoMap.find(itemSlotId); From 1ab44cf123e0828ec294a8d2a696f547fe8b1e74 Mon Sep 17 00:00:00 2001 From: Nimetu Date: Tue, 31 Mar 2020 17:37:48 +0300 Subject: [PATCH 4/7] Fixed: mp3 player keeps playing after clearing files from playlist --- ryzom/client/src/interface_v3/music_player.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ryzom/client/src/interface_v3/music_player.cpp b/ryzom/client/src/interface_v3/music_player.cpp index f4d8b7db9..717ea7e66 100644 --- a/ryzom/client/src/interface_v3/music_player.cpp +++ b/ryzom/client/src/interface_v3/music_player.cpp @@ -85,7 +85,7 @@ void CMusicPlayer::playSongs (const std::vector &songs) rebuildPlaylist(); // If pause, stop, else play will resume - if (_State == Paused) + if (_State == Paused || _Songs.empty()) _State = Stopped; } From 505283a17cb40e862746c6e04a1e3a66e89211bc Mon Sep 17 00:00:00 2001 From: Nimetu Date: Wed, 1 Apr 2020 17:23:09 +0300 Subject: [PATCH 5/7] Changed: Use worker thread to gather song title/duration. --- .../client/src/interface_v3/music_player.cpp | 244 +++++++++++++++--- ryzom/client/src/interface_v3/music_player.h | 17 +- 2 files changed, 226 insertions(+), 35 deletions(-) diff --git a/ryzom/client/src/interface_v3/music_player.cpp b/ryzom/client/src/interface_v3/music_player.cpp index 717ea7e66..10b4cd288 100644 --- a/ryzom/client/src/interface_v3/music_player.cpp +++ b/ryzom/client/src/interface_v3/music_player.cpp @@ -28,6 +28,9 @@ #include "interface_manager.h" #include "../client_cfg.h" +#include "nel/misc/thread.h" +#include "nel/misc/mutex.h" + using namespace std; using namespace NLMISC; using namespace NL3D; @@ -48,6 +51,88 @@ extern UDriver *Driver; #define MP3_SAVE_REPEAT "UI:SAVE:MP3_REPEAT" CMusicPlayer MusicPlayer; +static NLMISC::CUnfairMutex MusicPlayerMutex; + +// *************************************************************************** +class CMusicPlayerWorker : public NLMISC::IRunnable +{ +private: + bool _Running; + IThread *_Thread; + + std::vector _Files; + +public: + CMusicPlayerWorker(): _Running(false), _Thread(NULL) + { + } + + ~CMusicPlayerWorker() + { + _Running = false; + if (_Thread) + { + _Thread->terminate(); + delete _Thread; + _Thread = NULL; + } + } + + bool isRunning() const { return _Running; } + + void run() + { + _Running = true; + + uint i = 0; + while(_Running && SoundMngr && i < _Files.size()) + { + // get copy incase _Files changes + std::string filename(_Files[i]); + + std::string title; + float length; + + if (SoundMngr->getMixer()->getSongTitle(filename, title, length)) + { + MusicPlayer.updateSong(filename, title, length); + } + + ++i; + } + + _Running = false; + _Files.clear(); + } + + // called from GUI + void getSongsInfo(const std::vector &filenames) + { + if (_Thread) + { + stopThread(); + } + + _Files = filenames; + + _Thread = IThread::create(this); + nlassert(_Thread != NULL); + _Thread->start(); + } + + void stopThread() + { + _Running = false; + if (_Thread) + { + _Thread->wait(); + delete _Thread; + _Thread = NULL; + } + } +}; + +static CMusicPlayerWorker MusicPlayerWorker; // *************************************************************************** @@ -69,11 +154,14 @@ bool CMusicPlayer::isShuffleEnabled() const return (NLGUI::CDBManager::getInstance()->getDbProp(MP3_SAVE_SHUFFLE)->getValue32() == 1); } - // *************************************************************************** -void CMusicPlayer::playSongs (const std::vector &songs) +void CMusicPlayer::playSongs (const std::vector &filenames) { - _Songs = songs; + _Songs.clear(); + for (uint i=0; i _Songs.size()) @@ -87,6 +175,9 @@ void CMusicPlayer::playSongs (const std::vector &songs) // If pause, stop, else play will resume if (_State == Paused || _Songs.empty()) _State = Stopped; + + // get song title/duration using worker thread + MusicPlayerWorker.getSongsInfo(filenames); } // *************************************************************************** @@ -107,6 +198,88 @@ void CMusicPlayer::updatePlaylist(sint prevIndex) if (pIE) pIE->setActive(true); } +// *************************************************************************** +// called from worker thread +void CMusicPlayer::updateSong(const std::string filename, const std::string title, float length) +{ + CAutoMutex mutex(MusicPlayerMutex); + + _SongUpdateQueue.push_back(CSongs(filename, title, length)); +} + +// *************************************************************************** +// called from GUI +void CMusicPlayer::updateSongs() +{ + CAutoMutex mutex(MusicPlayerMutex); + if (!_SongUpdateQueue.empty()) + { + for(uint i = 0; i < _SongUpdateQueue.size(); ++i) + { + updateSong(_SongUpdateQueue[i]); + } + _SongUpdateQueue.clear(); + } +} + +// *************************************************************************** +void CMusicPlayer::updateSong(const CSongs &song) +{ + uint index = 0; + while(index < _Songs.size()) + { + if (_Songs[index].Filename == song.Filename) + { + _Songs[index].Title = song.Title; + _Songs[index].Length = song.Length; + break; + } + + ++index; + } + if (index == _Songs.size()) + { + nlwarning("Unknown song file '%s'", song.Filename.c_str()); + return; + } + + std::string rowId(toString("%s:s%d", MP3_PLAYER_PLAYLIST_LIST, index)); + CInterfaceGroup *pIG = dynamic_cast(CWidgetManager::getInstance()->getElementFromId(rowId)); + if (!pIG) + { + nlwarning("Playlist row '%s' not found", rowId.c_str()); + return; + } + + CViewText *pVT; + pVT = dynamic_cast(pIG->getView(TEMPLATE_PLAYLIST_SONG_TITLE)); + if (pVT) + { + pVT->setHardText(song.Title); + } + else + { + nlwarning("title element '%s' not found", TEMPLATE_PLAYLIST_SONG_TITLE); + } + + pVT = dynamic_cast(pIG->getView(TEMPLATE_PLAYLIST_SONG_DURATION)); + if (pVT) + { + uint min = (sint32)(song.Length / 60) % 60; + uint sec = (sint32)(song.Length) % 60; + uint hour = song.Length / 3600; + std::string duration(toString("%02d:%02d", min, sec)); + if (hour > 0) + duration = toString("%02d:", hour) + duration; + + pVT->setHardText(duration); + } + else + { + nlwarning("duration element '%s' not found", TEMPLATE_PLAYLIST_SONG_DURATION); + } +} + // *************************************************************************** void CMusicPlayer::shuffleAndRebuildPlaylist() { @@ -131,12 +304,16 @@ void CMusicPlayer::rebuildPlaylist() _CurrentSongIndex = i; } - uint min = (sint32)(_Songs[i].Length / 60) % 60; - uint sec = (sint32)(_Songs[i].Length) % 60; - uint hour = _Songs[i].Length / 3600; - std::string duration(toString("%02d:%02d", min, sec)); - if (hour > 0) - duration = toString("%02d:", hour) + duration; + std::string duration("--:--"); + if (_Songs[i].Length > 0) + { + uint min = (sint32)(_Songs[i].Length / 60) % 60; + uint sec = (sint32)(_Songs[i].Length) % 60; + uint hour = _Songs[i].Length / 3600; + duration = toString("%02d:%02d", min, sec); + if (hour > 0) + duration = toString("%02d:", hour) + duration; + } vector< pair > vParams; vParams.push_back(pair("id", "s" + toString(i))); @@ -289,6 +466,12 @@ void CMusicPlayer::update () { if(!SoundMngr) return; + + if (MusicPlayerWorker.isRunning() || !_SongUpdateQueue.empty()) + { + updateSongs(); + } + if (_State == Playing) { CViewText *pVT = dynamic_cast(CWidgetManager::getInstance()->getElementFromId("ui:interface:mp3_player:screen:text")); @@ -331,7 +514,7 @@ void CMusicPlayer::update () } // *************************************************************************** -static void addFromPlaylist(const std::string &playlist, std::vector &filenames) +static void addFromPlaylist(const std::string &playlist, const std::vector &extensions, std::vector &filenames) { static uint8 utf8Header[] = { 0xefu, 0xbbu, 0xbfu }; @@ -362,9 +545,19 @@ static void addFromPlaylist(const std::string &playlist, std::vector songs; - for (i=0; igetMixer()->getSongTitle(filenames[i], song.Title, song.Length); - if (song.Length > 0) - songs.push_back (song); - } + // Sort songs by filename + sort(filenames.begin(), filenames.end()); - MusicPlayer.playSongs(songs); + MusicPlayer.playSongs(filenames); } else if (Params == "update_playlist") { diff --git a/ryzom/client/src/interface_v3/music_player.h b/ryzom/client/src/interface_v3/music_player.h index 549fa169d..3569d6439 100644 --- a/ryzom/client/src/interface_v3/music_player.h +++ b/ryzom/client/src/interface_v3/music_player.h @@ -42,12 +42,16 @@ public: class CSongs { public: + CSongs(std::string file = std::string(), std::string title = std::string(), float length = 0.f) + : Filename(file), Title(title), Length(length) + { } + std::string Filename; std::string Title; float Length; }; - void playSongs (const std::vector &songs); + void playSongs (const std::vector &filenames); void play (sint index = -1); // Play the song at current position, if playing, restart. If paused, resume. void pause (); void stop (); @@ -68,12 +72,23 @@ public: // Update playlist active row void updatePlaylist(sint prevIndex = -1); + // Update single song title/duration on _Songs and on playlist + void updateSong(const CSongs &song); + + // Update _Songs and playlist from _SongUpdateQueue + void updateSongs(); + + // update song from worker thread + void updateSong(const std::string filename, const std::string title, float length); + private: // The playlist CSongs _CurrentSong; uint _CurrentSongIndex; // If (!_Songs.empty()) must always be <_Songs.size() std::vector _Songs; + // updated info from worker thread + std::vector _SongUpdateQueue; // State enum TState { Stopped, Playing, Paused } _State; From 2a2431b1d319ca2ea1ea195d2d7847109ba41318 Mon Sep 17 00:00:00 2001 From: Nimetu Date: Thu, 2 Apr 2020 11:10:51 +0300 Subject: [PATCH 6/7] Changed: Automatically build playlist on play if needed --- .../client/src/interface_v3/music_player.cpp | 125 ++++++++++-------- ryzom/client/src/interface_v3/music_player.h | 3 + 2 files changed, 75 insertions(+), 53 deletions(-) diff --git a/ryzom/client/src/interface_v3/music_player.cpp b/ryzom/client/src/interface_v3/music_player.cpp index 10b4cd288..2f02352fb 100644 --- a/ryzom/client/src/interface_v3/music_player.cpp +++ b/ryzom/client/src/interface_v3/music_player.cpp @@ -353,6 +353,17 @@ void CMusicPlayer::play (sint index) if(!SoundMngr) return; + if (_Songs.empty()) + { + index = 0; + createPlaylistFromMusic(); + } + + if (_Songs.empty()) + { + _State = Stopped; + return; + } sint prevSongIndex = _CurrentSongIndex; @@ -465,7 +476,10 @@ void CMusicPlayer::next () void CMusicPlayer::update () { if(!SoundMngr) + { + _State = Stopped; return; + } if (MusicPlayerWorker.isRunning() || !_SongUpdateQueue.empty()) { @@ -564,6 +578,63 @@ static void addFromPlaylist(const std::string &playlist, const std::vector extensions; + SoundMngr->getMixer()->getMusicExtensions(extensions); + + // no format supported + if (extensions.empty()) + { + // in the very unlikely scenario + const ucstring message("Sound driver has no support for music."); + CInterfaceManager::getInstance()->displaySystemInfo(message, "SYS"); + nlinfo("%s", message.toUtf8().c_str()); + return; + } + std::string newPath = CPath::makePathAbsolute(CPath::standardizePath(ClientCfg.MediaPlayerDirectory), CPath::getCurrentPath(), true); + std::string extlist; + join(extensions, ", ", extlist); + extlist += ", m3u, m3u8"; + + std::string msg(CI18N::get("uiMk_system6").toUtf8()); + msg += ": " + newPath + " (" + extlist + ")"; + CInterfaceManager::getInstance()->displaySystemInfo(ucstring::makeFromUtf8(msg), "SYS"); + nlinfo("%s", msg.c_str()); + + // Recursive scan for files from media directory + vector filesToProcess; + CPath::getPathContent (newPath, true, false, true, filesToProcess); + + uint i; + std::vector filenames; + std::vector playlists; + + for (i = 0; i < filesToProcess.size(); ++i) + { + std::string ext = toLower(CFile::getExtension(filesToProcess[i])); + if (std::find(extensions.begin(), extensions.end(), ext) != extensions.end()) + { + filenames.push_back(filesToProcess[i]); + } + else if (ext == "m3u" || ext == "m3u8") + { + playlists.push_back(filesToProcess[i]); + } + } + + // Add songs from playlists + for (i = 0; i < playlists.size(); ++i) + { + addFromPlaylist(playlists[i], extensions, filenames); + } + + // Sort songs by filename + sort(filenames.begin(), filenames.end()); + + playSongs(filenames); +} + // *************************************************************************** class CMusicPlayerPlaySongs: public IActionHandler { @@ -581,59 +652,7 @@ public: if (Params == "play_songs") { - std::vector extensions; - SoundMngr->getMixer()->getMusicExtensions(extensions); - - // no format supported - if (extensions.empty()) - { - // in the very unlikely scenario - const ucstring message("Sound driver has no support for music."); - CInterfaceManager::getInstance()->displaySystemInfo(message, "SYS"); - nlinfo("%s", message.toUtf8().c_str()); - return; - } - std::string newPath = CPath::makePathAbsolute(CPath::standardizePath(ClientCfg.MediaPlayerDirectory), CPath::getCurrentPath(), true); - std::string extlist; - join(extensions, ", ", extlist); - extlist += ", m3u, m3u8"; - - std::string msg(CI18N::get("uiMk_system6").toUtf8()); - msg += ": " + newPath + " (" + extlist + ")"; - CInterfaceManager::getInstance()->displaySystemInfo(ucstring::makeFromUtf8(msg), "SYS"); - nlinfo("%s", msg.c_str()); - - // Recursive scan for files from media directory - vector filesToProcess; - CPath::getPathContent (newPath, true, false, true, filesToProcess); - - uint i; - std::vector filenames; - std::vector playlists; - - for (i = 0; i < filesToProcess.size(); ++i) - { - std::string ext = toLower(CFile::getExtension(filesToProcess[i])); - if (std::find(extensions.begin(), extensions.end(), ext) != extensions.end()) - { - filenames.push_back(filesToProcess[i]); - } - else if (ext == "m3u" || ext == "m3u8") - { - playlists.push_back(filesToProcess[i]); - } - } - - // Add songs from playlists - for (i = 0; i < playlists.size(); ++i) - { - addFromPlaylist(playlists[i], extensions, filenames); - } - - // Sort songs by filename - sort(filenames.begin(), filenames.end()); - - MusicPlayer.playSongs(filenames); + MusicPlayer.createPlaylistFromMusic(); } else if (Params == "update_playlist") { diff --git a/ryzom/client/src/interface_v3/music_player.h b/ryzom/client/src/interface_v3/music_player.h index 3569d6439..1932591e3 100644 --- a/ryzom/client/src/interface_v3/music_player.h +++ b/ryzom/client/src/interface_v3/music_player.h @@ -81,6 +81,9 @@ public: // update song from worker thread void updateSong(const std::string filename, const std::string title, float length); + // scan music folder and rebuild playlist + void createPlaylistFromMusic(); + private: // The playlist From 0f5dbc318c4bff55c9d25cd4f6c411bb43ac292e Mon Sep 17 00:00:00 2001 From: Nimetu Date: Fri, 3 Apr 2020 12:12:40 +0300 Subject: [PATCH 7/7] Fixed: Clear scene background if sky filter is used --- ryzom/client/src/main_loop.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ryzom/client/src/main_loop.cpp b/ryzom/client/src/main_loop.cpp index a3f6f3059..760a551c2 100644 --- a/ryzom/client/src/main_loop.cpp +++ b/ryzom/client/src/main_loop.cpp @@ -556,11 +556,11 @@ void clearBuffers() } // Sky is used to clear the frame buffer now, but if in line or point polygon mode, we should draw it - if (Driver->getPolygonMode() != UDriver::Filled) + if (Driver->getPolygonMode() != UDriver::Filled || !Filter3D[FilterSky]) { if (!Driver->isLost()) { - Driver->clearBuffers (CRGBA(127, 127, 127)); + Driver->clearBuffers (ClientCfg.BGColor); } } }