rtissera / libchdr

Standalone library for reading MAME's CHDv1-v5 formats.
BSD 3-Clause "New" or "Revised" License
98 stars 42 forks source link

use virtual file i/o API which defaults to a stdio wrapper #78

Closed snickerbockers closed 2 years ago

snickerbockers commented 2 years ago

This PR implements a virtual file I/O layer that goes between libchdr and stdio. Users of libchdr can optionally implement their own file I/O functions instead of using the stdio ones so that they can load files from more sources than just simple files.

For example, users can create core_file implementations that load files over a network connection, or from RAM, from an archive, etc.

Another benefit is that in the future users which require platform-specific behavior will be able to implement it themselves instead of making changes to libchdr.

For the sake of backwards-compatibility, chd_open_file now accepts a FILE instead of a core_file. Users wishing to provide their own I/O function implementations may do so by calling the new chd_open_core_file function.

rtissera commented 2 years ago

Hey, look good, two questions here

  1. Has it been checked to compile under Windows / macOS / Linux ?
  2. How does it play with libretro ?
snickerbockers commented 2 years ago

Hey, look good, two questions here

1. Has it been checked to compile under Windows / macOS / Linux ?

I tested it under Windows and Linux and found no problems. I can't test macOS because I don't have one.

This testing was done on WashingtonDC's new chd branch, which uses the new function-pointer API (chd_open_core_file) introduced in this PR. I also tested WashingtonDC using the pre-existing chd_open_file(FILE*, ... and chd_open(char const*,... APIs and observed that they still work.

2. How does it play with libretro ?

I downloaded flycast and copied sources from this PR's branch into its copy of libchdr. It was able to compile and play .chd games with no problems.

chd_open_file still accepts a FILE* so there shouldn't be any incompatibilities. emulators that want to use the new function pointer API to provide their own read/write/size/close implementations do so by calling a new function, which is chd_open_core_file. This way the new changes won't interfere with anybody who doesn't want to use them.

Thanks, snickerbockers

rtissera commented 2 years ago

@snickerbockers can you rebase following another PR merge and I will merge ? This issue seems to be a blocker for chd-rs project.

snickerbockers commented 2 years ago

i just did that and tested to make sure it still works (it does). merge should be able to fast-forward now.

thanks, snickerbockers ------- Original Message ------- On Monday, August 1st, 2022 at 1:49 PM, Romain TISSERAND @.***> wrote:

@.***(https://github.com/snickerbockers) can you rebase following another PR merge and I will merge ? This issue seems to be a blocker for chd-rs project.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

snickerbockers commented 2 years ago

thanks for your feedback @PatrickvL. I've pushed out a new commit that implements every change you requested.

thanks, snickerbockers

"PatrickvL" @.***> writes:

@PatrickvL requested changes on this pull request.

Since review remarks

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.

rtissera commented 2 years ago

@PatrickvL @snickerbockers good work, let's fix the couple inconsistencies and I will merge so we can move on

snickerbockers commented 2 years ago

I've fixed all the inconsistencies mentioned except for the one i disagreed with (see my other comment). Thanks again for feedback @PatrickvL.

snickerbockers commented 2 years ago

i believe its ready for merge now as long as nobody else has any objections or criticism

rtissera commented 2 years ago

Let's merge. Thanks the coordinated work guys.