Documentation, cleaup, fix race in frame stats.

src/common/AudioQueue.cxx

@@ -101,7 +101,7 @@ Int16* AudioQueue::enqueue(Int16* fragment)
   if (mySize < capacity) mySize++;
   else {
     myNextFragment = (myNextFragment + 1) % capacity;
-    //(cerr << "audio buffer overflow\n").flush();
+    (cerr << "audio buffer overflow\n").flush();
   }
 
   return newFragment;

src/emucore/Console.cxx

@@ -937,7 +937,7 @@ void Console::generateColorLossPalette()
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 float Console::getFramerate() const
 {
-  return myTIA->frameRate();
+  return myTIA->frameBufferFrameRate();
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

src/emucore/EmulationWorker.cxx

@@ -34,7 +34,7 @@ EmulationWorker::EmulationWorker() : myPendingSignal(Signal::none), myState(Stat
     &EmulationWorker::threadMain, this, &threadInitialized, &mutex
   );
 
-  // Wait until the thread has acquired myWakeupMutex and moved on
+  // Wait until the thread has acquired myThreadIsRunningMutex and moved on
   while (myState == State::initializing) threadInitialized.wait(lock);
 }
 
@@ -43,7 +43,7 @@ EmulationWorker::~EmulationWorker()
 {
   // This has to run in a block in order to release the mutex before joining
   {
-    std::unique_lock<std::mutex> lock(myWakeupMutex);
+    std::unique_lock<std::mutex> lock(myThreadIsRunningMutex);
 
     if (myState != State::exception) {
       signalQuit();
@@ -71,27 +71,36 @@ void EmulationWorker::handlePossibleException()
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 void EmulationWorker::start(uInt32 cyclesPerSecond, uInt32 maxCycles, uInt32 minCycles, DispatchResult* dispatchResult, TIA* tia)
 {
-  waitForSignalClear();
+  // Wait until any pending signal has been processed
+  waitUntilPendingSignalHasProcessed();
 
-  // Aquire the mutex -> wait until the thread is suspended
+  // Run in a block to release the mutex before notifying; this avoids an unecessary
-  std::unique_lock<std::mutex> lock(myWakeupMutex);
+  // block that will waste a timeslice
+  {
+    // Aquire the mutex -> wait until the thread is suspended
+    std::unique_lock<std::mutex> lock(myThreadIsRunningMutex);
 
-  // Pass on possible exceptions
+    // Pass on possible exceptions
-  handlePossibleException();
+    handlePossibleException();
 
-  // NB: The thread does not suspend execution in State::initialized
+    // Make sure that we don't overwrite the exit condition.
-  if (myState != State::waitingForResume)
+    // This case is hypothetical and cannot happen, but handling it does not hurt, either
-    fatal("start called on running or dead worker");
+    if (myPendingSignal == Signal::quit) return;
 
-  // Store the parameters for emulation
+    // NB: The thread does not suspend execution in State::initialized
-  myTia = tia;
+    if (myState != State::waitingForResume)
-  myCyclesPerSecond = cyclesPerSecond;
+      fatal("start called on running or dead worker");
-  myMaxCycles = maxCycles;
-  myMinCycles = minCycles;
-  myDispatchResult = dispatchResult;
 
-  // Set the signal...
+    // Store the parameters for emulation
-  myPendingSignal = Signal::resume;
+    myTia = tia;
+    myCyclesPerSecond = cyclesPerSecond;
+    myMaxCycles = maxCycles;
+    myMinCycles = minCycles;
+    myDispatchResult = dispatchResult;
+
+    // Raise the signal...
+    myPendingSignal = Signal::resume;
+  }
 
   // ... and wakeup the thread
   myWakeupCondition.notify_one();
@@ -100,29 +109,40 @@ void EmulationWorker::start(uInt32 cyclesPerSecond, uInt32 maxCycles, uInt32 min
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 uInt64 EmulationWorker::stop()
 {
-  waitForSignalClear();
+  // See EmulationWorker::start above for the gory details
+  waitUntilPendingSignalHasProcessed();
 
-  std::unique_lock<std::mutex> lock(myWakeupMutex);
+  uInt64 totalCycles;
-  handlePossibleException();
+  {
+    std::unique_lock<std::mutex> lock(myThreadIsRunningMutex);
 
-  // If the worker has stopped on its own, we return
+    // Paranoia: make sure that we don't doublecount an emulation timeslice
-  if (myState == State::waitingForResume) return 0;
+    totalCycles = myTotalCycles;
+    myTotalCycles = 0;
 
-  // NB: The thread does not suspend execution in State::initialized or State::running
+    handlePossibleException();
-  if (myState != State::waitingForStop)
-    fatal("stop called on a dead worker");
 
-  myPendingSignal = Signal::stop;
+    if (myPendingSignal == Signal::quit) return totalCycles;
+
+    // If the worker has stopped on its own, we return
+    if (myState == State::waitingForResume) return totalCycles;
+
+    // NB: The thread does not suspend execution in State::initialized or State::running
+    if (myState != State::waitingForStop)
+      fatal("stop called on a dead worker");
+
+    myPendingSignal = Signal::stop;
+  }
 
   myWakeupCondition.notify_one();
 
-  return myTotalCycles;
+  return totalCycles;
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 void EmulationWorker::threadMain(std::condition_variable* initializedCondition, std::mutex* initializationMutex)
 {
-  std::unique_lock<std::mutex> lock(myWakeupMutex);
+  std::unique_lock<std::mutex> lock(myThreadIsRunningMutex);
 
   try {
     {
@@ -137,11 +157,18 @@ void EmulationWorker::threadMain(std::condition_variable* initializedCondition,
       initializedCondition->notify_one();
     }
 
+    // Loop until we have an exit condition
     while (myPendingSignal != Signal::quit) handleWakeup(lock);
   }
   catch (...) {
+    // Store away the exception and the state accordingly
     myPendingException = std::current_exception();
     myState = State::exception;
+
+    // Raising the exit condition is consistent and makes shure that the main thread
+    // will not deadlock if an exception is raised while it is waiting for a signal
+    // to be processed.
+    signalQuit();
   }
 }
 
@@ -150,6 +177,7 @@ void EmulationWorker::handleWakeup(std::unique_lock<std::mutex>& lock)
 {
   switch (myState) {
     case State::initialized:
+      // Enter waitingForResume and sleep after initialization
       myState = State::waitingForResume;
       myWakeupCondition.wait(lock);
       break;
@@ -172,13 +200,19 @@ void EmulationWorker::handleWakeupFromWaitingForResume(std::unique_lock<std::mut
 {
   switch (myPendingSignal) {
     case Signal::resume:
+      // Clear the pending signal and notify the main thread
       clearSignal();
+
+      // Reset virtual clock and cycle counter
       myVirtualTime = high_resolution_clock::now();
       myTotalCycles = 0;
+
+      // Enter emulation. This will emulate a timeslice and set the state upon completion.
       dispatchEmulation(lock);
       break;
 
     case Signal::none:
+      // Reenter sleep on spurious wakeups
       myWakeupCondition.wait(lock);
       break;
 
@@ -195,16 +229,21 @@ void EmulationWorker::handleWakeupFromWaitingForStop(std::unique_lock<std::mutex
 {
   switch (myPendingSignal) {
     case Signal::stop:
-      myState = State::waitingForResume;
+      // Clear the pending signal and notify the main thread
       clearSignal();
 
+      // Enter waiting for resume and sleep
+      myState = State::waitingForResume;
       myWakeupCondition.wait(lock);
       break;
 
     case Signal::none:
       if (myVirtualTime <= high_resolution_clock::now())
+        // The time allotted to the emulation timeslice has passed and we haven't been stopped?
+        // -> go for another emulation timeslice
         dispatchEmulation(lock);
       else
+        // Wakeup was spurious, reenter sleep
         myWakeupCondition.wait_until(lock, myVirtualTime);
 
       break;
@@ -220,6 +259,7 @@ void EmulationWorker::handleWakeupFromWaitingForStop(std::unique_lock<std::mutex
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 void EmulationWorker::dispatchEmulation(std::unique_lock<std::mutex>& lock)
 {
+  // Technically, we could do without State::running, but it is cleaner and might be useful in the future
   myState = State::running;
 
   uInt64 totalCycles = 0;
@@ -234,18 +274,22 @@ void EmulationWorker::dispatchEmulation(std::unique_lock<std::mutex>& lock)
   bool continueEmulating = false;
 
   if (myDispatchResult->getStatus() == DispatchResult::Status::ok) {
-    // If emulation finished successfully, we can go for another round
+    // If emulation finished successfully, we are free to go for another round
     duration<double> timesliceSeconds(static_cast<double>(totalCycles) / static_cast<double>(myCyclesPerSecond));
     myVirtualTime += duration_cast<high_resolution_clock::duration>(timesliceSeconds);
 
-    myState = State::waitingForStop;
+    // If we aren't fast enough to keep up with the emulation, we stop immediatelly to avoid
+    // starving the system for processing time --- emulation will stutter anyway.
     continueEmulating = myVirtualTime > high_resolution_clock::now();
   }
 
   if (continueEmulating) {
+    // If we are free to continue emulating, we sleep until either the timeslice has passed or we
+    // have been signalled from the main thread
     myState = State::waitingForStop;
     myWakeupCondition.wait_until(lock, myVirtualTime);
   } else {
+    // If can't continue, we just stop and wait to be signalled
     myState = State::waitingForResume;
     myWakeupCondition.wait(lock);
   }
@@ -254,8 +298,10 @@ void EmulationWorker::dispatchEmulation(std::unique_lock<std::mutex>& lock)
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 void EmulationWorker::clearSignal()
 {
-  std::unique_lock<std::mutex> lock(mySignalChangeMutex);
+  {
-  myPendingSignal = Signal::none;
+    std::unique_lock<std::mutex> lock(mySignalChangeMutex);
+    myPendingSignal = Signal::none;
+  }
 
   mySignalChangeCondition.notify_one();
 }
@@ -263,17 +309,20 @@ void EmulationWorker::clearSignal()
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 void EmulationWorker::signalQuit()
 {
-  std::unique_lock<std::mutex> lock(mySignalChangeMutex);
+  {
-  myPendingSignal = Signal::quit;
+    std::unique_lock<std::mutex> lock(mySignalChangeMutex);
+    myPendingSignal = Signal::quit;
+  }
 
   mySignalChangeCondition.notify_one();
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-void EmulationWorker::waitForSignalClear()
+void EmulationWorker::waitUntilPendingSignalHasProcessed()
 {
   std::unique_lock<std::mutex> lock(mySignalChangeMutex);
 
+  // White until there is no pending signal (or the exit condition has been raised)
   while (myPendingSignal != Signal::none && myPendingSignal != Signal::quit)
     mySignalChangeCondition.wait(lock);
 }

src/emucore/EmulationWorker.hxx

@@ -15,6 +15,26 @@
 // this file, and for a DISCLAIMER OF ALL WARRANTIES.
 //============================================================================
 
+/*
+ * This class is the core of stella's real time scheduling. Scheduling is a two step
+ * process that is shared between the main loop in OSystem and this class.
+ *
+ * In emulation mode (as opposed to debugger, menu, etc.), each iteration of the main loop
+ * instructs the emulation worker to start emulation on a separate thread and then proceeds
+ * to render the last frame produced by the TIA (if any). After the frame has been rendered,
+ * the worker is stopped, and the main thread sleeps until the time allotted to the emulation
+ * timeslice (as calculated from the 6507 cycles that have passed) has been reached. After
+ * that, it iterates.
+ *
+ * The emulation worker contains its own microscheduling. After emulating a timeslice, it sleeps
+ * until either the allotted time is up or it has been signalled to stop. If the time is up
+ * without the signal, the worker will emulate another timeslice, etc.
+ *
+ * In combination, the scheduling in the main loop and the microscheduling in the worker
+ * ensure that the emulation continues to run even if rendering blocks, ensuring the real
+ * time scheduling required for cycle exact audio to work.
+ */
+
 #ifndef EMULATION_WORKER_HXX
 #define EMULATION_WORKER_HXX
 
@@ -47,8 +67,14 @@ class EmulationWorker
 
     ~EmulationWorker();
 
+    /**
+      Wake up the worker and start emulation with the specified parameters.
+     */
     void start(uInt32 cyclesPerSecond, uInt32 maxCycles, uInt32 minCycles, DispatchResult* dispatchResult, TIA* tia);
 
+    /**
+      Stop emulation and return the number of 6507 cycles emulated.
+     */
     uInt64 stop();
 
   private:
@@ -58,12 +84,6 @@ class EmulationWorker
     // Passing references into a thread is awkward and requires std::ref -> use pointers here
     void threadMain(std::condition_variable* initializedCondition, std::mutex* initializationMutex);
 
-    void clearSignal();
-
-    void signalQuit();
-
-    void waitForSignalClear();
-
     void handleWakeup(std::unique_lock<std::mutex>& lock);
 
     void handleWakeupFromWaitingForResume(std::unique_lock<std::mutex>& lock);
@@ -72,6 +92,12 @@ class EmulationWorker
 
     void dispatchEmulation(std::unique_lock<std::mutex>& lock);
 
+    void clearSignal();
+
+    void signalQuit();
+
+    void waitUntilPendingSignalHasProcessed();
+
     void fatal(string message);
 
   private:
@@ -80,8 +106,10 @@ class EmulationWorker
 
     std::thread myThread;
 
+    // Condition variable for waking up the thread
     std::condition_variable myWakeupCondition;
-    std::mutex myWakeupMutex;
+    // THe thread is running while this mutex is locked
+    std::mutex myThreadIsRunningMutex;
 
     std::condition_variable mySignalChangeCondition;
     std::mutex mySignalChangeMutex;

src/emucore/FrameBuffer.cxx

@@ -344,7 +344,7 @@ void FrameBuffer::updateInEmulationMode()
   if(myStatsMsg.enabled)
     drawFrameStats();
 
-  myLastScanlines = myOSystem.console().tia().scanlinesLastFrame();
+  myLastScanlines = myOSystem.console().tia().frameBufferScanlinesLastFrame();
   myPausedCount = 0;
 
   // Draw any pending messages
@@ -389,9 +389,9 @@ void FrameBuffer::drawFrameStats()
   myStatsMsg.surface->invalidate();
 
   // draw scanlines
-  color = myOSystem.console().tia().scanlinesLastFrame() != myLastScanlines ?
+  color = myOSystem.console().tia().frameBufferScanlinesLastFrame() != myLastScanlines ?
       uInt32(kDbgColorRed) : myStatsMsg.color;
-  std::snprintf(msg, 30, "%3u", myOSystem.console().tia().scanlinesLastFrame());
+  std::snprintf(msg, 30, "%3u", myOSystem.console().tia().frameBufferScanlinesLastFrame());
   myStatsMsg.surface->drawString(font(), msg, xPos, YPOS,
                                  myStatsMsg.w, color, TextAlign::Left, 0, true, kBGColor);
   xPos += font().getStringWidth(msg);

src/emucore/tia/TIA.cxx

@@ -181,6 +181,9 @@ void TIA::reset()
     frameReset();  // Recalculate the size of the display
   }
 
+  myFrontBufferFrameRate = myFrameBufferFrameRate = 0;
+  myFrontBufferScanlines = myFrameBufferScanlines = 0;
+
   myNewFramePending = false;
 
   // Must be done last, after all other items have reset
@@ -287,6 +290,11 @@ bool TIA::save(Serializer& out) const
     out.putByteArray(myShadowRegisters, 64);
 
     out.putLong(myCyclesAtFrameStart);
+
+    out.putInt(myFrameBufferScanlines);
+    out.putInt(myFrontBufferScanlines);
+    out.putDouble(myFrameBufferFrameRate);
+    out.putDouble(myFrontBufferFrameRate);
   }
   catch(...)
   {
@@ -355,6 +363,11 @@ bool TIA::load(Serializer& in)
     in.getByteArray(myShadowRegisters, 64);
 
     myCyclesAtFrameStart = in.getLong();
+
+    myFrameBufferScanlines = in.getInt();
+    myFrontBufferScanlines = in.getInt();
+    myFrameBufferFrameRate = in.getDouble();
+    myFrontBufferFrameRate = in.getDouble();
   }
   catch(...)
   {
@@ -831,6 +844,9 @@ void TIA::renderToFrameBuffer()
   if (!myNewFramePending) return;
 
   memcpy(myFramebuffer, myFrontBuffer, 160 * TIAConstants::frameBufferHeight);
+
+  myFrameBufferFrameRate = myFrontBufferFrameRate;
+  myFrameBufferScanlines = myFrontBufferScanlines;
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
@@ -1181,6 +1197,10 @@ void TIA::onFrameComplete()
     memset(myBackBuffer + 160 * myFrameManager->getY(), 0, missingScanlines * 160);
 
   memcpy(myFrontBuffer, myBackBuffer, 160 * TIAConstants::frameBufferHeight);
+
+  myFrontBufferFrameRate = frameRate();
+  myFrontBufferScanlines = scanlinesLastFrame();
+
   myNewFramePending = true;
 }
 

src/emucore/tia/TIA.hxx

@@ -250,6 +250,11 @@ class TIA : public Device
 
     float frameRate() const { return myFrameManager ? myFrameManager->frameRate() : 0; }
 
+    /**
+      The same, but for the frame in the frame buffer.
+     */
+    float frameBufferFrameRate() const { return myFrameBufferFrameRate; }
+
     /**
       Enables/disables color-loss for PAL modes only.
 
@@ -295,6 +300,11 @@ class TIA : public Device
     */
     uInt32 scanlinesLastFrame() const { return myFrameManager->scanlinesLastFrame(); }
 
+    /**
+      The same, but for the frame in the frame buffer.
+     */
+    uInt32 frameBufferScanlinesLastFrame() const { return myFrameBufferScanlines; }
+
     /**
       Answers the total system cycles from the start of the emulation.
     */
@@ -681,6 +691,11 @@ class TIA : public Device
     uInt8 myBackBuffer[160 * TIAConstants::frameBufferHeight];
     uInt8 myFrontBuffer[160 * TIAConstants::frameBufferHeight];
 
+    // We snapshot frame statistics when the back buffer is copied to the front buffer
+    // and when the front buffer is copied to the frame buffer
+    uInt32 myFrontBufferScanlines, myFrameBufferScanlines;
+    float myFrontBufferFrameRate, myFrameBufferFrameRate;
+
     // Did we emit a frame?
     bool myNewFramePending;