Replace frame layout detection with dedicated detector.

src/emucore/Console.cxx

@@ -58,6 +58,7 @@
 #include "TIAConstants.hxx"
 #include "FrameLayout.hxx"
 #include "frame-manager/FrameManager.hxx"
+#include "frame-manager/FrameLayoutDetector.hxx"
 
 #ifdef DEBUGGER_SUPPORT
   #include "Debugger.hxx"
@@ -125,20 +126,29 @@ Console::Console(OSystem& osystem, unique_ptr<Cartridge>& cart,
     bool fastscbios = myOSystem.settings().getBool("fastscbios");
     myOSystem.settings().setValue("fastscbios", true);
 
-    uInt8 initialGarbageFrames = TIAConstants::initialGarbageFrames;
+    FrameLayoutDetector frameLayoutDetector;
-    uInt8 linesPAL = 0;
+    myTIA->setFrameManager(&frameLayoutDetector);
-    uInt8 linesNTSC = 0;
+    mySystem->reset(true);
 
-    mySystem->reset(true);  // autodetect in reset enabled
+    for(int i = 0; i < 60; ++i) myTIA->update();
-    myTIA->autodetectLayout(true);
-    for(int i = 0; i < 60; ++i) {
-      if (i > initialGarbageFrames)
-        myTIA->frameLayout() == FrameLayout::pal ? linesPAL++ : linesNTSC++;
 
-      myTIA->update();
+    myTIA->setFrameManager(myFrameManager.get());
+
+    (cout << int(frameLayoutDetector.detectedLayout()) << std::endl).flush();
+
+    switch (frameLayoutDetector.detectedLayout()) {
+      case FrameLayout::ntsc:
+        myDisplayFormat = "NTSC";
+        break;
+
+      case FrameLayout::pal:
+        myDisplayFormat = "PAL";
+        break;
+
+      default:
+        throw runtime_error("cannot happen");
     }
 
-    myDisplayFormat = linesPAL > linesNTSC  ? "PAL" : "NTSC";
     if(myProperties.get(Display_Format) == "AUTO")
     {
       autodetected = "*";
@@ -657,8 +667,6 @@ void Console::setTIAProperties()
   if(height != 0)
     height = BSPF::clamp(height, TIAConstants::minViewableHeight, TIAConstants::maxViewableHeight);
 
-  myTIA->autodetectLayout(false);
-
   if(myDisplayFormat == "NTSC" || myDisplayFormat == "PAL60" ||
      myDisplayFormat == "SECAM60")
   {

src/emucore/tia/TIA.hxx

@@ -216,7 +216,6 @@ class TIA : public Device
     void setHeight(uInt32 height) { myFrameManager->setFixedHeight(height); }
     void setYStart(uInt32 ystart) { myFrameManager->setYstart(ystart); }
 
-    void autodetectLayout(bool toggle) { myFrameManager->autodetectLayout(toggle); }
     void setLayout(FrameLayout layout) { myFrameManager->setLayout(layout); }
     FrameLayout frameLayout() const { return myFrameManager->layout(); }
 

src/emucore/tia/frame-manager/AbstractFrameManager.hxx

@@ -190,11 +190,6 @@ class AbstractFrameManager : public Serializable
      */
     virtual bool ystartIsAuto(uInt32 line) const { return false; }
 
-    /**
-     * TODO: this has to go
-     */
-    virtual void autodetectLayout(bool toggle) {}
-
     /**
      * Set the frame layout. This may be a noop (on the autodetection manager).
      */

src/emucore/tia/frame-manager/FrameLayoutDetector.cxx

@@ -0,0 +1,133 @@
+//============================================================================
+//
+//   SSSS    tt          lll  lll
+//  SS  SS   tt           ll   ll
+//  SS     tttttt  eeee   ll   ll   aaaa
+//   SSSS    tt   ee  ee  ll   ll      aa
+//      SS   tt   eeeeee  ll   ll   aaaaa  --  "An Atari 2600 VCS Emulator"
+//  SS  SS   tt   ee      ll   ll  aa  aa
+//   SSSS     ttt  eeeee llll llll  aaaaa
+//
+// Copyright (c) 1995-2017 by Bradford W. Mott, Stephen Anthony
+// and the Stella Team
+//
+// See the file "License.txt" for information on usage and redistribution of
+// this file, and for a DISCLAIMER OF ALL WARRANTIES.
+//============================================================================
+
+#include "FrameLayoutDetector.hxx"
+#include "TIAConstants.hxx"
+
+enum Metrics: uInt32 {
+  frameLinesNTSC            = 262,
+  frameLinesPAL             = 312,
+  waitForVsync              = 100,
+  tvModeDetectionTolerance  = 20,
+  initialGarbageFrames      = TIAConstants::initialGarbageFrames
+};
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+FrameLayout FrameLayoutDetector::detectedLayout() const{
+  return myPalFrames > myNtscFrames ? FrameLayout::pal : FrameLayout::ntsc;
+}
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+void FrameLayoutDetector::onReset()
+{
+  myState = State::waitForVsyncStart;
+  myNtscFrames = myPalFrames = 0;
+  myLinesWaitingForVsync = 0;
+}
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+void FrameLayoutDetector::onSetVsync()
+{
+  if (myVsync)
+    setState(State::waitForVsyncEnd);
+  else
+    setState(State::waitForVsyncStart);
+}
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+void FrameLayoutDetector::onNextLine()
+{
+  const uInt32 frameLines = layout() == FrameLayout::ntsc ? Metrics::frameLinesNTSC : Metrics::frameLinesPAL;
+
+  switch (myState) {
+    case State::waitForVsyncStart:
+      if (myCurrentFrameTotalLines > frameLines - 3 || myTotalFrames == 0)
+        myLinesWaitingForVsync++;
+
+      if (myLinesWaitingForVsync > Metrics::waitForVsync) setState(State::waitForVsyncEnd);
+
+      break;
+
+    case State::waitForVsyncEnd:
+      if (++myLinesWaitingForVsync > Metrics::waitForVsync) setState(State::waitForVsyncStart);
+
+      break;
+
+    default:
+      throw runtime_error("cannot happen");
+  }
+}
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+void FrameLayoutDetector::setState(State state)
+{
+  if (state == myState) return;
+
+  myState = state;
+
+  switch (myState) {
+    case State::waitForVsyncEnd:
+      myLinesWaitingForVsync = 0;
+      break;
+
+    case State::waitForVsyncStart:
+      myLinesWaitingForVsync = 0;
+
+      finalizeFrame();
+      notifyFrameStart();
+      break;
+
+    default:
+      throw new runtime_error("cannot happen");
+  }
+}
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+void FrameLayoutDetector::finalizeFrame()
+{
+  notifyFrameComplete();
+
+  if (myTotalFrames <= Metrics::initialGarbageFrames) return;
+
+  const uInt32
+    deltaNTSC = abs(Int32(myCurrentFrameFinalLines) - Int32(frameLinesNTSC)),
+    deltaPAL =  abs(Int32(myCurrentFrameFinalLines) - Int32(frameLinesPAL));
+
+  if (std::min(deltaNTSC, deltaPAL) <= Metrics::tvModeDetectionTolerance)
+    layout(deltaNTSC <= deltaPAL ? FrameLayout::ntsc : FrameLayout::pal);
+  else if (
+    (myCurrentFrameFinalLines < frameLinesPAL) &&
+    (myCurrentFrameFinalLines > frameLinesNTSC) &&
+    (myCurrentFrameFinalLines % 2)
+  )
+    layout(FrameLayout::ntsc);
+  else
+    layout(deltaNTSC <= deltaPAL ? FrameLayout::ntsc : FrameLayout::pal);
+
+  switch (layout()) {
+    case FrameLayout::ntsc:
+      myNtscFrames++;
+      break;
+
+    case FrameLayout::pal:
+      myPalFrames++;
+      break;
+
+    default:
+      throw runtime_error("cannot happen");
+  }
+}

src/emucore/tia/frame-manager/FrameLayoutDetector.hxx

@@ -0,0 +1,65 @@
+//============================================================================
+//
+//   SSSS    tt          lll  lll
+//  SS  SS   tt           ll   ll
+//  SS     tttttt  eeee   ll   ll   aaaa
+//   SSSS    tt   ee  ee  ll   ll      aa
+//      SS   tt   eeeeee  ll   ll   aaaaa  --  "An Atari 2600 VCS Emulator"
+//  SS  SS   tt   ee      ll   ll  aa  aa
+//   SSSS     ttt  eeeee llll llll  aaaaa
+//
+// Copyright (c) 1995-2017 by Bradford W. Mott, Stephen Anthony
+// and the Stella Team
+//
+// See the file "License.txt" for information on usage and redistribution of
+// this file, and for a DISCLAIMER OF ALL WARRANTIES.
+//============================================================================
+
+#ifndef TIA_FRAME_LAYOUT_DETECTOR
+#define TIA_FRAME_LAYOUT_DETECTOR
+
+#include "AbstractFrameManager.hxx"
+#include "FrameLayout.hxx"
+
+class FrameLayoutDetector: public AbstractFrameManager {
+  public:
+
+    FrameLayoutDetector() = default;
+
+  public:
+
+    FrameLayout detectedLayout() const;
+
+  protected:
+
+    void onSetVsync() override;
+
+    void onReset() override;
+
+    void onNextLine() override;
+
+  private:
+
+    enum State {
+      waitForVsyncStart,
+      waitForVsyncEnd
+    };
+
+
+  private:
+
+    void setState(State state);
+
+    void finalizeFrame();
+
+  private:
+
+    State myState;
+
+    uInt32 myNtscFrames, myPalFrames;
+
+    uInt32 myLinesWaitingForVsync;
+
+};
+
+#endif // TIA_FRAME_LAYOUT_DETECTOR

src/emucore/tia/frame-manager/FrameManager.cxx

@@ -30,25 +30,15 @@ enum Metrics: uInt32 {
   overscanPAL                   = 36,
   vsync                         = 3,
   maxLinesVsync                 = 32,
-  maxLinesVsyncDuringAutodetect = 100,
   visibleOverscan               = 20,
   tvModeDetectionTolerance      = 20,
   initialGarbageFrames          = TIAConstants::initialGarbageFrames,
-  framesForModeConfirmation     = 5,
   minStableFrames               = 10,
   maxStabilizationFrames        = 20,
   minDeltaForJitter             = 3,
   framesForStableHeight         = 2
 };
 
-static constexpr uInt32
-  frameLinesNTSC = Metrics::vsync + Metrics::vblankNTSC + Metrics::kernelNTSC + Metrics::overscanNTSC,
-  frameLinesPAL = Metrics::vsync + Metrics::vblankPAL + Metrics::kernelPAL + Metrics::overscanPAL;
-
-inline static uInt32 vsyncLimit(bool autodetect) {
-  return autodetect ? maxLinesVsyncDuringAutodetect : maxLinesVsync;
-}
-
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 FrameManager::FrameManager() :
   myHeight(0)
@@ -64,8 +54,6 @@ void FrameManager::onReset()
   myState = State::waitForVsyncStart;
   myLineInState = 0;
   myTotalFrames = 0;
-  myFramesInMode = 0;
-  myModeConfirmed = false;
   myVsyncLines = 0;
   myY = 0;
   myFramePending = false;
@@ -89,12 +77,12 @@ void FrameManager::onNextLine()
       if ((myCurrentFrameTotalLines > myFrameLines - 3) || myTotalFrames == 0)
         myVsyncLines++;
 
-      if (myVsyncLines > vsyncLimit(myAutodetectLayout)) setState(State::waitForFrameStart);
+      if (myVsyncLines > Metrics::maxLinesVsync) setState(State::waitForFrameStart);
 
       break;
 
     case State::waitForVsyncEnd:
-      if (++myVsyncLines > vsyncLimit(myAutodetectLayout))
+      if (++myVsyncLines > Metrics::maxLinesVsync)
         setState(State::waitForFrameStart);
 
       break;
@@ -230,8 +218,6 @@ void FrameManager::finalizeFrame()
 #ifdef TIA_FRAMEMANAGER_DEBUG_LOG
   (cout << "frame complete @ " << myLineInState << " (" << myCurrentFrameFinalLines << " total)" << "\n").flush();
 #endif // TIA_FRAMEMANAGER_DEBUG_LOG
-
-  if (myAutodetectLayout) updateAutodetectedLayout();
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
@@ -247,41 +233,7 @@ void FrameManager::handleJitter(Int32 scanlineDifference)
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-void FrameManager::updateAutodetectedLayout()
+// TODO: kill this with fire once frame manager refactoring is complete
-{
-  if (myTotalFrames <= Metrics::initialGarbageFrames) {
-    return;
-  }
-
-  const FrameLayout oldLayout = layout();
-
-  const uInt32
-    deltaNTSC = abs(Int32(myCurrentFrameFinalLines) - Int32(frameLinesNTSC)),
-    deltaPAL =  abs(Int32(myCurrentFrameFinalLines) - Int32(frameLinesPAL));
-
-  if (std::min(deltaNTSC, deltaPAL) <= Metrics::tvModeDetectionTolerance)
-    layout(deltaNTSC <= deltaPAL ? FrameLayout::ntsc : FrameLayout::pal);
-  else if (!myModeConfirmed) {
-    if (
-      (myCurrentFrameFinalLines < frameLinesPAL) &&
-      (myCurrentFrameFinalLines > frameLinesNTSC) &&
-      (myCurrentFrameFinalLines % 2)
-    )
-      layout(FrameLayout::ntsc);
-    else
-      layout(deltaNTSC <= deltaPAL ? FrameLayout::ntsc : FrameLayout::pal);
-  }
-
-  if (oldLayout == layout())
-    myFramesInMode++;
-  else
-    myFramesInMode = 0;
-
-  if (myFramesInMode > Metrics::framesForModeConfirmation)
-    myModeConfirmed = true;
-}
-
-// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 void FrameManager::onLayoutChange()
 {
 #ifdef TIA_FRAMEMANAGER_DEBUG_LOG
@@ -353,7 +305,6 @@ bool FrameManager::onSave(Serializer& out) const
 {
   if (!myVblankManager.save(out)) return false;
 
-  out.putBool(myAutodetectLayout);
   out.putInt(uInt32(myState));
   out.putInt(myLineInState);
   out.putInt(myVsyncLines);
@@ -361,9 +312,6 @@ bool FrameManager::onSave(Serializer& out) const
   out.putInt(myLastY);
   out.putBool(myFramePending);
 
-  out.putInt(myFramesInMode);
-  out.putBool(myModeConfirmed);
-
   out.putInt(myStableFrames);
   out.putInt(myStabilizationFrames);
   out.putBool(myHasStabilized);
@@ -388,7 +336,6 @@ bool FrameManager::onLoad(Serializer& in)
 {
   if (!myVblankManager.load(in)) return false;
 
-  myAutodetectLayout = in.getBool();
   myState = State(in.getInt());
   myLineInState = in.getInt();
   myVsyncLines = in.getInt();
@@ -396,9 +343,6 @@ bool FrameManager::onLoad(Serializer& in)
   myLastY = in.getInt();
   myFramePending = in.getBool();
 
-  myFramesInMode = in.getInt();
-  myModeConfirmed = in.getBool();
-
   myStableFrames = in.getInt();
   myStabilizationFrames = in.getInt();
   myHasStabilized = in.getBool();

src/emucore/tia/frame-manager/FrameManager.hxx

@@ -52,9 +52,7 @@ class FrameManager: public AbstractFrameManager {
 
     bool ystartIsAuto(uInt32 line) const override { return myVblankManager.ystartIsAuto(line); };
 
-    void autodetectLayout(bool toggle) override { myAutodetectLayout = toggle; }
+    void setLayout(FrameLayout mode) override { layout(mode); }
-
-    void setLayout(FrameLayout mode) override { if (!myAutodetectLayout) layout(mode); }
 
     void onSetVblank() override;
 
@@ -99,16 +97,12 @@ class FrameManager: public AbstractFrameManager {
 
     VblankManager myVblankManager;
 
-    bool myAutodetectLayout;
     State myState;
     uInt32 myLineInState;
     uInt32 myVsyncLines;
     uInt32 myY, myLastY;
     bool myFramePending;
 
-    uInt32 myFramesInMode;
-    bool myModeConfirmed;
-
     uInt32 myStableFrames;
     uInt32 myStabilizationFrames;
     bool myHasStabilized;

src/emucore/tia/frame-manager/module.mk

@@ -3,7 +3,8 @@ MODULE := src/emucore/tia/frame-manager
 MODULE_OBJS := \
 	src/emucore/tia/frame-manager/FrameManager.o \
 	src/emucore/tia/frame-manager/VblankManager.o \
-	src/emucore/tia/frame-manager/AbstractFrameManager.o
+	src/emucore/tia/frame-manager/AbstractFrameManager.o \
+	src/emucore/tia/frame-manager/FrameLayoutDetector.o
 
 MODULE_DIRS += \
 	src/emucore/tia/frame-manager