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

CDFS: DVD video support #130

Merged
merged 2 commits into from
Jun 28, 2020
Merged

Conversation

uyjulian
Copy link
Member

@uyjulian uyjulian commented Jun 23, 2020

Fixes support for DVD video discs.

@fjtrujy
Copy link
Member

fjtrujy commented Jun 24, 2020

Thanks @uyjulian for updating the cdfs support.
Why aren't you including the AllowDVDV as well in the PR?
I have been taking a look to the code of AllowDVDV the the irx generated is so small, maybe we should attach inside of the cdfs driver.

What do you think @rickgaiser ?
Thanks

@rickgaiser
Copy link
Member

Agreed, AllowDVDV is small and probably only usefull in combination with cdfs.
Likewise, this PR is only usefull in combination with AllowDVDV.

Is there a reason to keep it separated?

@TnA-Plastic
Copy link

TnA-Plastic commented Jun 24, 2020

I suppose he left it out, because it is up to the program to "decide" how to access DVD-based discs.

Edit: Actually just "discs", also CD-based types...

@fjtrujy
Copy link
Member

fjtrujy commented Jun 24, 2020

@TnA-Plastic you're right, let the developer choose, but offer the possibility in the SDK, right now with this PR we are doing changes in the SDK than are "not functional" at all unless you include AllowDVDV methods.

Thanks

@TnA-Plastic
Copy link

Yeah, well it was a change/PR to the disc-driver and FS-access and AllowDVDV is an own function/module.

@uyjulian
Copy link
Member Author

uyjulian commented Jun 24, 2020

I integrated AllowDVDV into the CDFS driver.

@uyjulian uyjulian force-pushed the cdfs_dvdv_support branch from 00d0e12 to 4372e76 Compare June 25, 2020 04:36
@sp193
Copy link
Member

sp193 commented Jun 25, 2020

AllowDVDV is used to unlock the DVD video playback function of the DVD drive. It's unlocked until something locks it. In the official flow, it probably gets unlocked when the DVD player is loaded - which is why no official software needed a hack like that.
FMCB will enable the DVD drive. So the only usecase for this - would be if you used a modchip or some disc (e.g. Swapmagic), to boot your software. (But if you're writing homebrew software and not using the DVD player with DVD video discs, why use discs?)

@uyjulian
Copy link
Member Author

An alternate entrypoint is used when FMCB is not used.

@AKuHAK
Copy link
Contributor

AKuHAK commented Jun 25, 2020

I integrated AllowDVDV into the CDFS driver.

Is it possible to comment out what each AllowDVD hex pattern means? For now, it is difficult to understand how does it work. Such documentation isn't needed for projects like wLe but for ps2sdk it is better to keep things documented.

@uyjulian
Copy link
Member Author

Is it possible to comment out what each AllowDVD hex pattern means?

Sure.

@rickgaiser
Copy link
Member

Unlike official sony software, homebrew does not know where it's loaded from and what modules are loaded. It's nice that FMCB unlocks the DVD, but it's probably also useless. An application that needs to use the DVD cannot assume it's being loaded from FMCB. So the application still needs to unlock the DVD to be sure it can be used.
The homebrew application should work not only from FMCB, but also from disc/fortuna/modchip/etc...

@sp193 you agree?

@TnA-Plastic
Copy link

TnA-Plastic commented Jun 26, 2020

At least for wLE as THE filemanager and app/ELF-Launcher, it is a "must have" IMO.

It's great to have the feature itself (implemented in the SDK), in any way!

Edit: SMS also had a kind of support, to access the files on a Video-DVD!

@sp193
Copy link
Member

sp193 commented Jun 27, 2020

It's up to you guys. Since the commit was already made and it isn't a change with detrimental effects.

As fjtrujy mentioned, the problem is with the purpose of this PR. I feel that it'll change what this module was meant for. I saw this module as originally meant for supporting new filesystems, not a means to support this hack. I thought that it would make more sense to use USB devices for homebrew, since they're easier to work with and are common. Unless one would like to build a DVD video player.
But in either case, it doesn't help if DVD video playback isn't enabled.

This commit seemed to be for a trick like ESR - the disc is a burned disc that appears to the MECHACON as a DVD Video Disc, but is not used for DVD Video playback.
Accesses to such a disc, must be done through sceCdReadDVDV instead of sceCdRead() - so even with AllowDVDV applied, one cannot use sceCdRead() on a burned disc. The AllowDVDV module unlocks DVD Video Disc reading - not reading of data DVD discs.

@uyjulian
Copy link
Member Author

The following is the main inspiration for this pull request: https://github.com/CTurt/FreeDVDBoot

@TnA-Plastic
Copy link

The news will be on psx-place.com later on today.
I'm quite busy right now, but I prepared some portions already.

@VersatileNinja
Copy link

It's already making the rounds today, saw it first on reddit. Can't wait to see how this evolves.

@uyjulian uyjulian force-pushed the cdfs_dvdv_support branch from b0e8b29 to dcf162e Compare June 28, 2020 01:05
@uyjulian
Copy link
Member Author

uyjulian commented Jun 28, 2020

Since FMCB and the recently released FreeDVDBoot allow DVD Video discs to be used, I removed the integrated AllowDVDV since it is redundant.

@sp193
Copy link
Member

sp193 commented Jun 28, 2020

Since that exploit is for the DVD player, I presume the DVD player must be first loaded. That process itself would also unlock DVD video playback.

@rickgaiser
Copy link
Member

So this PR without AllowDVDV is now good? It can be pulled?

@uyjulian
Copy link
Member Author

I've tested the DVD video disc functionality using uLaunchELF, so I can say that this PR works fine for me.

In the future, when CDVDMAN is reimplemented, I plan to remove this, so this is a workaround for now.

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.

7 participants