Autofire Overhaul

[tonytoon]

I've been working on the autofire system and wanted to get some feedback on my proposed changes as I work towards implementing them.

Done:

• Display which buttons currently have autofire enabled anytime autofire is toggled for any button. Helps identify accidental activations of autofire.
• Calculate and display autofire in hertz rather than milliseconds to better match how hardware devices and emulators present the information.
• Allow defining custom autofire rates in mister.ini, with sane defaults if nothing is entered.

TODO:

• Per-button autofire rates. This has been a frequent ask I've seen from users in the shooting game community and the way I've rewritten autofire timing it should not be very complicated.
• I propose that we have two autofire rates by default (low and high) and pressing menu/osd while holding a button steps through them. This would be the most discoverable and least disruptive for users since it would visually indicate what is happening when they press a button combo they already use for autofire.
• Advanced users could take advantage of the custom autofire rates in mister.ini I've already implemented if low/high or the specific rates are insufficient.
• Remove left/right + menu/osd as a way to change global autofire rates. Users (or at least I) tend to hit that on accident and simply stepping each button through off/low/high would replace that functionality.

Thoughts? On the backend I've implemented a single 60hz timer for autofire which would make handling per-button rates straightforward and still easy to read.

[Kitrinx]

You should probably just convince Zakk to use his. Ideally there would also be two modes, one that used simple hz based repetition like NES and one correlated with polls from the system like TG16

[tonytoon]

That would be ideal since I'm sure he'd do it right but I don't have his address to send any bribes

@zakk4223 are you sure you want the guy who wrote gems like https://github.com/MiSTer-devel/Main_MiSTer/blob/master/input.cpp#L5851 submitting another PR?

[zakk4223]

You can't use the existing mister timer mechanism to do a framerate-matched timer; it only has a resolution of 1ms so even at a bog standard 60hz rate it is wrong (the fractional milliseconds matter) and will just suffer the same problems the current one has; maybe if you're lucky the misses will be spaced farther apart but maybe not.

You can try to compensate for the loss/gain over time but that vastly complicates the code and probably doesn't fully solve the issue anyways.

Maybe a super high resolution timer would work, I'm not sure.

ideally the framework would just increment a counter (3 bits is more than enough) that is query-able via something like UIO_GET_FRAMECNT.

The implementation I had used the scaler buffer's headers as a frame timer; the scaler increments a counter in the header every time it writes an input frame.

That code will never see a PR because reading out of ddr3 every poll cycle is insane behavior. It worked pretty well for every arcade core I used it on, but adding additional ddr3 load to something like psx/n64/saturn is probably asking for problems.

Poll based autofire probably has to be implemented per core. The only way I can think to do it generically is if you turned the above proposed UIO_GET_FRAMECNT into a generic 'poll counter' that defaulted to a frame counter but the core could override and increment itself. You could most likely stuff all the per-controller poll counters into a single 32 bit value.

On a more opinion-based comment: I generally consider the use of ini parameters for stuff like this to be incredibly user-unfriendly.

[tonytoon]

I switched to using clock_gettime() instead of the existing GetTimer() and did some quick and dirty tests. This is 5 runs of 1024 samples of our offset between intended and actual time we decided it was a new frame.

variance: mean: 12291ns, median: 12550ns, stddev: 7309.640830, lowest: 20ns, highest: 32541ns
variance: mean: 12302ns, median: 12197ns, stddev: 6973.608030, lowest: 16ns, highest: 35289ns
variance: mean: 12085ns, median: 11730ns, stddev: 7225.286400, lowest: 16ns, highest: 36316ns
variance: mean: 12268ns, median: 12412ns, stddev: 7096.275647, lowest: 1ns, highest: 36982ns

Which comes out to being off by around .0125ms on average. That seems really good for a software timer. It's not 100% but I think the worst case is around .2% inaccuracy.

