Raw BD read by Deterous · Pull Request #355 · superg/redumper

Deterous · 2026-02-23T06:53:37Z

Summary by CodeRabbit

Release Notes

New Features

Added Blu-ray disc support for reading, dumping, and extracting disc images
Introduced --bd-raw command-line option for Blu-ray sector dumping operations
Added Blu-ray data descrambling capability for disc processing

Chores

Updated build configuration to include new disc handling modules
Extended disc format detection to recognize Blu-ray image files

coderabbitai · 2026-02-23T06:53:50Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

📝 Walkthrough

Walkthrough

This PR introduces Blu-ray disc support to redumper by adding BD-specific data structures, a descrambling algorithm, and integrated raw read and extraction workflows alongside existing DVD functionality.

Changes

Cohort / File(s)	Summary
Build Configuration `CMakeLists.txt`	Added `dvd/bd.ixx` and `dvd/bd_scrambler.ixx` to the redumper target sources PUBLIC FILES section.
Blu-ray Core Definitions `dvd/bd.ixx`, `dvd/bd_scrambler.ixx`	Introduced new C++20 modules defining BD namespace with constants (`LBA_START`, `LBA_IZ`, `ECC_FRAMES`), data structures (`DataFrame`, `OmniDriveDataFrame`), and a `Scrambler` class implementing ISO/IEC 30190-based descrambling with EDC validation.
Blu-ray Dump Handling `dvd/dvd_dump.ixx`	Extended raw read logic to handle Blu-ray: added BD imports, updated `dump_get_config` to return `.sbram` for BD discs, implemented `read_dvd_sectors` Blu-ray branch with frame descrambling and EDC error handling, adjusted `dump_read_size` logic for BD (size 7 vs. 31 for DVD).
Blu-ray Extraction `dvd/dvd_split.ixx`	Renamed `extract_iso` to `dvd_extract_iso` with explicit sdram path parameter, added new `bd_extract_iso` function for processing sbram files via BD scrambler, updated `redumper_split_dvd` to dispatch between DVD and BD extraction paths based on file existence.
Options and Integration `options.ixx`, `cd/protection.ixx`, `redumper.ixx`	Added `--bd-raw` option to Options struct; updated file suffix checks in `redumper_split` and `redumper_protection` to recognize `.sbram` alongside `.iso` and `.sdram`.

Sequence Diagram(s)

sequenceDiagram
    participant Reader as Disc Reader
    participant Dumper as DVD Dumper
    participant Scrambler as BD Scrambler
    participant EDC as EDC Validator
    participant Output as Output Buffer

    Reader->>Dumper: read_dvd_sectors(BD disc, raw mode)
    Dumper->>Reader: read OmniDriveDataFrame
    Reader-->>Dumper: frame data
    Dumper->>Scrambler: descramble(df, psn)
    Scrambler->>Scrambler: process(data span, psn)
    Scrambler->>EDC: validate EDC
    EDC-->>Scrambler: match/mismatch
    alt EDC Valid
        Scrambler-->>Dumper: return true
    else EDC Invalid
        Scrambler->>Scrambler: revert via process()
        Scrambler-->>Dumper: return false, mark error
    end
    Dumper->>Output: copy decrypted frame data

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

descramble refactored, verify descrambling on dump, disable cache rea… #354: Modifies DVD descrambling and raw-read logic with updated scrambler usage and descramble flows.
Better default for raw DVD dump read size #351: Adjusts raw dump read-size handling in dvd_dump.ixx and options parsing.
OmniDrive raw dvd fixes #348: Extends raw dumping logic modifications in dvd_dump.ixx for sectors-per-read behavior.

Suggested reviewers

superg

🐰 Blu-ray discs come dancing through,
With frames descrambled, shiny and new!
EDC checks sparkle, data rings true,
A carrot-load of features for me and for you! 🥕✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Raw BD read' directly summarizes the main change: adding raw Blu-ray (BD) read support across multiple files including BD data structures, scrambler implementation, and DVD dump/split modules.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

🧹 Nitpick comments (1)

dvd/bd_scrambler.ixx (1)

34-34: Operator precedence is correct but fragile-looking — consider adding parentheses for clarity.

The expression relies on & binding tighter than ^ binding tighter than |, which is correct in C++. However, the chain of |, ^, and & without explicit grouping is easy to misread during future maintenance.

♻️ Suggested clarification

-            shift_register = (shift_register << 1) | (shift_register >> 15 & 1) ^ (shift_register >> 14 & 1) ^ (shift_register >> 12 & 1) ^ (shift_register >> 3 & 1);
+            shift_register = (shift_register << 1) | ((shift_register >> 15 & 1) ^ (shift_register >> 14 & 1) ^ (shift_register >> 12 & 1) ^ (shift_register >> 3 & 1));

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@dvd/bd_scrambler.ixx` at line 34, The bitwise expression assigning to
shift_register is correct but hard to read; update the assignment in the
scrambler where shift_register is computed so each operation is explicitly
grouped: parenthesize each (shift_register >> N & 1) term and then group the
XORs together (e.g. ((t1) ^ (t2) ^ (t3) ^ (t4))) before ORing with
(shift_register << 1), so the final statement makes the intended precedence
obvious while preserving existing behaviour.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@dvd/bd_scrambler.ixx`:
- Line 34: The bitwise expression assigning to shift_register is correct but
hard to read; update the assignment in the scrambler where shift_register is
computed so each operation is explicitly grouped: parenthesize each
(shift_register >> N & 1) term and then group the XORs together (e.g. ((t1) ^
(t2) ^ (t3) ^ (t4))) before ORing with (shift_register << 1), so the final
statement makes the intended precedence obvious while preserving existing
behaviour.

