Allow to choose between old and new speed limiter from ricrpi

[Gillou68310]

New speed limiter from ricrpi doesn't perform very well on android.

[Narann]

Is this function really usefull?

[Gillou68310]

Not really, I'll remove it.

[richard42]

I don't really like this because it introduces complexity rather than properly solving the problem. What is the (qualitative/quantitative) performance difference between the Old and New speed limiters in Android? What is it about the new speed limiter mechanism which causes an issue in Android? It would be better to improve the new algorithm so that it performs well in all operating systems, rather than choosing one or the other.

[Gillou68310]

@richard42 I agree that I took the easy way here. Using the new framelimiter on android causes non linear framerate (eg: framerate beeing to high for a few ms). I was not able to reproduce this issue on PC, so I actually don't know what is causing this issue on Android.

[littleguy77]

@Gillou68310 @richard42
I can confirm that the new framelimiter seems to produce less stable framerates. I think the problem is more pronounced on high-end Android devices. On Super Mario, whereas the old framelimiter maintained a stable 29-30 FPS, the new framelimiter only keeps the FPS in the 28-32 FPS range.

One theory I have is that the averaging window is too large. @Gillou68310 could you try changing the following lines in the new (current) framelimiter, and posting back?

• Change array size to 8 or 16 at https://github.com/mupen64plus/mupen64plus-core/blob/master/src/main/main.c#L740
• Change mask to 7 or 15 at https://github.com/mupen64plus/mupen64plus-core/blob/master/src/main/main.c#L789

[Gillou68310]

Thanks @littleguy77 I lowered the array size to 2 and the framerate is now completely stable. @richard42 what is your opinion on how to deal with this issue?

[wareya]

The framelimiter is a bit screwey, but I guess it's to keep it all in int math. It still has its precision limits because of the array though, but then again I've studied it for minutes instead of hours. Have there been any thoughts about a direct deviance-tracking design, or is the whole floating point part a dealbreaker for certain users?

[littleguy77]

Lowering the array size to 2 will almost completely remove the moving average feature of the framelimiter. I think it's helpful for diagnosis but practically speaking I'd say use a larger value like 8, or eliminate the moving-average algorithm altogether.

I noticed on my 2012 Nexus 7 in DK64 intro, if I set the value to 8, the video seemed a bit more choppy in some places. The lower the array size the choppier the video gets. Basically it's a balancing act between desyncing the audio/video vs. maintaining visual smoothness. Lower array sizes keep tighter sync but reduce smoothness, and vice versa.

[wareya]

I just noticed that ThisFrameDelta is calculated with the CurrentFPSTime whose timestamp reading is from https://github.com/mupen64plus/mupen64plus-core/blob/master/src/main/main.c#L764 and the LastFPSTime whose reading is from https://github.com/mupen64plus/mupen64plus-core/blob/master/src/main/main.c#L780. There are branches between the cracks of these timing reads, which can make the OS scheduler take over for a millisecond and say "screw you". Basically, the LastFPSTime is lower than it should be in those cases, because it doesn't count the time that those branches took.

[littleguy77]

@wareya
So if I understand correctly, we should move lines 778-781 out of and below the if (ThisFrameDelta < 0) block? That way the deltas would always be computed at exactly the same place. The earlier calculation on line 764 would just be a hint to trigger the wait, but not the final retained value.

[wareya]

My bad, I read the code wrong. I was confused by the variables being reused for different things. It should already function correctly.

[richard42]

Are you using the SDL audio plugin in Android, or have you written your own? I ask because this plugin also has a speed limiter which inserts delays based upon the buffer levels at the time when new audio data is fed in from the running ROM.

[Gillou68310]

@richard42 I made some new tests with the dummy audio plugin and the problem is even more noticeable, so I don't think this is related to the audio plugin speed limiter. BTW I just wrote a new audio plugin using OpenSLES, but till now we were using the SDL plugin ;-)

@littleguy77 could you check if this is the same for you, I based all my tests on ZeldaOOT US

[richard42]

Ok, I have done a fair amount of thinking about this. I actually quite like the current speed limiter code, but it does have one big flaw which is that it doesn't communicate with the audio plugin. The reason for the history array is to stay in sync with the audio plugin, but it is open loop due to the lack of communication. And the fact that the audio plugin has it's own speed limiter is also a problem, because they can potentially fight each other.

A proper fix for this requires higher-level design changes. It would be wasted work to do this if we are going to redesign the system API anyway, remove the audio plugin, and add audio functionality to the front-end. I propose that we fix this after bsmiles32 does the system-level audio refactoring work.

I think the way that this ought to work is that when the audio is disabled (or paused for sync) this function should wait if necessary to to consume exactly the amount of time given by the AdjustedLimit. But if the audio is running, then this function should wait as necessary in order to cause the resampled audio buffer level ("secondary" buffer) to converge on its target level. The details are somewhat tricky because this buffer gets fed by callbacks from the running emulator, and drained by callbacks from the audio output (SDL), and these callbacks don't always come at regular intervals. But there's definitely room for improvement here.

Another possible approach, which may be better, is to implement this speed limiter without regard to the audio buffer level, but only inserting delays as necessary to maintain the correct output frame rate (AdjustedLimit). And then feeding forward the actual frame rate to the audio system, which would then dynamically slow down audio as necessary to match the output video frame rate and maintain its target buffer fullness. I implemented this type of mechanism (called Varispeed in the movie industry) in a video player about a year ago. It may also be the best option for us here.

[wareya]

Random thought: If the emulator speed limiter treated the audio's own speed limiter as part of simulation time, would the timing jitter of each limiter fight the other's? That might be where the need for framerate smoothing comes from.

[richard42]

Yes, I think in some cases there could be conflict between the 2 speed limiters, and really there should only be one.

[littleguy77]

@Gillou68310 I definitely see more instability in framerate when using dummy audio, but without audio, I'm not really sure what the correct behavior should be.

@richard42 I have no issue with you delaying the proposed changes until after bsmiles32 refactoring is further along. I personally don't notice the jitter with the audio-sdl, but then again I am not very sensitive to it compared to a lot of users.

[Gillou68310]

@richard42 thanks for taking time to think about this, these are awesome ideas. @bsmiles32 just updated is audio refactoring branch so I'll take a look at his work and see how we can send the current framerate to the audio system.

Before closing this PR I'd like to make a reference of it in @bsmiles32's PR so we can keep trace of this discussion. What it the process to make a reference in git?

[wareya]

Where can I find this updated branch?

[bsmiles32]

@wareya : for the core 1, for the ui-console 2.

[wareya]

Ah, I missed the console end of it :)