libretro / RetroArch

Cross-platform, sophisticated frontend for the libretro API. Licensed GPLv3.
http://www.libretro.com
GNU General Public License v3.0
10.08k stars 1.81k forks source link

simultaneous button presses don't produce the expected behavior #2043

Closed andres-asm closed 8 years ago

rsn8887 commented 8 years ago

Can you test what happens if you flush every 15ms instead, to get at least one complete input update per frame? Although it is great progress to find something that solves this problem, the 67ms delay is too long to be a viable solution. Many retro games depend on updating the input every frame. Street Fighter 2 comes to mind.

If 15ms doesn't work, can you test the smallest possible delay that does work?

diablodiab commented 8 years ago

I initially lowered the flushing delay to 25 ms but it became increasingly difficult to hit two buttons simultaneously within that window of time. I think I managed to do it once every 9-10 times I tried.

In the commit description, I referred to some articles discussing controller input delays and the reasoning behind choosing 67ms as a starting point.

The problem occurs when input is updated once per frame as that window is simply too small to be able to hit two buttons at once. My guess it that the input delay in the other drivers is higher than we expect which is why it is easier to do button combinations in those drivers.

You should try the compiled version and see for yourself if you can tell the difference.

Googer commented 8 years ago

Conceptually isn't this the same as if RA only polled the controller input queue once every ~67 ms (i.e., once for every 4 frames of emulation on a 60 fps emulated core) instead of every one? I haven't tried this yet but I could see this being a bit annoying from a responsiveness point-of-view to the user, since sometimes a button press will respond nearly instantly (if pressed right before a flush); other times it'll be close to the full buffer length (i.e., 67 ms)

Also on average this'll add another ~33 ms / 2 frames of input delay on average on top of whatever internal buffering the emulated game likely already has and intrinsic delay from the display being used. So it may work but it's definitely not optimal. :stuck_out_tongue:

Googer commented 8 years ago

Having actually tested this now, I can say it basically works but the large buffer actually causes another problem as well that's probably worse in a way - if you press and release a button quickly, the press can be missed altogether. This is because both the press and release can be registered in the same buffer, so the button press is never propagated to the emulated core at all. Note that using a short buffer wouldn't get rid of this problem entirely but would reduce the likelihood of it occurring. Theoretically the way RA does controller polling this has always been an issue if a person is capable of hitting and releasing a button in less than a single emulated frame. :wink:

meepingsnesroms commented 8 years ago

