libchdr icon indicating copy to clipboard operation
libchdr copied to clipboard

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

Open snickerbockers opened this issue 3 years ago • 8 comments
trafficstars

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.

snickerbockers avatar May 23 '22 05:05 snickerbockers

Hey, look good, two questions here

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

rtissera avatar May 23 '22 07:05 rtissera

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

snickerbockers avatar May 25 '22 04:05 snickerbockers

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

rtissera avatar Aug 01 '22 17:08 rtissera

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 avatar Aug 01 '22 19:08 snickerbockers

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.

snickerbockers avatar Aug 02 '22 04:08 snickerbockers

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

rtissera avatar Aug 02 '22 20:08 rtissera

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

snickerbockers avatar Aug 03 '22 06:08 snickerbockers

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

snickerbockers avatar Aug 04 '22 03:08 snickerbockers

Let's merge. Thanks the coordinated work guys.

rtissera avatar Aug 04 '22 11:08 rtissera