From 04ca54623cb896c06776668f2cb6abb5bd25d7f8 Mon Sep 17 00:00:00 2001 From: Rachel Bryk Date: Sun, 8 Mar 2015 06:50:47 -0400 Subject: [PATCH 1/3] Compare timebase of netplay users to detect desyncs. --- Source/Core/Core/ConfigManager.h | 2 ++ Source/Core/Core/Movie.cpp | 4 +++ Source/Core/Core/NetPlayClient.cpp | 33 +++++++++++++++++++ Source/Core/Core/NetPlayClient.h | 2 ++ Source/Core/Core/NetPlayProto.h | 5 ++- Source/Core/Core/NetPlayServer.cpp | 51 ++++++++++++++++++++++++++++-- 6 files changed, 94 insertions(+), 3 deletions(-) diff --git a/Source/Core/Core/ConfigManager.h b/Source/Core/Core/ConfigManager.h index 42cd8d5d2b..ee60a059ae 100644 --- a/Source/Core/Core/ConfigManager.h +++ b/Source/Core/Core/ConfigManager.h @@ -118,6 +118,8 @@ struct SConfig : NonCopyable bool m_GameCubeAdapter; bool m_AdapterRumble; + bool m_NetplayDesyncCheck; + SysConf* m_SYSCONF; // Save settings diff --git a/Source/Core/Core/Movie.cpp b/Source/Core/Core/Movie.cpp index b59ff1d78a..2df26d93e8 100644 --- a/Source/Core/Core/Movie.cpp +++ b/Source/Core/Core/Movie.cpp @@ -17,6 +17,7 @@ #include "Core/Core.h" #include "Core/CoreTiming.h" #include "Core/Movie.h" +#include "Core/NetPlayClient.h" #include "Core/NetPlayProto.h" #include "Core/State.h" #include "Core/DSP/DSPCore.h" @@ -163,6 +164,9 @@ void FrameUpdate() FrameSkipping(); s_bPolled = false; + + if (NetPlay::IsNetPlayRunning() && SConfig::GetInstance().m_NetplayDesyncCheck) + NetPlayClient::SendTimeBase(); } // called when game is booting up, even if no movie is active, diff --git a/Source/Core/Core/NetPlayClient.cpp b/Source/Core/Core/NetPlayClient.cpp index 3f53d58a32..6670acb202 100644 --- a/Source/Core/Core/NetPlayClient.cpp +++ b/Source/Core/Core/NetPlayClient.cpp @@ -374,6 +374,7 @@ unsigned int NetPlayClient::OnData(sf::Packet& packet) case NP_MSG_START_GAME: { { + SConfig::GetInstance().m_NetplayDesyncCheck = true; std::lock_guard lkg(m_crit.game); packet >> m_current_game; packet >> g_NetPlaySettings.m_CPUthread; @@ -442,6 +443,24 @@ unsigned int NetPlayClient::OnData(sf::Packet& packet) } break; + case NP_MSG_DESYNC_DETECTED: + { + if (!SConfig::GetInstance().m_NetplayDesyncCheck) + break; + + int id; + u32 frame; + packet >> id; + packet >> frame; + std::string sID = ""; + if (id != -1) + sID = StringFromFormat(" from player ID %d", id); + + m_dialog->AppendChat("Possible desync detected" + sID + StringFromFormat(" on frame: %u", frame)); + SConfig::GetInstance().m_NetplayDesyncCheck = false; + } + break; + default: PanicAlertT("Unknown message received with id : %d", mid); break; @@ -1044,6 +1063,20 @@ u8 NetPlayClient::LocalWiimoteToInGameWiimote(u8 local_pad) return ingame_pad; } +void NetPlayClient::SendTimeBase() +{ + std::lock_guard lk(crit_netplay_client); + + u64 timebase = SystemTimers::GetFakeTimeBase(); + + sf::Packet* spac = new sf::Packet; + *spac << (MessageId)NP_MSG_TIMEBASE; + *spac << (u32)timebase; + *spac << (u32)(timebase << 32); + *spac << (u32)Movie::g_currentFrame; + netplay_client->SendAsync(spac); +} + // stuff hacked into dolphin // called from ---CPU--- thread diff --git a/Source/Core/Core/NetPlayClient.h b/Source/Core/Core/NetPlayClient.h index ce5d0ee63b..f12b489b7d 100644 --- a/Source/Core/Core/NetPlayClient.h +++ b/Source/Core/Core/NetPlayClient.h @@ -77,6 +77,8 @@ public: u8 LocalWiimoteToInGameWiimote(u8 local_pad); + static void SendTimeBase(); + enum State { WaitingForTraversalClientConnection, diff --git a/Source/Core/Core/NetPlayProto.h b/Source/Core/Core/NetPlayProto.h index 7f17e0aa60..e473c660fa 100644 --- a/Source/Core/Core/NetPlayProto.h +++ b/Source/Core/Core/NetPlayProto.h @@ -29,7 +29,7 @@ struct Rpt : public std::vector typedef std::vector NetWiimote; -#define NETPLAY_VERSION "Dolphin NetPlay 2014-01-08" +#define NETPLAY_VERSION "Dolphin NetPlay 2015-03-10" extern u64 g_netplay_initial_gctime; @@ -53,6 +53,9 @@ enum NP_MSG_STOP_GAME = 0xA2, NP_MSG_DISABLE_GAME = 0xA3, + NP_MSG_TIMEBASE = 0xB0, + NP_MSG_DESYNC_DETECTED = 0xB1, + NP_MSG_READY = 0xD0, NP_MSG_NOT_READY = 0xD1, diff --git a/Source/Core/Core/NetPlayServer.cpp b/Source/Core/Core/NetPlayServer.cpp index f53793f3be..d47857e723 100644 --- a/Source/Core/Core/NetPlayServer.cpp +++ b/Source/Core/Core/NetPlayServer.cpp @@ -23,6 +23,8 @@ u64 g_netplay_initial_gctime = 1272737767; +static std::map>> s_timebase; + NetPlayServer::~NetPlayServer() { if (is_connected) @@ -110,8 +112,7 @@ void NetPlayServer::ThreadFunc() while (m_do_loop) { // update pings every so many seconds - - if ((m_ping_timer.GetTimeElapsed() > 1000) || m_update_pings) + if (m_update_pings || (m_ping_timer.GetTimeElapsed() > 1000)) { m_ping_key = Common::Timer::GetTimeMs(); @@ -208,6 +209,7 @@ void NetPlayServer::ThreadFunc() break; } } + Common::SleepCurrentThread(1); } // close listening socket and client sockets @@ -581,6 +583,50 @@ unsigned int NetPlayServer::OnData(sf::Packet& packet, Client& player) } break; + case NP_MSG_TIMEBASE: + { + u32 x, y, frame; + packet >> x; + packet >> y; + packet >> frame; + s_timebase[frame].emplace_back(x | ((u64)y >> 32), player.pid); + + if (!std::all_of(s_timebase[frame].begin(), s_timebase[frame].end(), [&frame](std::pair i){ return i.first == s_timebase[frame][0].first; })) + { + sf::Packet spac; + spac << (MessageId) NP_MSG_DESYNC_DETECTED; + + int pid = -1; + if (s_timebase[frame].size() > 2) + { + for (auto time : s_timebase[frame]) + { + int count = 0; + for (auto _time : s_timebase[frame]) + { + if (_time.first == time.first) + count++; + } + if ((size_t)count != s_timebase[frame].size() - 1) + { + if (pid == -1) + { + pid = time.second; + } + else + { + pid = -1; + break; + } + } + } + } + spac << pid; + spac << frame; + SendToClients(spac); + } + } + break; default: PanicAlertT("Unknown message with id:%d received from player:%d Kicking player!", mid, player.pid); // unknown message, kick the client @@ -635,6 +681,7 @@ void NetPlayServer::SetNetSettings(const NetSettings &settings) // called from ---GUI--- thread bool NetPlayServer::StartGame() { + s_timebase.clear(); std::lock_guard lkg(m_crit.game); m_current_game = Common::Timer::GetTimeMs(); From f2631a835e0b2838326e91ef5f544470ae9762a5 Mon Sep 17 00:00:00 2001 From: comex Date: Fri, 5 Jun 2015 19:00:26 -0400 Subject: [PATCH 2/3] Simplify and improve. Note - I removed a SleepCurrentThread(1) the patch added which seemed to be unrelated to the actual job at hand. If there was a real need for it (which sounds like it would be an enet-related bug - enet_host_service is supposed to *sleep*), that needs to be dealt with... --- Source/Core/Core/ConfigManager.h | 2 -- Source/Core/Core/Movie.cpp | 2 +- Source/Core/Core/NetPlayClient.cpp | 23 ++++++------ Source/Core/Core/NetPlayServer.cpp | 57 +++++++++++++++--------------- Source/Core/Core/NetPlayServer.h | 4 +++ 5 files changed, 46 insertions(+), 42 deletions(-) diff --git a/Source/Core/Core/ConfigManager.h b/Source/Core/Core/ConfigManager.h index ee60a059ae..42cd8d5d2b 100644 --- a/Source/Core/Core/ConfigManager.h +++ b/Source/Core/Core/ConfigManager.h @@ -118,8 +118,6 @@ struct SConfig : NonCopyable bool m_GameCubeAdapter; bool m_AdapterRumble; - bool m_NetplayDesyncCheck; - SysConf* m_SYSCONF; // Save settings diff --git a/Source/Core/Core/Movie.cpp b/Source/Core/Core/Movie.cpp index 2df26d93e8..facef9ce6a 100644 --- a/Source/Core/Core/Movie.cpp +++ b/Source/Core/Core/Movie.cpp @@ -165,7 +165,7 @@ void FrameUpdate() s_bPolled = false; - if (NetPlay::IsNetPlayRunning() && SConfig::GetInstance().m_NetplayDesyncCheck) + if (NetPlay::IsNetPlayRunning()) NetPlayClient::SendTimeBase(); } diff --git a/Source/Core/Core/NetPlayClient.cpp b/Source/Core/Core/NetPlayClient.cpp index 6670acb202..d456ae0da2 100644 --- a/Source/Core/Core/NetPlayClient.cpp +++ b/Source/Core/Core/NetPlayClient.cpp @@ -374,7 +374,6 @@ unsigned int NetPlayClient::OnData(sf::Packet& packet) case NP_MSG_START_GAME: { { - SConfig::GetInstance().m_NetplayDesyncCheck = true; std::lock_guard lkg(m_crit.game); packet >> m_current_game; packet >> g_NetPlaySettings.m_CPUthread; @@ -445,19 +444,21 @@ unsigned int NetPlayClient::OnData(sf::Packet& packet) case NP_MSG_DESYNC_DETECTED: { - if (!SConfig::GetInstance().m_NetplayDesyncCheck) - break; - - int id; + int pid_to_blame; u32 frame; - packet >> id; + packet >> pid_to_blame; packet >> frame; - std::string sID = ""; - if (id != -1) - sID = StringFromFormat(" from player ID %d", id); + const char* blame_str = ""; + const char* blame_name = ""; + std::lock_guard lkp(m_crit.players); + if (pid_to_blame != -1) + { + auto it = m_players.find(pid_to_blame); + blame_str = " from player "; + blame_name = it != m_players.end() ? it->second.name.c_str() : "??"; + } - m_dialog->AppendChat("Possible desync detected" + sID + StringFromFormat(" on frame: %u", frame)); - SConfig::GetInstance().m_NetplayDesyncCheck = false; + m_dialog->AppendChat(StringFromFormat("/!\\ Possible desync detected%s%s on frame %u", blame_str, blame_name, frame)); } break; diff --git a/Source/Core/Core/NetPlayServer.cpp b/Source/Core/Core/NetPlayServer.cpp index d47857e723..aa10e56afb 100644 --- a/Source/Core/Core/NetPlayServer.cpp +++ b/Source/Core/Core/NetPlayServer.cpp @@ -23,8 +23,6 @@ u64 g_netplay_initial_gctime = 1272737767; -static std::map>> s_timebase; - NetPlayServer::~NetPlayServer() { if (is_connected) @@ -112,7 +110,7 @@ void NetPlayServer::ThreadFunc() while (m_do_loop) { // update pings every so many seconds - if (m_update_pings || (m_ping_timer.GetTimeElapsed() > 1000)) + if ((m_ping_timer.GetTimeElapsed() > 1000) || m_update_pings) { m_ping_key = Common::Timer::GetTimeMs(); @@ -209,7 +207,6 @@ void NetPlayServer::ThreadFunc() break; } } - Common::SleepCurrentThread(1); } // close listening socket and client sockets @@ -589,41 +586,44 @@ unsigned int NetPlayServer::OnData(sf::Packet& packet, Client& player) packet >> x; packet >> y; packet >> frame; - s_timebase[frame].emplace_back(x | ((u64)y >> 32), player.pid); - if (!std::all_of(s_timebase[frame].begin(), s_timebase[frame].end(), [&frame](std::pair i){ return i.first == s_timebase[frame][0].first; })) + if (m_desync_detected) + break; + + u64 timebase = x | ((u64)y << 32); + std::vector>& timebases = m_timebase_by_frame[frame]; + timebases.emplace_back(player.pid, timebase); + if (timebases.size() >= m_players.size()) { - sf::Packet spac; - spac << (MessageId) NP_MSG_DESYNC_DETECTED; + // we have all records for this frame - int pid = -1; - if (s_timebase[frame].size() > 2) + if (!std::all_of(timebases.begin(), timebases.end(), [&](std::pair pair){ return pair.second == timebases[0].second; })) { - for (auto time : s_timebase[frame]) + int pid_to_blame = -1; + if (timebases.size() > 2) { - int count = 0; - for (auto _time : s_timebase[frame]) + for (auto pair : timebases) { - if (_time.first == time.first) - count++; - } - if ((size_t)count != s_timebase[frame].size() - 1) - { - if (pid == -1) + if (std::all_of(timebases.begin(), timebases.end(), [&](std::pair other) { + return other.first == pair.first || other.second != pair.second; + })) { - pid = time.second; - } - else - { - pid = -1; + // we are the only outlier + pid_to_blame = pair.first; break; } } } + + sf::Packet spac; + spac << (MessageId) NP_MSG_DESYNC_DETECTED; + spac << pid_to_blame; + spac << frame; + SendToClients(spac); + + m_desync_detected = true; } - spac << pid; - spac << frame; - SendToClients(spac); + m_timebase_by_frame.erase(frame); } } break; @@ -681,7 +681,8 @@ void NetPlayServer::SetNetSettings(const NetSettings &settings) // called from ---GUI--- thread bool NetPlayServer::StartGame() { - s_timebase.clear(); + m_timebase_by_frame.clear(); + m_desync_detected = false; std::lock_guard lkg(m_crit.game); m_current_game = Common::Timer::GetTimeMs(); diff --git a/Source/Core/Core/NetPlayServer.h b/Source/Core/Core/NetPlayServer.h index 73f8d6d85e..842c795c38 100644 --- a/Source/Core/Core/NetPlayServer.h +++ b/Source/Core/Core/NetPlayServer.h @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include "Common/Timer.h" @@ -99,6 +100,9 @@ private: std::map m_players; + std::unordered_map>> m_timebase_by_frame; + bool m_desync_detected; + struct { std::recursive_mutex game; From 9c63b78397f2707e3df37f1f632f1705916d5075 Mon Sep 17 00:00:00 2001 From: comex Date: Sat, 6 Jun 2015 01:20:51 -0400 Subject: [PATCH 3/3] Fix indeterminism in GPU thread mode. --- Source/Core/Core/Core.cpp | 7 +++++++ Source/Core/Core/Core.h | 2 ++ Source/Core/Core/Movie.cpp | 5 ++--- Source/Core/Core/NetPlayClient.cpp | 4 +++- Source/Core/Core/NetPlayClient.h | 2 ++ Source/Core/VideoCommon/PixelEngine.cpp | 3 +++ 6 files changed, 19 insertions(+), 4 deletions(-) diff --git a/Source/Core/Core/Core.cpp b/Source/Core/Core/Core.cpp index e08c3f58d2..604de6f170 100644 --- a/Source/Core/Core/Core.cpp +++ b/Source/Core/Core/Core.cpp @@ -29,6 +29,7 @@ #include "Core/Host.h" #include "Core/MemTools.h" #include "Core/Movie.h" +#include "Core/NetPlayClient.h" #include "Core/NetPlayProto.h" #include "Core/PatchEngine.h" #include "Core/State.h" @@ -129,6 +130,12 @@ void SetIsFramelimiterTempDisabled(bool disable) std::string GetStateFileName() { return s_state_filename; } void SetStateFileName(const std::string& val) { s_state_filename = val; } +void FrameUpdateOnCPUThread() +{ + if (NetPlay::IsNetPlayRunning()) + NetPlayClient::SendTimeBase(); +} + // Display messages and return values // Formatted stop message diff --git a/Source/Core/Core/Core.h b/Source/Core/Core/Core.h index b86168f3f6..1eea2be226 100644 --- a/Source/Core/Core/Core.h +++ b/Source/Core/Core/Core.h @@ -63,6 +63,8 @@ void SetStateFileName(const std::string& val); void SetBlockStart(u32 addr); +void FrameUpdateOnCPUThread(); + bool ShouldSkipFrame(int skipped); void VideoThrottle(); void RequestRefreshInfo(); diff --git a/Source/Core/Core/Movie.cpp b/Source/Core/Core/Movie.cpp index facef9ce6a..de457e7b16 100644 --- a/Source/Core/Core/Movie.cpp +++ b/Source/Core/Core/Movie.cpp @@ -140,6 +140,8 @@ std::string GetInputDisplay() void FrameUpdate() { + // TODO[comex]: This runs on the GPU thread, yet it messes with the CPU + // state directly. That's super sketchy. g_currentFrame++; if (!s_bPolled) g_currentLagCount++; @@ -164,9 +166,6 @@ void FrameUpdate() FrameSkipping(); s_bPolled = false; - - if (NetPlay::IsNetPlayRunning()) - NetPlayClient::SendTimeBase(); } // called when game is booting up, even if no movie is active, diff --git a/Source/Core/Core/NetPlayClient.cpp b/Source/Core/Core/NetPlayClient.cpp index d456ae0da2..eeae193110 100644 --- a/Source/Core/Core/NetPlayClient.cpp +++ b/Source/Core/Core/NetPlayClient.cpp @@ -663,6 +663,8 @@ bool NetPlayClient::StartGame(const std::string &path) m_dialog->AppendChat(" -- STARTING GAME -- "); + m_timebase_frame = 0; + m_is_running.store(true); NetPlay_Enable(this); @@ -1074,7 +1076,7 @@ void NetPlayClient::SendTimeBase() *spac << (MessageId)NP_MSG_TIMEBASE; *spac << (u32)timebase; *spac << (u32)(timebase << 32); - *spac << (u32)Movie::g_currentFrame; + *spac << netplay_client->m_timebase_frame++; netplay_client->SendAsync(spac); } diff --git a/Source/Core/Core/NetPlayClient.h b/Source/Core/Core/NetPlayClient.h index f12b489b7d..c80afda9f7 100644 --- a/Source/Core/Core/NetPlayClient.h +++ b/Source/Core/Core/NetPlayClient.h @@ -143,6 +143,8 @@ private: std::string m_player_name; bool m_connecting; TraversalClient* m_traversal_client; + + u32 m_timebase_frame; }; void NetPlay_Enable(NetPlayClient* const np); diff --git a/Source/Core/VideoCommon/PixelEngine.cpp b/Source/Core/VideoCommon/PixelEngine.cpp index 45db963d23..91662e9931 100644 --- a/Source/Core/VideoCommon/PixelEngine.cpp +++ b/Source/Core/VideoCommon/PixelEngine.cpp @@ -11,6 +11,7 @@ #include "Common/ChunkFile.h" #include "Common/CommonTypes.h" #include "Core/ConfigManager.h" +#include "Core/Core.h" #include "Core/CoreTiming.h" #include "Core/State.h" #include "Core/HW/MMIO.h" @@ -282,6 +283,8 @@ void SetFinish_OnMainThread(u64 userdata, int cyclesLate) s_signal_finish_interrupt.store(1); UpdateInterrupts(); CommandProcessor::SetInterruptFinishWaiting(false); + + Core::FrameUpdateOnCPUThread(); } // SetToken