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

fix: read_physical_structures failing for multi-layer BDs. #175

Merged
merged 1 commit into from
Jul 31, 2024

Conversation

Didrole
Copy link
Contributor

@Didrole Didrole commented Jul 30, 2024

Proposal fix for issue #174

@superg
Copy link
Owner

superg commented Jul 30, 2024

I don't understand what you did here, it's a relatively massive code change that might affect DVD dumping (while we're fixing BluRay dumping) and taking into account that the ticket you created specifies the wrong root cause I would not rather spend time reviewing it.

I assume there is an issue with incorrectly calculated layerbreak for the disc you're trying to dump. Please create a ticket with the detail information on incorrect layerbreak value and what do you think is correct so we could establish why the calculation is wrong.

@superg superg closed this Jul 30, 2024
@Didrole
Copy link
Contributor Author

Didrole commented Jul 30, 2024

I understand that the diff is a bit messy looking and I gave no explanation.
I basically moved all the BD specific logic out of the layers_count loop to clearly differentiate BD logic from DVD logic.

A simple break statement to stop the layers_count loop may be another solution (but IMO less readable).

@superg
Copy link
Owner

superg commented Jul 30, 2024

Let me get back to my PC tonight and I will review everything again. Just want to make sure I don't introduce any regressions while I took a break from active redumper development.

@superg superg reopened this Jul 30, 2024
@Deterous
Copy link
Contributor

I have tested this and confirmed it also fixes #153

@superg
Copy link
Owner

superg commented Jul 31, 2024

Ok, I pulled it locally and this is very good. I agree that it makes sense to have logic split between bluray and dvd even if there is some code duplication.

@superg superg merged commit adde32b into superg:main Jul 31, 2024
6 checks passed
@JohnVeness
Copy link

Many thanks for working together on this - I can now dump Xbox One games (which are almost all 2 layer) which were previously failing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants