Issue with how retro_serialize/retro_unserialize work

[barbudreadmon]

retro_serialize/retro_unserialize is needed for a lot of libretro features, to the point it breaks standard savestates saving/loading in some cases.

I'll talk about 2 issues i have with this in fbalpha :

• savestates are not compatible with standalone fbalpha, standalone compress its savestates with deflate, but if we do that in retro_serialize/retro_unserialize we could end up having performance issues with netplay, rewind, read ahead, ...
• sometimes the libretro port needs to remove a part of the savestate to save bandwidth because it's not necessary and harmful for netplay. However it prevents proper sprite loading when loading a savestate. One good example is CPS3 games : try saving, leaving, then reloading your savestate.

If we can't determine the purpose of a call to retro_serialize/retro_unserialize through a third argument, an api call, or whatever; then the only option i see is having a set of 2 new separate functions to handle normal savestate saving/loading.

[inactive123]

But you point out yourself why compression is not really desired from a libretro core perspective, since that would cause performance issues for netplay, rewind, readahead.

It's too bad if certain standalone emulators do stuff a certain way that is not 1:1 applicable to how libretro does things. Libretro requires things to be done differently, and most of the time, there are very valid reasons for this. You would not want states to be compressed.

I really do not want to start introducing separate paths for savestate saving/loading - there is no reason to spin this off from the retro_serialize calls. What is the motivation here? So the libretro core can use the same savestates that standalone would? But why would we do that or want that when there is nothing but drawbacks to this method when they start compressing a state or whatever differs from the libretro states? It is a non-issue as far as I am concerned.

Emulator developers will just need to accept that this is the way libretro does things, and we are not going to change how the entire API works, introduce an API break, just because of certain (bad IMHO) habits that die hard. Libretro will require different codepaths vs. a standalone emulator, and that is just the way it has to be.

[inactive123]

sometimes the libretro port needs to remove a part of the savestate to save bandwidth because it's not necessary and harmful for netplay. However it prevents proper sprite loading when loading a savestate. One good example is CPS3 games : try saving, leaving, then reloading your savestate.

We made the conscious decision to change this in FBAlpha 2012 because it meant much faster netplay. This is simply an implementation detail unrelated to the API. If it is not wanted, simply don't implement it in your core. I believe this isn't done in mainline FBA anyway so what is the issue there?

Honestly, I see no point to changing the entire way libretro works (and this would not be possible since it would break the API anyway) just for these niche usecases.

[inactive123]

. However it prevents proper sprite loading when loading a savestate. One good example is CPS3 games : try saving, leaving, then reloading your savestate.

Yes, we were aware of this when @Themaister and me made that hack. However, those missing sprites only last for about a few frames - after less than half a second or so they return to normal. This was seen as an acceptable tradeoff vs. sending back and forth 8 megabytes of sprite/character data.

I don't see why this would be a big issue if you could just turn this into a core option.

[andres-asm]

With regard to compression, the frontend should compress / decompress states on the fly (only when flushing).

RetroArch doesn't flush for rewind, runahead or netplay. Regarding upstream compatibility, it's not important imho.

If it becomes important we could add an "import/export state" feature or something like that that imports/exports states for upstream compatibility but doesn't have the usual hooks (again netplay, rewind, runahead) it would basically leverage upstream savestate code to read/write compatible states in a predefined folder.

For netplay, and removing data. Well there is an environment callback for FAST_SAVESTATES already, we just need to use it. For instance we could request FAST_SAVESTATES when netplay is underway.

[inactive123]

BTW @barbudreadmon, none of this is meant negatively. I hope none of my prior posts were taken the wrong way since I highly value all the work you have done on FBAlpha.

I am just trying to prevent a hard break in the libretro API 8+ years into its lifespan. I just want to avoid that at all costs, hence my trepidation with issues raised like this.

[inactive123]

@barbudreadmon Would leveraging the environment callback FAST_SAVESTATES perhaps be a solution as @fr500 proposed? For instance, the CPS3 speedhack for serialization could be applied only when that environment callback returns true for instance.

Just trying to think of possible solutions here while staying within the framework of libretro.

[barbudreadmon]

@twinaphex fair enough, i wouldn't want to break anything, still if there is a way to do this without breaking the api :

• about savestates compatibility, i've seen a lot of topics about how to convert saves from emulator X to emulator Y over the years, so it seems it is kinda a hot topic for users. I would use this compatibility myself for debugging.
• about the missing sprites, they are not missing for "only a few seconds", in the case of a mid-battle sfiii save, background will be missing until the end of the fight after you reload. Rewind is also barely usable on sfiii with sprites becoming artifacts.

So i think they are real issues and they should be taken in consideration. I don't think controlling the content of savestates through a core option would be "sane". Also, i don't know anything about fbalpha2012 (i don't even know if the work done over the last year for full cross-platform netplay compatibility was backported). I'm talking about mainline fbalpha here, and when i removed the cps3 "hack" in the past (i didn't understand why it was there at first), i'm pretty sure you were the one who told me to put it back. Yeah i'm all for @fr500's solution.

[Dwedit]

Savestates being forced to a fixed size has always been problematic. I'd redesign the whole thing to use a COM-style memory stream object instead of a plain pointer, then you wouldn't need to worry about malloc/free functions not matching between modules.

[Dwedit]

And I am completely serious about the COM-style interfaces thing, here's an example of what a Memory Stream class might look like if implemented in C.

https://pastebin.com/9RGSnGfA

[barbudreadmon]

@twinaphex @fr500 Any news about the leveraging of the environment callback FAST_SAVESTATES ?

[andres-asm]

let me check right now

[andres-asm]

+++ b/dynamic.c
@@ -1837,6 +1837,8 @@ bool rarch_environment_cb(unsigned cmd, void *data)
 #ifdef HAVE_NETWORKING
          if (netplay_driver_ctl(RARCH_NETPLAY_CTL_IS_REPLAYING, NULL))
             result &= ~(1|2);
+         if (netplay_driver_ctl(RARCH_NETPLAY_CTL_IS_ENABLED, NULL))
+            result |= 4;
 #endif
          if (data != NULL)
          {

This should do it. Of course you need to add something like this to the core: https://github.com/snes9xgit/snes9x/blob/master/libretro/libretro.cpp#L1529

[andres-asm]

Are you on discord or IRC? (it's far easier to work these things out in real time

[barbudreadmon]

If you don't mind the text mode (i'm not fluent enough in english), i can connect to discord, IRC is a pain in the ass to configure...

[andres-asm]

Yeah we prefer discord nowadays too, and don't worry I don't voicechat either :) https://discord.gg/6xdkfr