Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

speed up init delay #176

Closed
enthusi opened this issue Jul 17, 2017 · 19 comments
Closed

speed up init delay #176

enthusi opened this issue Jul 17, 2017 · 19 comments
Assignees
Milestone

Comments

@enthusi
Copy link

enthusi commented Jul 17, 2017

I read about Stella's initial couple of frames (~20) to determine the proper framerate etc. to make sure it settles down after reset.
Since this takes a notable amount of time now, maybe it could be carried out without any vsync or other sync-code that slows down this procedure which could be carried out at full CPU speed in principle.

@DirtyHairy
Copy link
Member

That's an interesting idea and certainly possible. However, it is not entirely trivial as we have to suppress audio during this time.

@sa666666
Copy link
Member

sa666666 commented Jul 19, 2017

So much for a little vacation 😈

I was wondering about this too. There's already a block of code in Console that runs for 60 frames to detect the console format. Why can't the ystart autodetection be done there too?

// Run the TIA, looking for PAL scanline patterns
// We turn off the SuperCharger progress bars, otherwise the SC BIOS
// will take over 250 frames!
// The 'fastscbios' option must be changed before the system is reset
bool fastscbios = myOSystem.settings().getBool("fastscbios");
myOSystem.settings().setValue("fastscbios", true);

uInt8 initialGarbageFrames = FrameManager::initialGarbageFrames();
uInt8 linesPAL = 0;
uInt8 linesNTSC = 0;

mySystem->reset(true);  // autodetect in reset enabled
myTIA->autodetectLayout(true);
for(int i = 0; i < 60; ++i) {
  if (i > initialGarbageFrames)
    myTIA->frameLayout() == FrameLayout::pal ? linesPAL++ : linesNTSC++;

  myTIA->update();
}

myDisplayFormat = linesPAL > linesNTSC  ? "PAL" : "NTSC";
if(myProperties.get(Display_Format) == "AUTO")
{
  autodetected = "*";
  myCurrentFormat = 0;
}

// Don't forget to reset the SC progress bars again
myOSystem.settings().setValue("fastscbios", fastscbios);

We could easily increase this to 120 frames or more, and you wouldn't even notice the difference, since the TIA is running without outputting anything to the screen and without v'sync enabled. In such a case, it is possible to run the emulation at over 1000 fps.

