Backport changes from my ClownCD fork.#143
Backport changes from my ClownCD fork.#143Clownacy wants to merge 53 commits intortissera:masterfrom
Conversation
For some reason, the libretro fork has this, but upstream does not!
This new one avoids the need for memory allocation, streamlines some
library internals, and provides a simpler interface for the user:
Old API:
```c
core_file* const file_callbacks = (core_file*)malloc(sizeof(core_file));
if (file_callbacks == NULL)
{
return NULL;
}
else
{
chd_file *file;
file_callbacks->argp = &disc->file;
file_callbacks->fsize = ClownCD_Disc_CHDFileSize;
file_callbacks->fread = ClownCD_Disc_CHDFileRead;
file_callbacks->fclose = ClownCD_Disc_CHDFileClose;
file_callbacks->fseek = ClownCD_Disc_CHDFileSeek;
chd_open_core_file(file_callbacks, CHD_OPEN_READ, NULL, &file);
return file;
}
```
New API:
```c
static const core_file_callbacks file_callbacks = {
ClownCD_Disc_CHDFileSize,
ClownCD_Disc_CHDFileRead,
ClownCD_Disc_CHDFileClose,
ClownCD_Disc_CHDFileSeek,
};
chd_file *file;
chd_open_core_file_callbacks(&file_callbacks, &disc->file, CHD_OPEN_READ, NULL, &file);
return file;
```
The old API has been preserved for backwards-compatibility; since
libchdr is packaged by Debian, I figured that API-breaking changes
would be undesirable.
Not a standard C type, and it conflicts with the rest of the code anyway: it only gets set to, and compared to, regular `size_t`!
Could cause build errors or other defects. https://stackoverflow.com/questions/69644782/extern-c-before-or-after-library-header-includes
Nothing ever actually caches a chunk.
Also did some tidying, while I was at it.
Applied applied the 'FALSE' constant a few times.
The call operator can be used on a function pointer as-is.
Should shut-up some warnings when compiling as ANSI C.
- Does not erroneously refer to frames as samples. - Correctly allocates a sector's worth of samples. - No longer defines a one-word macro that could cause a name collision in unity builds.
`strncmp` is redundant, and the compiler complains about `rawheader` not being guaranteed to be null-terminated.
This has better code locality, making it simpler to add new versions in the future, and exposing the bug that the `flags` field is filled with garbage when parsing a v5 header. That has now been fixed.
The reader already performs validation, and even returns different error codes compared to the validator, so it is best if they are merged.
They report failure for a reason, so we may as well act on it.
Now there's just a nice little function for doing that.
This should prevent a read-error being reported when reading a non-v5 CHD file with zero hunks, as it will not accidentally read past the end of the header data.
Less complexity; more portability. Also much easier to perform a unity build with. There is a slight difference in licensing (zlib -> MIT), but that should not be a problem.
As with miniz, this version of the library is simpler, making it more portable, and also easier to perform a unity build with.
It is easier to maintain code when you don't have to figure which parts aren't used at all.
Forced include directories are incompatible with unity builds.
Unity builds cannot use forced include directories.
The only truly-portable build system!
Some libretro platforms do not support the `inline` keyword, so using a macro makes it possible to work-around.
Static builds of libretro cores link against a static build of RetroArch, which includes its own copy of LZMA, causing error due to the same symbols being defined twice. To work around this, use macro magic to namespace our copy of LZMA. This is not too unusual: the SDL library does the same thing for its copy of libusb: libsdl-org/SDL@08b2176 It may be worth doing this to miniz and zstd as well.
Having everything tangled in a single file makes for bad code; keeping them separate allows for encapsulation. This will also make it easier to allow switching dr_flac for libFLAC, if it ever comes to that.
It belongs there.
All of them (besides CDFL) are identical aside from which decompression functions are used. The vast majority of code can be punted to a shared function.
|
Ouch, this is big.... but very good work. |
|
@Clownacy Also the question is how "bad" it will break current libretro integrations ? |
|
The CI seems to be failing because of a timeout when fetching Backwards-CompatibilityThis update should be mostly backwards-compatible with existing projects, though there are some edge-cases where it is not: Libretro cores tend to bypass libchdr's build system and compile its C files manually. Cores that do this will now encounter linker errors due to libchdr having additional C files which the core needs to be updated to reference. If these projects relied on libchdr's CMake script instead, then this would not have been a problem. For projects that cannot use CMake, the Projects which rely on The new file IO API implements the old API as a layer atop the new one, maintaining backwards-compatibility. There is one slight change in behaviour though: if a file were opened with
Other than these edge-cases, this update should not cause any breakage for libretro cores. SwanStation TestAs a test, I modified a copy of SwanStation to use this version of libchdr, and, while there were a couple of issues that needed sorting, it was mostly a smooth upgrade. The issues that I encountered were...
Once these changes were made, SwanStation compiled and ran correctly. |
SwanStation defines its own `lzma` target, which lacks the namespacing that libchdr's copy has, so linking against it will result in linker errors. To avoid this, give libchdr's copy a different name.
…RIFY_BLOCK_CRC`. Better than forcing the user to modify a libchdr header. It may be worth namespacing these to prevent name collisions with user code.
2 participants
This makes libchdr portable enough to compile for at least 32 libretro platforms. I've also cleaned the code up a bit, addressing a few bugs which I found along the way.
libchdr can now be compiled in a unity build, which makes the library as easy to integrate into a project as a single-file library. This is useful for projects which do not use CMake and cannot rely on pkg-config, such as libretro cores.
This contains my new file IO API, so it makes #142 redundant.