Extract Content IDs from PS4/PS5 PKGs#390
Conversation
📝 WalkthroughWalkthroughAdds a PS4 helper to extract package content IDs from ISO9660 app entries and prints them in PS4::printInfo; PS5 now inherits from SystemPS4 and uses the helper to extract and print app_sc.pkg content IDs. ChangesPS4 Content ID Extraction and PS5 Inheritance
Sequence DiagramsequenceDiagram
participant SystemPS4
participant getContentIds
participant ISO9660Root as app directory
participant SystemPS5
SystemPS4->>getContentIds: getContentIds(root_directory, pkg_filenames)
getContentIds->>ISO9660Root: enumerate subdirectories
ISO9660Root-->>getContentIds: pkg entries
getContentIds->>getContentIds: validate magic and extract content IDs
getContentIds-->>SystemPS4: concatenated IDs or empty
SystemPS5->>getContentIds: getContentIds(root_directory, app_sc.pkg) [inherited]
getContentIds->>ISO9660Root: enumerate subdirectories
getContentIds-->>SystemPS5: concatenated IDs or empty
SystemPS5->>SystemPS5: print IDs or return early
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@systems/ps4.ixx`:
- Around line 85-105: The code reads fixed offsets from app_pkg_raw (via
app_pkg_entry->read()) without size checks, risking out-of-bounds access when
the buffer is truncated; update the logic that computes app_pkg_magic and
constructs app_pkg_content_id (using pkg_magic_offset, pkg_magic_size,
pkg_content_id_offset, pkg_content_id_size, and app_pkg_content_id) to first
verify app_pkg_raw.size() is at least the maximum required offset+size (e.g.
max(pkg_magic_offset+pkg_magic_size, pkg_content_id_offset+pkg_content_id_size))
and if not, silently skip this entry (no warning) so truncated/corrupt PKGs are
ignored safely. Ensure subsequent uses of app_pkg_raw assume the validated size.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4cad8068-5283-4805-97c9-5446d4855bb9
📒 Files selected for processing (2)
systems/ps4.ixxsystems/ps5.ixx
- added size validation, - removed unused const, - set some of params/variables to const, - adapted code style to match project.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@systems/ps4.ixx`:
- Around line 87-90: The current implementation in the app_pkg_entry->read()
call loads the entire PKG file contents, but the subsequent logic only needs the
header bytes up to _PKG_HEADER_SIZE. Replace the full-file read with a bounded
read operation that retrieves only the header bytes (0x1000 bytes as defined by
_PKG_HEADER_SIZE) to reduce latency and memory consumption for large PKG files.
Ensure the bounded read still provides enough data for the size validation check
and any subsequent magic/content-ID checks that depend on the header.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3c22fa38-cb8f-4e89-8f44-67f60fee3f39
📒 Files selected for processing (2)
systems/ps4.ixxsystems/ps5.ixx
🚧 Files skipped from review as they are similar to previous changes (1)
- systems/ps5.ixx
| auto app_pkg_raw = app_pkg_entry->read(); | ||
|
|
||
| if(app_pkg_raw.size() < _PKG_HEADER_SIZE) | ||
| continue; |
There was a problem hiding this comment.
Avoid full-file reads for PKG header parsing.
At Line 87, app_pkg_entry->read() loads the entire PKG even though this logic only needs header bytes up to _PKG_HEADER_SIZE (0x1000). For large PS4/PS5 PKGs, this can significantly increase redumper info latency and memory pressure. Prefer a bounded header read (or a header-only helper) before magic/content-ID checks.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@systems/ps4.ixx` around lines 87 - 90, The current implementation in the
app_pkg_entry->read() call loads the entire PKG file contents, but the
subsequent logic only needs the header bytes up to _PKG_HEADER_SIZE. Replace the
full-file read with a bounded read operation that retrieves only the header
bytes (0x1000 bytes as defined by _PKG_HEADER_SIZE) to reduce latency and memory
consumption for large PKG files. Ensure the bounded read still provides enough
data for the size validation check and any subsequent magic/content-ID checks
that depend on the header.
There was a problem hiding this comment.
I thought the read() opens the pointer to the data that then can be read on demand? Unsure if I am in the wrong or AI.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find iso9660 filesystem module files
fd -e ixx -e hh -e h -e cc -e cpp iso9660
# Search for the read() method implementation in iso9660
rg -n "read" --type-add 'ixx:*.ixx' --type ixx -C3Length of output: 203090
@RobbedColek — good instinct to question it, but having looked at the actual implementation in filesystem/iso9660/iso9660_entry.ixx, read() is an eager, full-file load, not a lazy reader:
std::vector<uint8_t> read(bool form2 = false, bool *form_hint = nullptr)
{
// allocates a buffer for the ENTIRE file upfront
std::vector<uint8_t> sectors(sectorsSize() * _dataReader->sectorSize(form2));
// reads ALL sectors into the buffer immediately
uint32_t sectors_read = _dataReader->read(sectors.data(), sectorsLBA(), sectorsSize(), form2, form_hint);
sectors.resize(size);
return sectors; // returns the full contents
}It pre-allocates sectorsSize() × sectorSize bytes (the whole file), calls through to _dataReader->read() (which hits the underlying file stream), and returns a fully populated std::vector<uint8_t>. There is no lazy/streaming interface here.
So the concern is valid: for a PS4/PS5 app PKG that can easily be several gigabytes, app_pkg_entry->read() will attempt to load the entire file into RAM just to check the first 0x40 + 36 = 76 bytes. A bounded read (if the underlying reader supports seeking + partial reads) or a dedicated header-reading helper would be significantly more efficient.
🧠 Learnings used
Learnt from: Didrole
Repo: superg/redumper PR: 356
File: systems/gc.ixx:53-59
Timestamp: 2026-02-26T22:08:07.395Z
Learning: In systems/gc.ixx and systems/wii.ixx (and other files under systems with .ixx extension), when BCA file validation fails (e.g., unexpected size), the function should return silently and not emit a warning. Reviewers should verify that BCA validation failures do not log warnings and consider adding tests to cover the silent failure behavior.
Learnt from: Didrole
Repo: superg/redumper PR: 356
File: systems/wii.ixx:54-60
Timestamp: 2026-02-26T22:08:15.577Z
Learning: In systems/wii.ixx and systems/gc.ixx, when a .bca file exists but its size does not match _WII_BCA_STRUCTURE_SIZE or _GC_BCA_STRUCTURE_SIZE, the code is expected to skip silently without emitting a warning or error. This behavior should be consistently maintained across similar modules. If this pattern could appear in other modules, prefer the glob pattern systems/**/*.ixx to cover all related files. Consider adding a brief inline comment near the relevant size-check to document the rationale, and add a targeted unit/integration test to verify there is no warning or error emitted for mismatched sizes.
There was a problem hiding this comment.
@RobbedColek looks like this is legit callout, those pkg files are huge.
It's a little bit inconvenient but I would ask instead of using directory read(), use data_reader directly and directory entry sectorsLBA() method to get LBA and only read what you need.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| auto app_pkg_raw = app_pkg_entry->read(); | ||
|
|
||
| if(app_pkg_raw.size() < _PKG_HEADER_SIZE) | ||
| continue; |
There was a problem hiding this comment.
@RobbedColek looks like this is legit callout, those pkg files are huge.
It's a little bit inconvenient but I would ask instead of using directory read(), use data_reader directly and directory entry sectorsLBA() method to get LBA and only read what you need.
| uint32_t app_pkg_magic = | ||
| (static_cast<uint32_t>(app_pkg_raw[_PKG_MAGIC_OFFSET + 0]) << 24) | | ||
| (static_cast<uint32_t>(app_pkg_raw[_PKG_MAGIC_OFFSET + 1]) << 16) | | ||
| (static_cast<uint32_t>(app_pkg_raw[_PKG_MAGIC_OFFSET + 2]) << 8) | | ||
| (static_cast<uint32_t>(app_pkg_raw[_PKG_MAGIC_OFFSET + 3]) << 0); |
There was a problem hiding this comment.
https://github.com/superg/redumper/blob/main/systems/ps3.ixx#L161
Can you do something like this here?
| static constexpr uint32_t _PKG_HEADER_SIZE = 0x1000; | ||
| static constexpr uint32_t _PKG_MAGIC = 0x7F434E54; | ||
| static constexpr uint32_t _PKG_MAGIC_OFFSET = 0x00; | ||
| static constexpr uint32_t _PKG_CONTENT_ID_OFFSET = 0x40; | ||
| static constexpr uint32_t _PKG_CONTENT_ID_SIZE = 36; |
There was a problem hiding this comment.
I'd ask you to define a FileHeader structure, like here: https://www.psdevwiki.com/ps4/PKG_files and just use file_header.pkg_content_id in the above code.
In general I prefer the code to be clear to understand for somebody who wants to learn what is read and why is read.
2 participants
Reads the Content ID from the PKG headers found on the disc for PS4/PS5 system in
appdirectory that match provided filename and prints them as part ofredump info.Summary by CodeRabbit
app_sc.pkg) during its info output.