(I'm surprised the numbers were this good.)

For another test, I launched 1942 and switched to 30hz autofire. When testing before I found that very few shots came out at 30hz, which I assume means we're exceeding how fast 1942 actually reads inputs. With the new timer resolution, I held the fire button for as long as I could dodge and a single shot came out after 40 seconds.

Here's the snippet in question for how I gathered the data.

if (joy[i] & autofire_mask[i] && !new_frame) {
				clock_gettime(CLOCK_MONOTONIC, &ts);
				uint64_t now = (uint64_t)ts.tv_sec * 1000000000ull + ts.tv_nsec;

				if (now >= next_frame_time) {
					new_frame = true;
					times[time_idx++] = now - next_frame_time;
					clock_gettime(CLOCK_MONOTONIC, &ts);
				    next_frame_time = now + pcurrent_video_info->vtime * 10ull;
				}

				

				if (time_idx >= 1024) {
					uint64_t low = UINT64_MAX;
					uint64_t high = 0;
					uint64_t sum = 0;
					for (int i = 0; i < 1024; i++) {
						if (times[i] < low) low = times[i];
						if (times[i] > high) high = times[i];
						sum += times[i];
					}
					uint64_t mean = sum / 1024;
					uint64_t median = median_u64(times, 1024);
					double stdev = jitter_stddev(times, 1024);
					printf("variance: mean: %lluns, median: %lluns, stddev: %f, lowest: %lluns, highest: %lluns\n", mean, median, stdev, low, high);
					time_idx = 0;
				}
}

[tonytoon]

@zakk4223 agreed on hiding stuff in the .ini being ugly and user-hostile. That's why I think extremely sane defaults are important so that updating the .ini isn't necessary for the vast majority of users, but it's still a nice option for the fraction of a fraction of users playing some game that really really wants 7.5hz autofire or something.

I planned on defaulting to 10hz for low, 20hz for high. Alternately, could do 10/20/30 and have low/high/max but then it's three button presses to turn off an autofire you accidentally enabled. If a user needs options beyond that, they're already hopping on the search engine of their choice to see how to set it.

[sorgelig]

So, what is the issue with current autofire?

[tonytoon]

The current system isn't broken, but I feel it can be improved in ways that benefit users without complicating things.

• People typically use hertz rather than milliseconds for autofire rates. I propose we instead use rates that are divisors of 60hz. (7.5, 15, 20, etc.).
• If you enable autofire on accident, it can take some guesswork to figure out which button you need to disable it on. I've modified the info box that appears when you enable/disable autofire to also list all buttons that have autofire turned on.
• I've seen people request the ability to have different autofires rate per-button. We are currently limited to a single global autofire rate that applies to all buttons. I propose that we have two autofire rates; low and high. The same shortcut that currently toggles autofire for a button would instead cycle through off/low/high/off for that button.
• Since we could toggle autofire rates for each button, we'd remove the left/right+osd to change the global rate. I know I hit it on accident more often than I actually use it as intended.
• I've implemented custom autofire rates in mister.ini. This is strictly optional for users. If a user wanted more than low/high or were unhappy with the values, they could modify the .ini to add or change those rates as they wish. If the user never touches the .ini it will default to the proposed low/high system with rates of 10hz and 20hz, which should cover the vast majority of users.

I've already implemented about 3/4 of this, and can have a PR ready for review within a week.

[tonytoon]

Note: These improvements would overwhelmingly be of interest mainly to players of shooting games like R-Type, Gradius, DoDonPachi, etc. That's the population that cares about things like autofire rates and they love their MiSTers as alternatives to pricy arcade boards. The only impact regular users should see is hopefully saying "Oh hey, it lets me set my turbo buttons to high or low now. Neat."

[sorgelig]

I'm ok in general. just some doubts:

• If you enable autofire on accident, it can take some guesswork to figure out which button you need to disable it on. I've modified the info box that appears when you enable/disable autofire to also list all buttons that have autofire turned on.

Looks like redundant info which may distract in action games. If autofire is enabled for specific button then you will feel it. Isn't?

Having only 2 rates sounds like "640kb will be enough for everything". Original intention was to minimize usage of ini. From my opinion, changing the rate of autofire according to specific game by buttons is more convenient than changing it in ini several times till you find an acceptable rate for specific game.

[zakk4223]

The fundamental issue with mister autofire is due to the implementation it cannot produce consistent autofire patterns. You can't have an autofire that toggles every other frame (so "30Hz" but really framerate/2) consistently because the timer has no knowledge of the core frame pacing.

You can try to fake it with a high resolution timer based on the reported framerate but I think that's still inconsistent due to being unable to control the clock phase relative to the core and getting unlucky and 'wiggling' around the input read.

The frame counter based timer experiments I did produced a very solid and consistent autofire result, but they are too resource intensive to put into the real world due to excessive ddr3 reads.

I still have some ideas to investigate that might make it very low impact but I'm not quite there yet.

I think if an overhaul is under consideration, it would be nice to think about the problem a bit deeper and see if we can come up with something that might turn out to be close to the best implementation of autofire available for many of the systems on MiSTer.

I realize some of this is just arcade people being weird, sorry ;)

