Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

savestate size #84

Open
zeromus opened this issue Sep 16, 2016 · 5 comments
Open

savestate size #84

zeromus opened this issue Sep 16, 2016 · 5 comments

Comments

@zeromus
Copy link

zeromus commented Sep 16, 2016

Not verified, just pasting from IRC for recordkeeping:

  • On a semi-related note, snes9x_next is (currently) reporting its savestate size as 5000000 bytes, which seems... unlikely.
  • looks like retro_serialize_size (and maybe others) need the memstream closed before calling memstream_get_last_size

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@Alcaro
Copy link
Contributor

Alcaro commented Sep 16, 2016

Yeah, that looks broken.

But I can't find how to fix that thing. There is a memstream_close(), but it takes a stream reference as argument, and memstream_get_last_size() doesn't use anything like that - it pokes some global variables instead.

That entire memstream file is a big mess. If it supports multiple streams, then it shouldn't have those globals at all, it should just require a reference for everything. The globals should be in libretro.c instead.

@zeromus
Copy link
Author

zeromus commented Sep 16, 2016

I'm glad you agree. It gave me a bad feeling too. I'm disappointed it wasn't as simple as I thought at my first glance. Blrrrrrr

@OV2
Copy link
Contributor

OV2 commented Sep 17, 2016

The stream is already correctly closed. memstream_open was changed at some point to have a "writing" parameter, but the mapping to OPEN_STREAM in snes9x.h always calls it with 0. That is why memstream_close always sets last_file_size to the complete size of the buffer. Changing the define to set the writing parameter depending on the mode should fix it.

@Alcaro
Copy link
Contributor

Alcaro commented Sep 17, 2016

size_t retro_serialize_size (void)
{
   uint8_t *tmpbuf;

   tmpbuf = (uint8_t*)malloc(5000000);
   memstream_set_buffer(tmpbuf, 5000000);
   S9xFreezeGame("");
   free(tmpbuf);
   return memstream_get_last_size();
}

memstream_open having a writing parameter would help if we actually used that function.

I guess the relevant refactoring was done after whatever version this is based on (1.52, I think?).

@OV2
Copy link
Contributor

OV2 commented Sep 17, 2016

The memstream here was a hack by themaister to get the old s9x version to save to memory instead of a file. The functions are mapped in snes9x.h:

#define STREAM memstream_t *
#define READ_STREAM(p, l, s)     memstream_read(s, p, l)
#define WRITE_STREAM(p, l, s)    memstream_write(s, p, l)
#define GETS_STREAM(p, l, s)     memstream_gets(s, p, l)
#define GETC_STREAM(s)           memstream_getc(s)
#define OPEN_STREAM(f, m)        memstream_open(0)
#define FIND_STREAM(f)           memstream_pos(f)
#define REVERT_STREAM(f, o, s)   memstream_seek(f, o, s)
#define CLOSE_STREAM(s) memstream_close(s)

S9xFreezeGame causes calls to OPEN_STREAM and CLOSE_STREAM, which then ends up in the memory set up by memstream_set_buffer.
Something like this might work:
#define OPEN_STREAM(f, m) memstream_open(strchr(m, 'w'))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants