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

VFS support #96

Closed
garbear opened this issue Oct 15, 2020 · 5 comments
Closed

VFS support #96

garbear opened this issue Oct 15, 2020 · 5 comments

Comments

@garbear
Copy link

garbear commented Oct 15, 2020

Feature description

I'm bringing up this issue because there's a feature I've started working on. It'll require rcheevos patches, and I'm wondering if it would be acceptable for merge.

The problem to solve is that fopen() is called in this library, but often times a frontend will be loading a game from a SAMBA share or from within a zip file.

The solution follows the path of RetroArch, which added a VFS API so that cores can access games on a VFS (first proposed by me in 2016).

Requirements and drawbacks

The downside of a VFS API is that they seem to be proliferating. RetroArch has an extensive one with 19 functions. Kodi has the equivalent, and I've started exposing kodi's vfs to libretro cores (kodi-game/game.libretro#58). It'll add a lot of code.

However, I think we only need a subset of a full VFS API, which would be these seven functions:

  • get_path()
  • open()
  • close()
  • size()
  • tell()
  • seek()
  • read()

Motivation

The motivation comes from my GSoC student, who finished his project early and went on to add a unique feature: machine captioning of savestates (garbear/xbmc#120).

He even kept going and managed to connect live presence to discord (garbear/xbmc#123).

Being able to caption savestates of games on SAMBA and within zips would be cool.

Thoughts?

@Jamiras
Copy link
Member

Jamiras commented Oct 15, 2020

Open, close, tell, seek and read are already abstracted through https://github.com/RetroAchievements/rcheevos/wiki/rc_hash_init_custom_filereader for most of the uses. When needed, size is calculated using seek to end and tell.

@garbear
Copy link
Author

garbear commented Oct 15, 2020

I didn't see those. That makes my life easier :) I'll open PRs if I need to patch anything.

@garbear
Copy link
Author

garbear commented Oct 17, 2020

We've added VFS support to kodi-game/game.libretro#67. @NikosSiak verified it worked. Thanks for pointing me in the right direction.

Before closing this issue, I noticed some UB (undefined behavior). This occurs if the client passes NULL to rc_hash_init_custom_filereader() here.

Kodi also has a problem, in that the custom filereader is a binary add-on API to Kodi, and binary add-ons can be loaded and unloaded. Therefore, the custom filereader can be invalidated.

To fix both problems, what if we have rc_hash_init_custom_filereader() unset the filereader if NULL is passed? This is sensible and desired behavior, because then we offer a "deinit" function for filereaders to safely unregister themselves.

@Hedda
Copy link

Hedda commented Nov 26, 2020

@garbear is #98 a solution for your issue?

@garbear
Copy link
Author

garbear commented Jun 6, 2021

Looks like that solves my problem. Thanks, issue closed!

@garbear garbear closed this as completed Jun 6, 2021
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