[Ultrastar-ng-cvs] SF.net SVN: ultrastar-ng:[1554] trunk/game

[<tr...@us...>]

Revision: 1554
          http://ultrastar-ng.svn.sourceforge.net/ultrastar-ng/?rev=1554&view=rev
Author:   tronic
Date:     2009-06-06 23:13:29 +0000 (Sat, 06 Jun 2009)

Log Message:
-----------
Fixed race condition in song reload while singing. Three Engine/Player optimizations, one of which is rather evil and is likely to cause concurrency issues and thus YOU need to test it heavily.

Modified Paths:
--------------
    trunk/game/engine.cc
    trunk/game/engine.hh
    trunk/game/screen_sing.cc

Modified: trunk/game/engine.cc
===================================================================
--- trunk/game/engine.cc	2009-06-06 22:48:23 UTC (rev 1553)
+++ trunk/game/engine.cc	2009-06-06 23:13:29 UTC (rev 1554)
@@ -3,13 +3,13 @@
 const double Engine::TIMESTEP = 0.01;
 
 void Player::update() {
+	if (m_pos == m_pitch.size()) return; // End of song already
 	Tone const* t = m_analyzer.findTone();
 	if (t) {
 		m_activitytimer = 1000;
-		m_scoreIt = m_song.notes.begin(); // TODO: optimize
-		m_pitch.push_back(std::make_pair(t->freq, t->stabledb));
-		double beginTime = Engine::TIMESTEP * (m_pitch.size() - 1);
-		double endTime = beginTime + 0.01;
+		double beginTime = Engine::TIMESTEP * m_pos;
+		m_pitch[m_pos++] = std::make_pair(t->freq, t->stabledb);
+		double endTime = Engine::TIMESTEP * m_pos;
 		while (m_scoreIt != m_song.notes.end()) {
 			m_score += m_song.m_scoreFactor * m_scoreIt->score(m_song.scale.getNote(t->freq), beginTime, endTime);
 			if (endTime < m_scoreIt->end) break;
@@ -18,7 +18,7 @@
 		m_score = clamp(m_score, 0.0, 1.0);
 	} else {
 		if (m_activitytimer > 0) --m_activitytimer;
-		m_pitch.push_back(std::make_pair(getNaN(), -getInf()));
+		m_pitch[m_pos++] = std::make_pair(getNaN(), -getInf());
 	}
 }
 

Modified: trunk/game/engine.hh
===================================================================
--- trunk/game/engine.hh	2009-06-06 22:48:23 UTC (rev 1553)
+++ trunk/game/engine.hh	2009-06-06 23:13:29 UTC (rev 1554)
@@ -29,6 +29,8 @@
 	typedef std::vector<std::pair<double, double> > pitch_t;
 	/// player's pitch
 	pitch_t m_pitch;
+	/// current position in pitch vector (first unused spot)
+	size_t m_pos;
 	/// score for current song
 	double m_score;
 	/// activity timer
@@ -36,7 +38,7 @@
 	/// score iterator
 	Notes::const_iterator m_scoreIt;
 	/// constructor
-	Player(Song& song, Analyzer& analyzer): m_song(song), m_analyzer(analyzer), m_score(), m_activitytimer() {}
+	Player(Song& song, Analyzer& analyzer, size_t frames): m_song(song), m_analyzer(analyzer), m_pitch(frames), m_pos(), m_score(), m_activitytimer(), m_scoreIt(m_song.notes.begin()) {}
 	/// prepares analyzer
 	void prepare() { m_analyzer.process(); }
 	/// updates player stats
@@ -68,7 +70,6 @@
 	volatile double m_latencyAR;  // Audio roundtrip latency (don't confuse with latencyAV)
 	size_t m_time;
 	volatile bool m_quit;
-	mutable boost::mutex m_mutex;
 	boost::scoped_ptr<boost::thread> m_thread;
 
   public:
@@ -84,7 +85,8 @@
 	template <typename FwdIt> Engine(Audio& audio, Song& song, FwdIt anBegin, FwdIt anEnd):
 	  m_audio(audio), m_song(song), m_latencyAR(config["audio/round-trip"].get_f()), m_time(), m_quit()
 	{
-		while (anBegin != anEnd) m_players.push_back(Player(song, *anBegin++));
+		size_t frames = m_audio.getLength() / Engine::TIMESTEP;
+		while (anBegin != anEnd) m_players.push_back(Player(song, *anBegin++, frames));
 		size_t player = 0;
 		for (std::list<Player>::iterator it = m_players.begin(); it != m_players.end(); ++it, ++player) it->m_color = playerColors[player % playerColorsSize];
 		m_thread.reset(new boost::thread(boost::ref(*this)));
@@ -103,16 +105,14 @@
 			double t = m_audio.getPosition() - m_latencyAR;
 			double timeLeft = m_time * TIMESTEP - t;
 			if (timeLeft > 0.0) { boost::thread::sleep(now() + std::min(TIMESTEP, timeLeft)); continue; }
-			boost::mutex::scoped_lock l(m_mutex);
 			for (Notes::const_iterator it = m_song.notes.begin(); it != m_song.notes.end(); ++it) it->power = 0.0f;
 			std::for_each(m_players.begin(), m_players.end(), boost::bind(&Player::update, _1));
 			++m_time;
 		}
 	}
 	/// gets list of players currently plugged in
-	std::list<Player> getPlayers() const {
-		boost::thread::yield(); // Try to let engine perform its run right before getting the data
-		boost::mutex::scoped_lock l(m_mutex);
+	std::list<Player> const& getPlayers() const {
+		// XXX: Technically this code is incorrect because it returns a reference to a structure that is being at the same modified by another thread (and nothing's even marked volatile). This is done in order to improve performance.
 		return m_players;
 	}
 };

Modified: trunk/game/screen_sing.cc
===================================================================
--- trunk/game/screen_sing.cc	2009-06-06 22:48:23 UTC (rev 1553)
+++ trunk/game/screen_sing.cc	2009-06-06 23:13:29 UTC (rev 1554)
@@ -85,8 +85,7 @@
 		else if (key == SDLK_UP) m_audio.seek(30.0);
 		else if (key == SDLK_DOWN) { m_audio.seek(-30.0); seekback = true; }
 		else if (key == SDLK_r && event.key.keysym.mod & KMOD_CTRL) {
-			m_songs.current().reload();
-			exit(); enter();
+			exit(); m_songs.current().reload(); enter();
 			m_audio.seek(time);
 		}
 		config["audio/video_delay"].f() = clamp(round(config["audio/video_delay"].get_f() * 1000.0) / 1000.0, -0.5, 0.5);
@@ -142,7 +141,7 @@
 		theme->bg_top->draw();
 	}
 
-	std::list<Player> players = m_engine->getPlayers();
+	std::list<Player> const& players = m_engine->getPlayers();
 	m_noteGraph->draw(time, players); // Draw notes and pitch waves
 
 	// Score display


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.