[sorgelig]

well, there is wait for vsync functionality but it will be time consuming as it will require a wait. HPS also may be busy time after time by other events and may miss a frame. I've played some shmup games in console cores and had no problem with autofire, btw. I've played RType in PCE and it had consistent rapid fire. Is it again some kind of OCD issue because sometimes fire miss the frame?
I understand that perfect autofire is something good to have but not by any price.

[sorgelig]

I think FPGA can provide a frame counter. HPS can check it periodically to see if frame is passed and then issue a press or release. This won't require too much time and will sync with frame rate.

[zakk4223]

I just looked through some of my notes/code, and forgot I changed things:

the MiSTer_fb kernel module writes a frame counter to /sys/module/MiSTer_fb/parameter/frame_count

It's derived from the HDMI_TX_VS based interrupt from here:

wire [63:0] f2h_irq = {video_sync,HDMI_TX_VS};
cyclonev_hps_interface_interrupts interrupts
(
	.irq(f2h_irq)
);

It works very well if vsync_adjust=1 or 2, obviously 0 is not a core matched framerate so not so good there.

video_sync is derived from the core video signal; the module currently ignores that interrupt but could be modified to also write a parameter like input_frame_count and that could be used as the clocking source.

reading those files is very fast since they are not real files, as long as you open it once and read from the fd when you need to they aren't very resource intensive.

Obviously something queryable directly from the core via spi_* is best but maybe a kernel update is easier than having to update a bunch of core frameworks again.

[tonytoon]

I think FPGA can provide a frame counter. HPS can check it periodically to see if frame is passed and then issue a press or release. This won't require too much time and will sync with frame rate.

That would be fantastic. I'm going to continue working on my PR, the way it's structured it'd be trivial to check a frame counter instead of my vsync/60hz timer. If vsync_adjust=0 makes the fpga frame counter unavailable, it can fall back to checking the estimated vsync clock.

[tonytoon]

I'm ok in general. just some doubts:

Looks like redundant info which may distract in action games. If autofire is enabled for specific button then you will feel it. Isn't?

It's possible to put autofire on a button that doesn't obviously act weird with autofire like start or maybe a jump button. I can make it display just the one button you toggled when you turn it on, but there'd be no way to display that information again if you miss it. Displaying all buttons every time doesn't take up too much extra space and lets you see at a glance what's enabled but that's just my opinion. I play on a CRT most of the time so I'll move my MiSTer there to get an idea of how intrusive it is.

Having only 2 rates sounds like "640kb will be enough for everything". Original intention was to minimize usage of ini. From my opinion, changing the rate of autofire according to specific game by buttons is more convenient than changing it in ini several times till you find an acceptable rate for specific game.

Agreed - but I also worry about how many presses for autofire would be too many if we're trying to make it nonintrusive for existing users. The .ini is great for us, but may as well not exist for a lot of users. Especially since this would allow arbitrary entries and ini_settings.sh wouldn't play nicely with it on a controller.

Maybe three rates by default? 10hz/20hz/30hz? Low/High/Max. Some games only poll inputs every other frame and 30hz results in zero shots, so 20hz as a fallback would be nice. (Other games work amazingly with perfect 30hz) 10 is about as fast a regular person tapping real fast, 20 is faster than any person can do, and 30 is as rapid as possible.