Project

General

Profile

Actions

Emulator Issues #13671

closed

Documentation: RVZ specification does not make mention of required alignment when group is uncompressed

Added by gemarcano 12 days ago. Updated 11 days ago.

Status:
Fixed
Priority:
Normal
Assignee:
-
% Done:

0%

Operating system:
N/A
Issue type:
Bug
Milestone:
Regression:
No
Relates to usability:
No
Relates to performance:
No
Easy:
No
Relates to maintainability:
No
Regression start:
Fixed in:
2409-301

Description

I'm not to what project to assign this to, as technically it's in the emulator repository, but it's related to documentation in the docs folder and thus isn't a bug in the emulator binary itself.

I found while implementing an RVZ parser (for fun) that there is an undocumented alignment requirement when parsing uncompressed group data in a partition. Specifically, it looks like after extracting any/all exception lists from an uncompressed group, the actual group data must be 4 byte aligned. This means, in my case, after extracting an empty exception list (2 bytes on disk, 0x0000), I need to skip an extra 2 bytes of padding so that I'm now pointing to the actual group data.

This behavior seems to be due to this line in the emulator: https://github.com/dolphin-emu/dolphin/blob/97ea64164b68a18aeb9af2d03e4b1da73536db0a/Source/Core/DiscIO/WIABlob.cpp#L1623

The fix is to add this requirement to the docs/WiaAndRvz.md specification. I don't think this impacts WIA, as RVZ is the one that allows for a group to be optionally uncompressed.

Actions #1

Updated by JosJuice 12 days ago

The behavior is documented as follows in WiaAndRvz.md:

For the compression methods NONE and PURGE, if the end offset of the last wia_except_list_t is not evenly divisible by 4, padding is inserted after it so that the data afterwards will start at a 4 byte boundary. This padding is not inserted for the other compression methods.

It may not be clear to the reader if this applies when the file-wide compression method is set to something other than NONE but a specific group is marked as uncompressed. Is this something you would like to see clarified?

(Also, for reference: This behavior is implemented in Dolphin not by the line that modifies uncompressed_size, but the line that calls pad_exception_lists. uncompressed_size is only used for determining if it's advantageous to store the group compressed.)

Actions #2

Updated by gemarcano 12 days ago

Ah, yep, that's what I missed. And yes, that is what bit me, that I assumed that alignment requirement didn't apply when the file-wide compression method isn't NONE. It would be nice to have a line or notice to that effect probably in the RVZ section, as RVZ is the format that can have uncompressed groups even when the file-wide compression is something else.

Actions #3

Updated by JosJuice 11 days ago

  • Tracker changed from Issue to Emulator Issues
  • Project changed from Infrastructure to Emulator
  • Issue type set to Bug
  • Regression set to No
  • Relates to usability set to No
  • Relates to performance set to No
  • Easy set to No
  • Relates to maintainability set to No
  • Operating system N/A added
Actions #4

Updated by JosJuice 11 days ago

  • Status changed from New to Fix pending

I've made a PR to make the documentation use clearer language: https://github.com/dolphin-emu/dolphin/pull/13185

There isn't a specific separate note about the problem you ran into, which is because I felt like it would be clunky to add to the next. The clarifications I added should hopefully make it unlikely that others make the same assumption in the future, but do let me know if you think the revised language isn't enough to address the problem.

Actions #5

Updated by gemarcano 11 days ago

Yep, I think the requested changes are good! Thanks!

Actions #6

Updated by JosJuice 11 days ago

  • Status changed from Fix pending to Fixed
  • Fixed in set to 2409-301
Actions

Also available in: Atom PDF