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

Add libretro VFS support #58

Merged
merged 4 commits into from Oct 19, 2021
Merged

Add libretro VFS support #58

merged 4 commits into from Oct 19, 2021

Conversation

garbear
Copy link
Member

@garbear garbear commented Jan 30, 2020

Description

This PR adds support for version 1 of the libretro VFS API (libretro/RetroArch#5664). It's a straightforward translation between the libretro side and the Kodi side.

How has this been tested?

Time is short, so I haven't started testing yet. It compiles against Matrix and visibly appears correct. I'm PRing now to get some visibility and keep track of this task for later.

Copy link
Contributor

@AlwinEsch AlwinEsch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good for me.

src/libretro/FrontendBridge.cpp Outdated Show resolved Hide resolved
src/libretro/LibretroEnvironment.cpp Show resolved Hide resolved
@Hedda
Copy link

Hedda commented Sep 4, 2020

Do you think that this will make it into Kodi's "Matrix" release?

@garbear
Copy link
Member Author

garbear commented Sep 4, 2020

The clock is ticking down, but we still have a couple months before Matrix's feature freeze. It just depends on time, and this PR is a rather low priority. I'm happy to test and merge if someone finishes the work though.

@garbear
Copy link
Member Author

garbear commented Oct 15, 2020

I added support for libretro v2 and v3 VFS API.

The code compiles, but I don't have time to test currently.

@Hedda can you help generate test procedures, like what games and cores should we test?

@Hedda
Copy link

Hedda commented Oct 21, 2020

what games and cores should we test?

@garbear Do you know where updated info is upstream about which game cores support libretro VFS and version of its VFS?

https://kodi.wiki/view/Game_add-ons#Libretro_cores

Does not look like libretro VFS support and version information is available in all cores today?

https://github.com/libretro/gambatte-libretro/search?q=vfs

Maybe metadata information about VFS and VFS version should be addon.xml for each Kodi Game Addon?

https://github.com/kodi-game/game.libretro.gambatte/tree/master/game.libretro.gambatte

That way the information would be available in downstream and easily accessible to Kodi RetroPlayer end-users for testing?

@garbear
Copy link
Member Author

garbear commented Oct 21, 2020

what games and cores should we test?

@garbear Do you know where updated info is upstream about which game cores support libretro VFS and version of its VFS?

https://kodi.wiki/view/Game_add-ons#Libretro_cores

The "VFS support" in the article is something different than this PR - it specifies whether a core can load from memory. As you can see, clearly games that accept entire disc images can't support loading from memory.

The support in this PR is about passing a path, even if "VFS support" is no, to the core, and the core uses Kodi's VFS API.

We can only detect if the libretro core supports loading from memory (VFS support in the core), but we can't detect if the core uses Kodi's VFS API, except by manual code inspection.

Does not look like libretro VFS support and version information is available in all cores today?

https://github.com/libretro/gambatte-libretro/search?q=vfs

834 results? https://github.com/search?q=org%3Alibretro+vfs&type=code

Maybe metadata information about VFS and VFS version should be addon.xml for each Kodi Game Addon?

https://github.com/kodi-game/game.libretro.gambatte/tree/master/game.libretro.gambatte

That way the information would be available in downstream and easily accessible to Kodi RetroPlayer end-users for testing?

Manual code inspection just isn't scalable, and has to be re-performed often as cores are updated to use frontend VFS.

@garbear
Copy link
Member Author

garbear commented Oct 19, 2021

We might as well merge this to allow for testing. @AlwinEsch should I target Matrix because that's what we're developing on? Do we later forward-port everything to Nexus and leave Matrix behind?

@AlwinEsch
Copy link
Contributor

Would be OK, but think that a Matrix and Nexus should remain largely the same. As long as nothing has changed in the API for Kodi.

@AlwinEsch AlwinEsch changed the base branch from Nexus to Matrix October 19, 2021 09:32
@AlwinEsch AlwinEsch changed the base branch from Matrix to Nexus October 19, 2021 09:34
@garbear
Copy link
Member Author

garbear commented Oct 19, 2021

So we merge into Matrix?

@AlwinEsch AlwinEsch changed the base branch from Nexus to Matrix October 19, 2021 09:36
Copy link
Contributor

@AlwinEsch AlwinEsch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes can be come in Matrix 👍

@garbear garbear merged commit c7970e9 into Matrix Oct 19, 2021
@garbear garbear deleted the vfs branch October 19, 2021 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants