joncampbell123 / dosbox-x

DOSBox-X fork of the DOSBox project
GNU General Public License v2.0
2.71k stars 381 forks source link

Support for CHD format #1822

Closed whocares0101 closed 3 weeks ago

whocares0101 commented 4 years ago

Is your feature request related to a problem? Please describe. I just tried to mount a CHD file and it did not work also I didn't find any info if this is even supported.

Describe the solution you'd like Make it possible to mount CHD files with imgmount, at least cd/dvd images should work. A lot of emulators support CHD files already.

Additional context https://github.com/mamedev/mame https://github.com/mamedev/mame/blob/master/src/tools/chdman.cpp

standalone library https://github.com/rtissera/libchdr

eadmaster commented 3 years ago

See also this: https://github.com/dosbox-staging/dosbox-staging/issues/166

Wengier commented 3 years ago

I think it is certainly a reasonable suggestion. I will add it to Todo list.

whocares0101 commented 3 years ago

So I was playing around with this and got it working. This only needs a bit of code in cdrom.h and cdrom_image.cpp but I don't know how to properly add the static libraries so it works with all platforms and where to put the new libs. I had to add three static libraries for it to compile and work, FLAC.lib, lzma.lib, and chdr-static.lib.

Only tested this on windows with a debug sdl2 x64 build but I installed a few games and the cd audio was also working after some debugging. It would be great if someone else could do the rest.

This uses the standalone library https://github.com/rtissera/libchdr To compile libchdr on windows some small changes need to be made https://github.com/rtissera/libchdr/issues/39 Also note cmake used /MD instead of /MT which dosbox-x uses.

And here are the modified cdrom.h and cdrom_image.cpp files dos.zip

Wengier commented 3 years ago

@whocares0101 I am glad to hear that you made the effort to get it work! But yes, as you said your approach requires external libraries such as FLAC.lib, lzma.lib, and chdr-static.lib in order to compile, which only works for specific Windows Visual Studio builds.

I decided to further improve the code in my pull request so that external libraries are no longer required - all required source files are now built-in, and I have tested it on both Windows (Visual Studio, MinGW) and Linux and could confirm it works on these platforms. But I am sure further work can still be done regarding CHD support, such as support for parent images.

@dreamer and @kcgen I see there were discussions for CHD support in DOSBox Staging issue tracker too. In case you are interested, my commit for CHD image support in DOSBox-X is here: https://github.com/joncampbell123/dosbox-x/pull/1985/commits/4215f1027b6bd49e92952cd1213f1feea89eef15.

dreamer commented 3 years ago

@Wengier any reason why dr_flac.h (header-only library, that we use in src/lib/decoders/) was not appropriate here?

Wengier commented 3 years ago

@dreamer It appears that FLAC (instead of dr_flac.h) is a built-in component of the libchdr library (see here: https://github.com/rtissera/libchdr/tree/master/deps/flac-1.3.3), so it is needed and used by the librchdr library. But I have indeed thought of this too so that we could possibly make the libchdr libary work with dr_flac.h instead, but it may require some amount of work (haven't checked yet) of course.

Wengier commented 3 years ago

@kcgen If menu -> Drive image mount did not show CHD at all as you mentioned, then it simply means you were not using the code with CHD support. Right now the code with CHD support is still in https://github.com/wengier/dosbox-x, not the master branch of this repository. Please check out the code at https://github.com/wengier/dosbox-x instead.

kcgen commented 3 years ago

Right now the code with CHD support is still in https://github.com/wengier/dosbox-x

:facepalm: My bad -- thank you for the correct pointer Wengier!

whocares0101 commented 3 years ago

I had another idea to improve the loading speed by using a prefetch thread to offload sequential reads to another core. No idea how much this affects speed but it should help mitigate decompression times a bit. chd_thread_prefetch.zip

whocares0101 commented 3 years ago

I only tested raw cd images and 2048 images did not work. This just subtracts 16 from the read buffer offset to account for the missing sync header for 2048 sector images. Now both 2048/2352 images should work. chd_skip_sync_fix.zip

kcgen commented 3 years ago

I only tested raw cd images and 2048 images did not work.

Thanks for the heads up @whocares0101!

CHD's generated from 2048-byte sector ISO+CUEs are actually perhaps the more important of the two, because this opens up the generic scenario for users generating CHDs from a bog-standard directory (using the above mkisofs as a middleman) - inside of which they can contain further CDROM files (such as ISO+FLAC/Opus) which DOSBox can mount. So it's fantastic to hear you've added this.

Where as CHDs from 2352-byte sector BIN+CUEs can only be strictly created from CDROM dumps, as there is no easy way to convert a directory into a BIN+CUE pair; well, besides burning the directory to a physical CD and then dumping it back to raw BIN+CUE.

I would like to hold off on testing further. @Wengier , I'm guessing you'll get these in when time permits. I'll keep an eye out and be ready to test various scenarios when ready!

@whocares0101, the read-ahead is indeed a good idea (although I'm biased :slightly_smiling_face:). This is currently in the queue for libchd: https://github.com/rtissera/libchdr/issues/34, as is an LRU cache: https://github.com/rtissera/libchdr/issues/36

The libchd team is very cooperative (hey, maybe you want to add improvements directly to it :wink:), and ideally those features would land there - so all applications using libchd will benefit from these performance improvements.

Wengier commented 3 years ago

@whocares0101 Thanks for the update! Yes, as @kcgen said they are certainly good improvements. I have already incorporated the changes here: https://github.com/joncampbell123/dosbox-x/pull/1985/commits/50b098cad0873751d731bdb586d91e4f8d331ac5. I did not get the time to do additional testing yet, but I have checked the files I created earlier and they indeed worked fine. Please keep us informed when you make further progress. Thanks!

kcgen commented 3 years ago

Tiny build fix.. 2020-11-12_07-20

Problem is the flac directory on disk is capitalized:

2020-11-12_07-22

libchdr #includes from an uppercase FLAC directory, so suggest we follow that in cdrom_image.cpp.

kcgen commented 3 years ago

Things are working mostly as expected:

Mode 1/2352 Test

I tested a basic BIN/CUE pair (Jones in the Fast Lane) consisting of a data track and one audio track:

FILE "jones.bin" BINARY
  TRACK 01 MODE1/2352
    INDEX 01 00:00:00
  TRACK 02 AUDIO
    FLAGS DCP
    PREGAP 00:02:00
    INDEX 01 02:47:53

Converted that to CHD using: chdman createcd -i jones.cue -o jones.chd. Tip: install mame-tools if you don't have chdman.

2020-11-11_13-44

The pregap isn't handled on the DOSBox side (in the above screenshot, note that tracks 2 includes 150 seconds of pregap).

The error manifests in seeks being positioned two frames (2 * 75 seconds) further ahead than they should be. So in this case, the voice CD-DA sections jump into the middle of their sentences. The immediate work-around is to knockout the pregap line in the CUE before feeding it to CHDMAN (note PREGAP:0 now):

Metadata:     Tag='CHT2'  Index=1  Length=89 bytes
              TRACK:2 TYPE:AUDIO SUBTYPE:NONE FRAMES:165952 PREGAP:0 PGTYPE:MODE1 PGSUB:NONE POSTGAP:0.

The CUE and pregap handling is in cdrom_image.cpp. Ideally you can re-use the TrackAdd(..) function and read the pregap values from the CHD library and mimic the same behavior as the current code does when it reads a CUE file.

Basically any pregap values "push" the position of the audio track (and all subsequent audio tracks) outward in time.

MODE1/2048 Test

I then tried creating a non-CD-DA ISO image from a local directory:

2020-11-11_13-53

And then to an ISO: mkisofs -lJR -o test.iso test with CUE file:

FILE "test.iso" BINARY
  TRACK 01 MODE1/2048
    INDEX 01 00:00:00

And then to CHD: chdman createcd -i test.cue -o test.chd

2020-11-11_13-43

Mounts clean and binary data can be read; however I couldn't subsequently perform an imgmount using the cdrom.cue I had stored inside the CHD.

Nice to haves

Ideally we could use CHD to store content for further mounting, such as for C:\ and then CDROM image/iso/audio track files. So a full DOS game could be represented inside a single CHD file.

However, right now using mount or imgmount to mount files stored inside the CHD directory doesn't work. I'm not sure what it would take to get that working.

Wengier commented 3 years ago

@kcgen Thanks for the testing! I am glad to hear that things are working mostly as expected. And yes, performing an imgmount with an image inside an image is indeed nice to have as you said. In fact if this can be supported maybe it can be extended to other types of disk images as well.

Also, you are certainly right that the subdirectoy is an uppercased one (FLAC) instead of lowercase (flac). I have already updated the include in cdrom_image.cpp accordingly. Thanks for pointing this out!

Torinde commented 1 year ago

@rderooy , should CHD support be mentioned in the wiki?

rderooy commented 1 year ago

Was this ever merged?

Torinde commented 1 year ago

Not sure, but it's mentioned at https://github.com/joncampbell123/dosbox-x/blob/0a82937e03e3244ab0f927b791260a3cefcf82d4/CREDITS.md

rderooy commented 1 year ago

I can confirm it worked. I installed 'mame-tools' from my Linux distro package manager to get 'chdman'. Then I converted a Cue/Bin set to CHD.

$ chdman createcd -i Monkey\ Island\ Madness\ \(USA\).cue -o test.chd
chdman - MAME Compressed Hunks of Data (CHD) manager 0.251 (unknown)
Output CHD:   test.chd
Input file:   Monkey Island Madness (USA).cue
Input tracks: 25
Input length: 59:48:56
Compression:  cdlz (CD LZMA), cdzl (CD Deflate), cdfl (CD FLAC)
Logical size: 659,001,600
Compression complete ... final ratio = 44.9%

A nice reduction in filesize...

Then I mounted the CHD in DOSBox-X as usual.

Z:\>imgmount d test.chd
MSCDEX installed.
Drive D is mounted as test.chd
rderooy commented 1 year ago

I updated the draft wiki to mention CHD image support.

Too bad CHD is so niche, otherwise it would be cool to convert my cue/bin collection to them.

Torinde commented 1 year ago

I don't know where to see the draft wiki - will it appear published at release time?

rderooy commented 1 year ago

The draft is located here: https://github.com/Wengier/dosbox-x-wiki/wiki/Guide%3AManaging-image-files-in-DOSBox%E2%80%90X

Since it also contains some other updates for VHD and QCOW2, which is yet to be merged, I will ask for it to be pulled with the next release.

rderooy commented 1 year ago

Also, I think this issue can be closed...

Torinde commented 1 year ago

Are HDDs also supported in CHD format - or only CD/DVD?