superg · 2026-02-23T13:56:27Z

@coderabbitai stop suggesting brackets for operations where operator precedence is correct.

coderabbitai · 2026-02-23T13:56:50Z

Tip

For best results, initiate chat on the files or code changes.

@superg, understood. I'll no longer suggest adding brackets or parentheses when the operator precedence is already correct and produces the intended result.

✏️ Learnings added

Learnt from: superg
URL: https://github.com/superg/redumper/pull/355

Timestamp: 2026-02-23T13:56:49.301Z
Learning: Do not suggest adding brackets or parentheses for operations where operator precedence is already correct in the superg/redumper repository.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

superg

Looks ok.

There is some code duplication but please just address what I asked and let's get that merged and I'll deduplicate later.

superg · 2026-02-24T01:58:10Z

dvd/bd.ixx

+namespace gpsxre
+{
+
+export constexpr int32_t BD_LBA_START = -0x100000;
+export constexpr int32_t BD_LBA_IZ = -0x2000;
+export constexpr uint32_t BD_ECC_FRAMES = 0x20;


let's namespace it to gpsxre::bd, strip BD_ prefix from constants and use it as bd::LBA_START etc, will look more tidy.

There is already CD_LBA_START and DVD_LBA_START, do you want me to prefix the DVD one too, dvd::LBA_START

Just do bd:: and I'll handle dvd and cd separately.

superg · 2026-02-24T02:02:13Z

dvd/bd_scrambler.ixx

+
+namespace gpsxre::bd
+{
+


I'd prefer to have it as a class with process in private: section
scrambler.descramble() looks more consistant with already existing CD/DVD code vs just bd::descramble().

superg · 2026-02-24T02:06:28Z

options.ixx

                    else if(key == "--dvd-raw")
                        dvd_raw = true;
+                    else if(key == "--bd-raw")
+                        dvd_raw = true;


I understand the intent here but let's make another variable bd_raw.

It's not immediately clear for the user to use --dvd-raw but get bluray raw if disc is bluray.

superg · 2026-02-24T02:13:23Z

dvd/dvd_split.ixx

+    bd_extract_iso(ctx, options);
+    dvd_extract_iso(ctx, options);


Flow is not obvious here.

Can you change:
void bd_extract_iso(Context &ctx, Options &options)
to:
void bd_extract_iso(Context &ctx, std::string image_path, Options &options)

and:
void dvd_extract_iso(Context &ctx, Options &options)
to:
void dvd_extract_iso(Context &ctx, std::string image_path, Options &options)

and move out decision logic on this level?

Something like:

auto image_prefix = (std::filesystem::path(options.image_path) / options.image_name).string(); std::filesystem::path sdram_path(image_prefix + ".sdram"); std::filesystem::path sbram_path(image_prefix + ".sbram"); if(std::filesystem::exists(sdram_path) dvd_extract_iso(ctx, sdram_path, options); else if(std::filesystem::exists(sbram_path) bd_extract_iso(ctx, sbram_path, options);

(and remove those checks from dvd_extract_iso and bd_extract_iso)

superg · 2026-02-24T02:19:37Z

dvd/dvd_dump.ixx

@@ -506,6 +508,10 @@ DumpConfig dump_get_config(DiscType disc_type, bool raw)
    {
        config = DumpConfig{ ".sdram", sizeof(RecordingFrame), DVD_LBA_START, DVD_LBA_RCZ - (int32_t)OVERREAD_COUNT, 0 };
    }
+    else if((disc_type == DiscType::BLURAY || disc_type == DiscType::BLURAY_R) && raw)


if(raw) { if(disc_type == DiscType::DVD) // ... else if(disc_type == DiscType::BLURAY || disc_type == DiscType::BLURAY_R) // ... }

superg · 2026-02-24T02:21:32Z

dvd/dvd_dump.ixx

Same as previous comment, add if(raw) above.

Deterous · 2026-02-25T01:25:55Z

Thanks - I'm aware of the code duplication in dvd_split but it would lead to a much harder piece of code to review, and I prefer you do that part.

Deterous · 2026-02-25T12:36:56Z

@CodeRabbit review

coderabbitai · 2026-02-25T12:41:46Z

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@dvd/dvd_dump.ixx`:
- Around line 895-899: The raw-mode flag is currently enabled if either
options.dvd_raw or options.bd_raw is set regardless of the loaded media; change
the logic that sets raw so it only enables when the option matches the current
disc type and omnidrive_firmware is present (e.g., set raw = options.dvd_raw &&
is_dvd_disc && omnidrive_firmware OR raw = options.bd_raw && is_bd_disc &&
omnidrive_firmware), and update the warning checks so they only warn when the
user requested a raw mode that does not match the loaded media (use the
disc-type predicate used elsewhere in this module such as is_bd_disc/is_dvd_disc
or the existing disc_type variable while keeping the existing omnidrive_firmware
condition).

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7e0d9c0 and fbbd872.

📒 Files selected for processing (5)

dvd/bd.ixx
dvd/bd_scrambler.ixx
dvd/dvd_dump.ixx
dvd/dvd_split.ixx
options.ixx

🚧 Files skipped from review as they are similar to previous changes (2)

options.ixx
dvd/bd_scrambler.ixx

coderabbitai · 2026-02-25T12:46:47Z

dvd/dvd_dump.ixx

+    bool raw = (options.dvd_raw || options.bd_raw || nintendo_key) && omnidrive_firmware;
    if(options.dvd_raw && !omnidrive_firmware)
        LOG("warning: drive not compatible with raw DVD dumping");
+    if(options.bd_raw && !omnidrive_firmware)
+        LOG("warning: drive not compatible with raw BD dumping");


⚠️ Potential issue | 🟠 Major

Scope raw-mode enablement by current disc type.

Line 895 currently enables raw when either options.dvd_raw or options.bd_raw is set, regardless of the loaded media type. That can trigger the wrong raw path/extension unexpectedly (e.g., dvd_raw on BD).

💡 Proposed fix

- bool raw = (options.dvd_raw || options.bd_raw || nintendo_key) && omnidrive_firmware; - if(options.dvd_raw && !omnidrive_firmware) + bool dvd_raw_requested = (ctx.disc_type == DiscType::DVD) && options.dvd_raw; + bool bd_raw_requested = (ctx.disc_type == DiscType::BLURAY || ctx.disc_type == DiscType::BLURAY_R) && options.bd_raw; + bool raw = (dvd_raw_requested || bd_raw_requested || nintendo_key) && omnidrive_firmware; + if(dvd_raw_requested && !omnidrive_firmware) LOG("warning: drive not compatible with raw DVD dumping"); - if(options.bd_raw && !omnidrive_firmware) + if(bd_raw_requested && !omnidrive_firmware) LOG("warning: drive not compatible with raw BD dumping");

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@dvd/dvd_dump.ixx` around lines 895 - 899, The raw-mode flag is currently enabled if either options.dvd_raw or options.bd_raw is set regardless of the loaded media; change the logic that sets raw so it only enables when the option matches the current disc type and omnidrive_firmware is present (e.g., set raw = options.dvd_raw && is_dvd_disc && omnidrive_firmware OR raw = options.bd_raw && is_bd_disc && omnidrive_firmware), and update the warning checks so they only warn when the user requested a raw mode that does not match the loaded media (use the disc-type predicate used elsewhere in this module such as is_bd_disc/is_dvd_disc or the existing disc_type variable while keeping the existing omnidrive_firmware condition).

* Raw BD read (#355) * Raw BD read * code review * fix * fix build * Fix bd scrambler * currently only works with dump size 1 * OmniDrive bluray data frame is 2070 bytes --------- Co-authored-by: Hennadiy Brych <gennadiy.brich@gmail.com> * compilation fixes * nintendo book type * structure padding fix * bluray support refactored, split progress output * raw by type, CR review addressed * CR review --------- Co-authored-by: Deterous <138427222+Deterous@users.noreply.github.com>

Raw BD read

7e0d9c0

coderabbitai bot reviewed Feb 23, 2026

View reviewed changes

Deterous marked this pull request as draft February 23, 2026 07:16

superg requested changes Feb 24, 2026

View reviewed changes

Deterous added 5 commits February 25, 2026 11:11

code review

b3f0f2e

fix

90e4eec

fix build

2ed8c76

Fix bd scrambler

0cae9ee

currently only works with dump size 1

fbbd872

coderabbitai bot reviewed Feb 25, 2026

View reviewed changes

OmniDrive bluray data frame is 2070 bytes

e0f3f73

superg changed the base branch from main to bluray March 1, 2026 21:09

superg marked this pull request as ready for review March 1, 2026 21:10

Merge branch 'bluray' into raw-bd

98fcae4

superg merged commit 7621463 into superg:bluray Mar 1, 2026
3 of 11 checks passed

coderabbitai bot mentioned this pull request Mar 1, 2026

Bluray #360

Merged

Deterous deleted the raw-bd branch March 1, 2026 23:27

coderabbitai bot mentioned this pull request Mar 8, 2026

Dvdraw edc #363

Merged

Conversation

Deterous commented Feb 23, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Release Notes

Uh oh!

coderabbitai bot commented Feb 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested reviewers

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

superg commented Feb 23, 2026

Uh oh!

coderabbitai bot commented Feb 23, 2026

Uh oh!

superg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Deterous commented Feb 25, 2026

Uh oh!

Deterous commented Feb 25, 2026

Uh oh!

coderabbitai bot commented Feb 25, 2026

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Deterous commented Feb 23, 2026 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Feb 23, 2026 •

edited

Loading