CHD support through libchdr #18198
CHD support through libchdr #18198
Conversation
|
Okay, one build error left.. Weird: I guess that's a newer libc function, but why would that build, aandroid armeabi-v7a, be so special.... EDIT: my hack below didn't work for some reason.. oh oops, need it in Android.mk as well. |
hrydgard
changed the title
hrydgard
force-pushed
the
libchdr-support
branch
from
48ba14e to
d5277f0
Compare
|
I spotted some stuff to clean up/fix in the UWP files, I'll make a PR for those tweaks once this is merged. Unless you want to fix these quirks right now. |
|
@GABO1423 I won't be merging this PR for a little while (until I'm happy with the stability of 1.16.x), so feel free to do a PR against this PR. |
| @@ -4,11 +4,45 @@ SRC := ../.. | |||
| include $(CLEAR_VARS) | |||
| include $(LOCAL_PATH)/Locals.mk | |||
|
|
|||
| LOCAL_CFLAGS += -D_7ZIP_ST -D__SWITCH__ | |||
There was a problem hiding this comment.
__SWITCH__?
-[Unknown]
There was a problem hiding this comment.
It only makes sense if you go look at the c file in question, CpuArch in lzma. It avoids using the getauxflags function on Switch and Vita to check for NEON, and that workaround for the lack of the function works fine for us.
There was a problem hiding this comment.
Heh, a bit gross, but got it. We don't get getauxval on some Android archs because we're still supporting older targets iirc. cpu_features had this issue too but I think had a cleaner check.
-[Unknown]
| Path paths[8]; | ||
| paths[0] = fileLoader->GetPath(); | ||
|
|
||
| chd_error err = chd_read_header(paths[0].c_str(), &childHeader); |
There was a problem hiding this comment.
Doesn't this mean it won't work on Android? Is this function even needed? Later it opens it from a FILE * anyway, which would probably also solve the international path thing too?
Of course it won't support read ISO into RAM or remote disc streaming, but I guess people using this format mostly want the hype and will probably feel like it's more powerful because stuff doesn't work.
-[Unknown]
There was a problem hiding this comment.
That is absolutely true, but as commented elsewhere, I haven't enabled this path yet.
There was a problem hiding this comment.
This is at the very beginning of the function. It's not a commented out part or after that part.
-[Unknown]
There was a problem hiding this comment.
Oh duh, was thinking of the below block, too lazy to click into the full view..
Yeah, this is bad. I just realize I only tested it on an old Android.
So this will need fixing for modern Android, and I'll probably end up submitting a patch to libchdr to add chd_read_header_file.
There was a problem hiding this comment.
Does it even need this though? It seems to just open from a file and actually get the header from there later?
-[Unknown]
There was a problem hiding this comment.
True that, I'll look into getting rid of it before merge.
Hack around build error port the last fix to Android.mk One more attempt Warning fixes Oops, forgot a slash
hrydgard
force-pushed
the
libchdr-support
branch
from
7924bea to
2f5f9df
Compare
|
@hrydgard To be honest is unfortunate you merged this at all. |
Why? |
|
@deltabeard the discussion, which should probably not be had again at this point, is available for you to read here: #13810 |
|
To sum it up, I figured out that we could do this without bloating things as much as the original PR did, which changed the equation from absolutely not worth it, to just barely worth doing to stop all the recurring complaints about the lack of support that I kept getting through multiple channels. So I did for my own long term benefit, even though the feature on its own really isn't worth a lot when we already have CSO. End of discussion. Further posts will be deleted. |
Yes, I'm adding basic CHD support. You asked for it, you got it - but I still recommend CSO! I'm just tired of the complaints, and it's just not that hard to do.
Basically a lean and mean replacement for the bloated 150000-line #13810 , which I couldn't merge as-is.
Uses libchdr through a submodule and unlike the previous PR doesn't add a lot of other unnecessary libraries - flac is entirely useless for example since UMDs don't have audio tracks.
I've disabled the parent/child thing due to file system performance concerns on Android and libchdr missing the functionality to just scan file headers from a FILE *, which we require. See rtissera/libchdr#55 for more details. I'm not sure how desirable that feature is for PSP games anyway.
Fixes #10417
The build system setup isn't finished yet, still need to add the new files to Android.mk and the libretro makefile.