I had actually meant to ask you why the autodetection must be done in 'user' time (ie, when the user can see what's happening), and not in auto-detect time (ie, before a window has even been opened). But with everything else going on, I forgot to get back to you on this.

@sa666666
Copy link
Member

@DirtyHairy, I wonder if after the TIA runs for 60 frames (or more, depending on what we use), then the ystart found at that point could be the one that's used? Then we can call VblankManager::setYstart() on that value, which will change the state to 'fixed', and disable detecting the ystart again. This call could be in VblankManager::reset(), and if it finds that 'myLastVblankLines' (aka, ystart) is non-zero, it calls setYStart() with the value.

I've tried this, and there still seems to be an issue. Actually, it may be a bug. If I specify '-ystart' on the commandline, it should override the auto-detection and always use that value. When I try it in, for example, River Raid, the screen still sometimes jumps. I think that is a bug; a specified value wouldn't cause the display to jump at all. I think once this is fixed, the above approach will work too.

Note that I don't yet understand all the code in VblankManager, so I'm only guessing and experimenting. I think the transitioning and garbage frame logic should be bypassed when we give a specific ystart value??

@sa666666
Copy link
Member

Sorry for the flurry of posts, but I'm having a difficult time figuring out what exactly is causing the image to 'jump' if we don't delay for 20 frames. In the old core, there was no concept of autodetecting ystart, so the start line was set, and the image was stable. In the current code, (a) I don't know why the image jumps, and (b) if we set ystart manually, then the behaviour should match Stella 4.x and have absolutely no jumps at all.

I may just be guessing, but it seems we may need to separate the logic of determining ystart from the actual drawing. Then the autodetection can happen in the first 60 generated frames in Console.cxx, thereafter we have a stable ystart, and then finally use that for drawing the image. So there would be no jumping in the image that you see; it would always start at a set location (aka, ystart).

@DirtyHairy
Copy link
Member

DirtyHairy commented Jul 22, 2017

@sa666666 I think this is very subtle, and we should not make it a prerequesite for 5.0.1 😏

First off, the idea of detecting ystart at the same time as TV mode is attractive, but I remember having trouble getting it to work reliably for all ROMs accross the board --- there always was some odd case that cropped up. I finally concluded that there are too many variables for this to work reliably, which is why TV mode detection uses a fixed ystart iirc. I don't see harm in trying it again, but I am not terribly optimistic.

The jump in River Raid is interesting, and I am not 100% sure where it comes from, either. I did a lot of debugging of the same case that you just looked at (before implementing skipping those garbage frames): fixed ystart and still jumping and I finally concluded that the jump was genuine and not a bug. However, I am curious about that myself and would like to do some more debugging to be 100% sure, but I am reasonably convinced that this may be to differences in the way frame boundary detection works between Stella 4 and 5.

In a nutshell:

  • I don't think we will be able to pull off TV mode autodetect and ystart autodetect at the same time
  • Doing those in subsequent runs should work fine, but would turn "ystart = 0 = autodetect" into ystart = fixed
  • My preference would be to remove frame limiting from the first 20 garbage frames without restarting the emulaton afterwards.
  • Before doing anything here, I'd also like to clarify the initial jump even if ystart is fixed --- it may be that there still might be an obscure bug after all.

@sa666666
Copy link
Member

I've just found that the initial jump in River Raid is from the jitter recovery option; if I turn that off, and make the changes described above, the ROM starts up right away and without jumping. So maybe the jitter effect should be disabled in the first 20 frames or so.

As to your last point, yes, the jump still happens even if ystart is fixed. I modify the code to use a specific value, and it still jumps. But as mentioned above, it's the jitter effect causing it, since River Raid starts out with an irregular scanline count.

So there are two approaches to this:

  • modify jitter effect to not happen in the first 10-20 frames or so
  • modify jitter effect to take the large jump into account, and not compensate for it

I think the first approach probably makes more sense. I'm attaching a patch of what I've done so far.
ystart.zip

@DirtyHairy
Copy link
Member

Interesting find. Jitter may be involved, but I can reproduce the jump for demon attack even if jitter is disabled, so there's more to it.

@sa666666
Copy link
Member

I will add that even with jitter enabled, this jump didn't happen in River Raid in Stella 4.x. So as you say above, there's something else going on.

@sa666666
Copy link
Member

But the jump in Demon Attack also happened in Stella 4.x 😄 My main concern is that we don't have a regression, but at the same time improve on the perceived 'slow' startup time of Stella 5. So Demon Attack isn't a regression, but River Raid is, IMHO.

@DirtyHairy
Copy link
Member

DirtyHairy commented Jul 22, 2017

I can also get the jump in Stella 4, but it looks differently 😄 Turning my debug output on suggests the the jump (in Stella 5) happens when the ROM spends a lot of frames without vsyncing properly before starting to generate a valid signal. This makes sense, because vsync is required in order to have a reference point as to when the frame starts. So, as you say, not a bug.

As for the other stuff:

  • I wouldn't waste time on finding out what specifically causes jitter to behave different from Stella 4 here. It is a completely different implementation, and it is bound to behave differently in edge cases.
  • I think deactivating it for the first 20 or so frames is the best course
  • I still think that trying to pull off TV mode AND ystart detection during the 60 frames dry run is a bad idea. I start remembering more about the issues I faced, and this is the reason that the FrameManager behaves differently during this run (check out FrameManager::vsyncLimit)
  • My suggestion would be to "just" remove frame limiting while the FrameManager is still ignoring garbage frames.
  • The quoted 20 frames until rendering starts are just an upper limit; the precise number is usually smaller and depends on when the signal has stabilized.

Of to bed now... 😏 💤

@sa666666
Copy link
Member

I know you're in bed now; this message will be waiting for you in the morning 😈

I'll address your points in order:

  • I'm not going to do any research on the implementation of jitter; both approaches are IMO an approximation of what's really happening anyway
  • OK, I agree - how do we do it?
  • OK, how about running for 60 frames to detect the TV, then when we know TV mode another 60 to detect ystart? 😄 My main concern is that it happens fast, and without the user seeing it.
  • Problem here is that (as you pointed out) the sound may have already started, and fast-forwarding at this point is in some sense already too late
  • As you say, the number itself is not really relevant; the delay the user sees is

Basically, I want to have a ROM start up as quickly as it did in Stella 4. Delaying showing the video image makes the emulation seem slow or clunky, somehow.

@sa666666
Copy link
Member

sa666666 commented Jul 23, 2017

Latest patch works for every ROM I've tried so far around 200 or so. As long as jitter effect is disabled, of course; we still need to add code to disable it for the first few frames.

@DirtyHairy, you mentioned that some ROMs caused you problems in point 3 above. I don't suppose you remember which ones they were??

ystart.zip

@thrust26
Copy link
Member

BTW: With TV effects enabled, IMO the screen SHOULD always roll initially. 😈

@DirtyHairy
Copy link
Member

DirtyHairy commented Jul 23, 2017

@sa666666 I think the prototypes linked in thread #32 are a good testcase.

I think I know now what I'd like to do, although this is propably too much for inclusion in 5.0.1. After thinking about possible options, I more and more feel drawn to the conclusion that the current FrameManager has long exceeded my personal bounds for acceptable complexity. Rathern than adding even more logic, I'd like to split and refactor it.

I think the best route to go forward is to factor out the interface into an abstract class and implement several versions that are used at different points in the emulation lifecycle:

  1. A TV mode detector that only counts the scanlines between vsync signals and uses this to identify the TV mode.
  2. A ystart detector that needs the TV mode to be configured and only detects ystart, but doesn't include any other tweaks or subleties (i.e. jitter).
  3. The actual runtime implementation which needs TV mode and ystart to be configured and is used during actual emulation.

Before emulation starts, there will be a dry run with the TV mode detector, and then a dry run with the ystart detector (if ystart is set to autodetect). After this, the parameters are fed to the runtime version which is then used for actual emulation. This will make the code considerably simpler and more robust. It also will get rid of all delays in "user time".

As a bonus, this will open the road to implement yet another version that tries to behave as much as possible as a real CRT at some point, including jitter and an initial roll (not simulated artificially, but as consequences of the algorithm).

@sa666666
Copy link
Member

Sounds good to me. I agree that the current implementation is starting to get complicated; that's part of the reason why I was having trouble visualizing what's going on. The old TIA class in Stella 4 started to get like that in the end; logic on top of new logic, and very difficult to modify.

Anyway, if we're pushing this to after 5.0.1, I can go ahead and do that release now. Is there anything else that needs to be included??

@DirtyHairy
Copy link
Member

DirtyHairy commented Jul 23, 2017

Nothing that I am aware of. Make it so 😉

@DirtyHairy
Copy link
Member

Just a quick update: I have started to work on this in a branch here. However, I guess it will still take some time until I finish as I am currently a bit short on spare time and will be off to Wacken for the better part of next week --- maybe the week after 🍺 🌞 🤘 😏

@thrust26 thrust26 added this to the Prio 1 milestone Aug 18, 2017
@thrust26
Copy link
Member

@DirtyHairy Can this be closed?

@DirtyHairy
Copy link
Member

Definitely, my bad.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants