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/STFS] STFS structure improvements & CON support #1793

Merged
merged 8 commits into from
May 5, 2021

Conversation

emoose
Copy link
Contributor

@emoose emoose commented May 5, 2021

This brings some improvements to the STFS structures and fixes some STFS handling issues, mostly related to CON profile/gamesave packages:

  • STFS structures have been reorganized & improved to use xe::be<T>/bitfields/accessors, and remove the need for read() functions.
  • BlockToOffsetSTFS has been fixed to work with CON/non-read-only-flagged packages.
  • GetBlockHash now checks whether each levels secondary hash-block should be used or not in packages that have them, and updated the algo used for finding hash block for a given data block.
  • XdbfLocale moved to xbox.h as XLanguage, so that XContentMetadata accessors can make use of it.

The structure improvements are pretty minor, existing structs already mapped out the headers pretty well, just reorganized them a little and let them use xe::be<T>/accessors instead of needing to use a read() function, so now data can be read as/casted to StfsHeader etc directly, and changed some flags fields to use bitfield structs instead.
(also added structs for the license entries at 0x22C since it might be worth linking them up with XamContentGetLicenseMask / XamContentCreateEx's license_mask_ptr param eventually, and added accessor funcs to access the per-language big-endian-unicode strings)

BlockToOffsetSTFS had a bug that seems to break all CON packages as the block_shift used by CONs would get pretty much cancelled out by mistake, I've fixed the block_shift and also renamed it/changed it to a multiply to make it more obvious what the purpose is.

GetBlockHash didn't really check for each levels secondary-block flag previously (there was some code that would make the caller use the secondary-block if a volume-descriptor flag was set, but that'd only be valid if there was just a single hash-level used, ie. 170 data blocks or less), which would prevent certain badly-formatted (but still valid) packages from loading properly.
Updated that code to walk up each hash level and check each secondary-block flag instead, but not sure if it's the most efficient way to handle it, so also added some code to cache each hash-table we read from to try to help reduce how many reads are needed.

Also updated the code for finding the hash block # for a given data block # too, old code seemed to work fine for LIVE/PIRS packages but didn't really handle CON files well, also only found the level0 hash table for a given block with no code for finding level1/2 hash blocks, changed this to use an algo close to what the kernel uses, with some changes to try make it a bit more clear about how it works.

Lastly XdbfLocale was renamed to XLanguage and moved to xbox.h so the accessors in XContentMetadata could make use of it (renamed because AFAIK locale is a different thing to languages, eg. XAM has a partner function for XGetLanguage named XamGetLocale, which seems to be country related going off the values in xam_locale.cc)


So far mostly all the CONs I've tested with seem to work fine now (luckily still have a bunch of badly-formatted packages from some old work on a profile-editing tool, heavily fragmented/corrupt/etc, many STFS tools would fail with these while X360 itself never had any trouble, looks like xenia-vfs-dump now also works fine with them)

Haven't been able to test with many XBLA packages yet, but the few I've tried (~700MB one + some smaller TU packages) still worked fine at least, only have a single SVOD/GoD package to test with too but didn't seem to cause any issues there, would appreciate if someone with access to more packages could test it out.


This doesn't help with letting Xenia/XamContent* actually see CON savegame/profile files though, even if Xenia could load from CONs there's still issues with writing back to them too (eg. a save loaded from CON package would fail when game tries to update it...)

In xenia-canary this was kinda worked around for profiles by making the profile-loader code extract the package automatically and read/write to the extracted folder instead, maybe something similar could be done with StfsContainerDevice itself too.

I've made a proof-of-concept stfs-folders branch that mostly works like that and it seems to work fine, just changes StfsContainerDevice so that it can extract the files next to the package & create/override itself with a HostPathDevice, with ContentPackage class updated to use StfsContainerDevice instead so that STFS packages can be seen by the XamContent* funcs.

With that any packages loaded via XamContent* should get the correct display-name/content-type/etc passed to the game, and if the package isn't read-only (such as gamesaves/profiles) it'll extract any contents and read/write to the extraction folder instead, packages created by Xenia will also have STFS headers written out for them automatically, so would probably help with #1407 & #1739 too (and makes #1408 no longer needed)

(package headers are read/written to "[package-path]" file, with "[package-path].data" folder used for the extraction folder/HostPathDevice, this follows SVODs convention of storing data files inside a "[package].data" folder next to the SVOD package/header file)

Didn't need as many changes as I thought to get working, but not really sure if this is the best way to handle this, would welcome any thoughts about it (maybe should make a separate draft PR for that after this one?)

uint32_t lang_id = uint32_t(language) - 1;

const be<uint16_t>* str = 0;
if (lang_id >= 0 && lang_id < 9) {
Copy link
Member

Choose a reason for hiding this comment

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

I'm not a big fan of magic numbers over there.
Same thing with few lines below

Copy link
Contributor Author

@emoose emoose May 5, 2021

Choose a reason for hiding this comment

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

Ah I did mean to do something about that but forgot to go over it, I guess since they're metadata_version related I could change them to kNumLanguagesV1/kNumLanguagesV2 instead.

emoose added 8 commits May 5, 2021 11:06
Now makes use of xe::be<T> and removes need for read() method
Previous code would give wrong results for those types of packages as the block_shift would cancel itself out, moving it to happen afterward let it give the right result though, probably a mistake while reversing it.
(eg. block 0 would give 0xB000, but that's wrong for non-read_only packages as 0xA000 and 0xB000 are reserved for the first hash-table there, fixed code returns the correct 0xC000 offset)

Also changed it to a multiply & renamed it to blocks_per_hash_table to make it more obvious what the purpose is.
GetBlockHash now visits upper-level hash tables (if needed) to check the active_index flag that decides if secondary hash-block should be used or not.
This should give better support for CON packages, important for X360 profiles/game saves (some well-used profiles can be heavily fragmented)
Renamed to XLanguage because AFAIK locale on Xbox is a different concept involving countries, this enum only involves languages though.
@emoose
Copy link
Contributor Author

emoose commented May 5, 2021

Updated with requested changes

@gibbed gibbed merged commit ce19d94 into xenia-project:master May 5, 2021
@gibbed
Copy link
Member

gibbed commented May 5, 2021

@emoose Draft PR for your folders idea is fine, though I'm not a fan of the folders thing with automatic file unpacking. In many cases this would be a large amount of data.

Want to eventually get non-mmap version of stfs stuff in so we can do mounts of content from non-host file sources, for things like #1247.

@gibbed
Copy link
Member

gibbed commented May 5, 2021

These changes fix mounting of YouTube's container which was breaking badly before. I know of at least one other title that was exploding previously as well, will test eventually.

@emoose
Copy link
Contributor Author

emoose commented May 11, 2021

@gibbed been looking into adding write support to StfsContainerDevice, got something working that writes out packages close to X360 CONs, with STFS headers/directories/hash tables/etc.

So far StfsContainerDevice can read back packages that it's written out at least, but still need to test it some more and fix up a few things.

Do you think STFS writing is something worth exploring more though? I wasn't sure about adding things that write to binary-blob files like this when plain folders would do, but figured if Xenia wants to emulate X360 fully it'd eventually include writing out the same save format.
Would help transferring stuff between console & Xenia at least, and should nail down the gamesave/DLC issues we've had because of STFS header crap too.

Also switched the code to use xe::filesystem::OpenFile instead of MappedMemory as part of this too, maybe can help with those nested STFS packages, could PR it separately maybe: https://github.com/emoose/xenia/commits/stfs-openfile

E: pushed what I got for STFS writing so far to https://github.com/emoose/xenia/commits/stfs-writer, testing with Goldeneye shows it's creating the save & reading/writing to it, and restarting GE lets it load in any changed settings fine. It should be handling deleting/resizing files & directories too, but I haven't tested that out much yet (if anyone knows any games that use folders in their save files please let me know)

This changes a lot of how STFS reading works too, commit msg & some of the comments go into it a little, but I think it should help with #1371 at least - problem there seems to be corrupt hash tables (maybe need to check it on a retail box?), new code should verify hash tables, notice that it's invalid, and then try salvaging things by using next_block = cur_block + 1 instead (this salvaging is only done for LIVE/PIRS containers since they're almost always sequential, which can't be guaranteed with CON)

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.

3 participants