libretro / snes9x2010

Snes9x 2010. Port of Snes9x 1.52+ to Libretro (previously called SNES9x Next). Rewritten in C and several optimizations and speedhacks.
Other
98 stars 70 forks source link

Add optional automatic frame skipping #139

Closed justinweiss closed 3 years ago

justinweiss commented 3 years ago

This PR uses the RETRO_ENVIRONMENT_SET_AUDIO_BUFFER_STATUS_CALLBACK environment to implement optional automatic frame skipping. This is based on the implementation in snes9x2005. (https://github.com/libretro/snes9x2005/pull/77 and follow-ups)

cc @jdgleaver

jdgleaver commented 3 years ago

Excellent! Looks good to me :)

Thanks for adding this!

justinweiss commented 3 years ago

When implementing automatic frameskip in other cores, did you notice any additional drops in framerate with frameskip on?

This core doesn't call video_cb if the frame is skipped, so it is reporting the number of frames actually rendered / s, instead of always reporting a frame every pass through retro_run.

The weird thing I'm seeing that with frameskip off on n3ds, using Mega Man X as my example (beginning of the intro stage), I see 50fps reported with frameskip off. With frameskip auto, the music is fixed, but it reports 40fps. I don't fully trust my ability to tell between 1 / 3 frames skipped and 1 / 6 frames skipped, but it seems much closer to 1 / 3 to me.

There could be a reasonable explanation for this -- maybe because we're not completely draining the sound buffer, it's skipping more aggressively (although I see the exact same framerate with manual / 15 and manual / 60). Is there anything you can think of in RetroArch that could cause such a drop between skip on and skip off? I would expect there to be a difference, but this seems high.

justinweiss commented 3 years ago

I did some logging, and it seems like the problem is that assuming the audio driver consumes audio at around the same rate as it's sent, and that we're consistently less than 60fps, it will always stay right around the underrun threshold. This means that any hiccup would cause a frameskip.

Should the audio driver try to do a better job of keeping itself somewhere in the middle of its buffer? Or maybe it's just that lower than a minimum framerate, there's not much hope of recovering the audio buffer back to a point where it's more than a frame past the risk of underrun?

jdgleaver commented 3 years ago

When implementing automatic frameskip in other cores, did you notice any additional drops in framerate with frameskip on?

I didn't notice anything obvious in any of the other cores...

This core doesn't call video_cb if the frame is skipped, so it is reporting the number of frames actually rendered / s, instead of always reporting a frame every pass through retro_run.

Ah, I didn't realise that!

This is a problem - whenever a frame is skipped, video_cb() must be called with NULL as the video buffer.

Should the audio driver try to do a better job of keeping itself somewhere in the middle of its buffer? Or maybe it's just that lower than a minimum framerate, there's not much hope of recovering the audio buffer back to a point where it's more than a frame past the risk of underrun?

This would be the ideal - to aggressively fill the audio buffer whenever possible. I'm not quite sure how to go about implementing this, however - and yes, when the mean framerate is below a certain level, there's not really much that we can do... (i.e. some standalone emulators artificially fill the audio buffer by pumping additional frames each iteration when the framerate really drops, but RA basically needs one frame per iteration...)

justinweiss commented 3 years ago

Thanks for your help!

This is a problem - whenever a frame is skipped, video_cb() must be called with NULL as the video buffer.

Ah, I didn't know that. It should be a straightforward fix, I'll add that and give it another try. Out of curiosity, what would be expected to go wrong by not having it?

I'm not quite sure how to go about implementing this, however - and yes, when the mean framerate is below a certain level, there's not really much that we can do...

Same, I tried a few things but they all just made the frameskipping worse / more noticeable. I wonder how the buffer fills to begin with, though, assuming we're only adding a frame's worth of audio every pass through retro_run. Does it maybe have to do with dynamic rate control? I have to admit that I'm still not too familiar with how all the parts fit together.

jdgleaver commented 3 years ago

Thanks for the video_cb(NULL, ...) fix!

Ah, I didn't know that. It should be a straightforward fix, I'll add that and give it another try. Out of curiosity, what would be expected to go wrong by not having it?

It's not an entirely fatal error, but if we don't call video_cb() each frame then it (a) completely breaks video stats, (b) messes up osd/widget timings and (b) means the internal frame counter is wrong, which can upset runahead and may also cause subtle issues at the gfx driver level (platform dependent - doesn't affect 3DS, fortunately!)

Same, I tried a few things but they all just made the frameskipping worse / more noticeable. I wonder how the buffer fills to begin with, though, assuming we're only adding a frame's worth of audio every pass through retro_run. Does it maybe have to do with dynamic rate control? I have to admit that I'm still not too familiar with how all the parts fit together.

It is a bit complicated - made worse by the fact that every audio driver handles things differently, and depending upon the platform/OS, we often don't really have much control over how the buffer is populated. Dynamic rate control doesn't have much affect here - the main thing this does is resample the audio for the current frame, to match the actual frame timing. The most practical thing we can do is just increase the audio latency, which requests a larger buffer (that doesn't necessary mean the driver will try to fill the buffer, but it will in general mean more data is pipelined).

Ultimately, if the device is too slow to consistently fill the buffer (even with frameskipping), there's little to be done. As I said, I've seen stand-alone emulators 'cheat the system' by running multiple frames as fast as possible to force data into the audio buffer when it's running dry, but RA doesn't quite support that sort of run mode... (and even if we did disable synchronisation to make iterations happen without any frame pacing, you'd effectively end up with fast-forward tripping on and off - not nice...)

justinweiss commented 3 years ago

Thanks for the info!

made worse by the fact that every audio driver handles things differently,

Ah, yeah -- I saw that when looking for examples when writing the dsp_thread driver :-) Anyway, this all makes a lot more sense to me now. Thanks a lot!

And thanks for the great example to follow. This makes the new Bahamut Lagoon translation play well enough, which 2005 isn't compatible with (though, strangely, snes9x_3ds is -- even though it's based on the same upstream version as 2005?)

inactive123 commented 3 years ago

Hi, is this ready to be merged?

jdgleaver commented 3 years ago

@twinaphex Yes, please merge

Renetrox commented 3 years ago

it would be great to have a similar solution in n64 emulators!