Could we flush the buffer exactly on frame updates(because most games probably don't check more than once per frame) then there would be no waiting.(It could be every 2 or three frames if needed)

With pushing it quick enough not doing anything,this applies to consoles too if you press and release the button before the game checks controller register.(Unless that specific console has an interrupt on button press)

Googer commented 8 years ago

Correct, the only way to completely avoid that issue altogether would be to use an interrupt-driven system. And as you say, this even applies to games being run on physical consoles. :laughing:

rsn8887 commented 8 years ago

So why does this work on other Android emulators, e.g. .emu programs, and even in Retroarch on other platforms? For example, I know it is possible in some Mame shmups to shoot 30 times a second, e.g. every second frame. But with this workaround it would be impossible to do so. Clearly there is an underlying issue that needs to be fixed and this workaround of "slow polling" bypasses that issue. In Windows I am using a controller that has a refresh polling rate of 120 Hz, and I have no problem pushing multiple buttons in any emulator.

meepingsnesroms commented 8 years ago

Because they don't poll,they use a soft interrupt(onKeyDown,onKeyUp,onKey) in java that then calls C.

This is the only emulator that uses the NativeActivity,and its unoptimized because almost no one uses it or cares.

rsn8887 commented 8 years ago

Are you saying that a single poll takes longer than one frame? If that is the case, then yes, this will never work correctly.

Otherwise, if an input poll can be completed once per frame, I don't think it should matter whether we use interrupts or polling, as long as we update once per frame.

Here's why: If we would react using interrupts, then the button bits will ideally be set as soon as they are pressed/released. However, the emulated game will only react to these button bits once per frame. So effectively from the games perspective, the key bits are updated once per frame.

If we poll fast enough (once per frame), then the same thing should happen.

That is why I think that there's an underlying issue. For example, if you look at Broglia's Java code, he is just reacting to the input events by doing:

void EmuSystem::handleInputAction(uint state, uint emuKey) { uint player = emuKey >> 30; // player is encoded in upper 2 bits of input code assert(player <= 4); uint16 &padData = input.pad[playerIdxMap[player]]; if(state == Input::PUSHED) setBits(padData, emuKey); else unsetBits(padData, emuKey); }

In retroarch, the same thing is happening (but somewhere there is clearly a bug).

The only difference is that Broglia's code reacts to an interrupt, while Retroarch actively polls. We just have to ensure that the polling is fast enough.

meepingsnesroms commented 8 years ago

The error is probably not in retroarch,but in the nativeactivity library side of the queue. Android might only push one keyevent to the queue per frame,but with the interrupt it just calls it for every event at the same time.

Since this is the only thing different between working and not working is interrupts vs polling,and no one else uses this library,it is most likely an os bug.

udev for Linux polls an event queue and it works too,so all that's left is that android does not provide events fast enough when you don't use the interrupt.

diablodiab commented 8 years ago

Do any of you have an idea of the typical delay one can expect when trying to hit two gamepad buttons at once. The human factor. I'm guessing it will be very rarely that you hit them exactly at the same time.

So how much time between the two button presses would you think there would be?

rsn8887 commented 8 years ago

The game/emu in question will have already solved this problem. All retroarch needs to ensure is that the button state is polled and updated once every frame. I suspect that there is some crazy lag/delay somewhere in the polling routine that prevents this update from happening fast enough. For example ifthe polling itself takes multiple frames, then the simultaneous key presses will most often appear as two separate presses several frames apart. A simple logging of button presses together with an integer frame count number would help debug this. The logging should only turn on on a certain button press and simply output button bits and framenumber on every frame as long as some other key is held down.

diablodiab commented 8 years ago

I'm still curious about this question.

Polling the input every frame at 60 fps will require you to press two buttons within a maximum of ~17ms - sometimes even less in order for it to be registered as a simultanous button press. I'm interested in knowing if that is what we are actually doing in those other emulators. Pressing both buttons within that timeframe - or if there is some sort of queue and flushing going on there as well to allow for button presses to stack up before they are checked (and over a time period that is longer than a single frame).

diablodiab commented 8 years ago

If the polling routine itself is taking longer than one frame, then the buffering routine i tried to implement should fix this when I set the delay to 0.

Then the polling routine would spend a few frames going through input events and updating the temporary state which is then finally flushed. The flushing is done within one frame otherwise the hack i did would not solve the problem.

I think meepingsnesroms has the right idea when he says that "Android might only push one keyevent to the queue per frame". At least with the current way that keyevents are placed into the inputqueue.

As I wrote earlier it starts getting difficult to do double presses when I lower the delay.

rsn8887 commented 8 years ago

I will use "polling" here to refer to the process of "queueing up events followed by processing events". Let's say you are pressing two keys almost simultaneously, however the first key is captured by the polling, but the second key came slightly too late when the polling already started. Now this polling takes several frames because it is slow. After the polling, the game will see a single button pressed. Only on the next polling the other simultaneous key will also be registered. To the game/emu this will look like

first key pressed several frames delay first and second key pressed

even though you pressed the keys simultaneously, maybe even on the same frame, this is not reflected due to the lag in polling.

The fact that Android has horrendous input lag ~100ms on most devices makes me wonder if the polling is just way too slow. The logging I described should reveal that.

rsn8887 commented 8 years ago

diablodiab: "Polling the input every frame at 60 fps will require you to press two buttons within a maximum of ~17ms - sometimes even less in order for it to be registered as a simultanous button press."

This is simply not true. It only means that the emu/game in question receives input updates at the maximum frequency. It is up to the game/emu to decide how to deal with those inputs. The coders of the original games will already have ensured how they want to queue/handle these inputs. Original consoles used interrupts, e.g. they received input changes every frame, but this didn't mean the user had to press the buttons in the same frame. The game code will deal with this on a game-by-game basis.

diablodiab commented 8 years ago

It would be interesting to see the results of the logging you mentioned, could you please look into this and provide the results?

zeromus commented 8 years ago

RSN8887's logging is the way to go. Print a high resolution timestamp when an event is processed, and the keymask each frame. Speculation is baseless without that data

diablodiab commented 8 years ago

I've done some performance measurements of the polling function in android_input.c and it's executing much faster than one frame, so that's not the bottleneck.

However, I did find an interesting thing in the polling function: It turns out that if you change the ALooper_pollAll call in the polling function from a non blocking call into a 1ms waiting call then the inputQueue will be filled up with more input events and it actually seems to solve the problem with hitting more than one button simultaneously.

This is the change to the code: https://github.com/diablodiab/RetroArch/commit/58c5cc6f46ce07581e221c6e2213a76701bf19e6

Here is a binary for testing: https://www.dropbox.com/s/41t3lbjfmk2onfo/retroarch-release.apk?dl=0

I've tested it with a bunch of games, and so far I've only noticed one side effect of this. On Nvidia Shield TV you would normally press the Nvidia button on the game controller to go into the menu. Normally this would send a keycode 84 to the input buffer, but since this is a special button that switches keycode when you hold it down it actually ends up doing both a key down and key up event during the 1ms wait. So instead one would have to use keycode 203 for that button, which is what it turns into when you hold it down.

I'm assuming that input is handled in its own thread and that the 1ms waiting in the input pulling does not have en effect on the general performance, but I would be interested to hear your thoughts on this.

Please try out the binary and let us hear.

diablodiab commented 8 years ago

Done :)

rsn8887 commented 8 years ago

Awesome work I think this might be the fix we have been waiting for. Thanks diablodiab for the prompt investigation of the polling loop performance. I stand corrected.

blackman91 commented 8 years ago

Wow so diablodiab managed to fix single-handedly the major bug everyone else failed at. You are freaking awesome man!

Googer commented 8 years ago

Works perfectly without the side effects of the buffering method. I approve. :smile: