joncampbell123 / dosbox-x

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

Assess DOSBox GUS DMA implementation vs -X #1961

Closed kcgen closed 4 years ago

kcgen commented 4 years ago

Dosbox SVN commit 4387 added DMA transfer handling to GUS specifically to support Quake.

https://sourceforge.net/p/dosbox/code-0/4387/#diff-2

This does not include the more complex timing or rate-control mechanism performed by the -X implementation.

I don't know which solution is more correct, as I don't have a specification descring the GUS DMA's behavior, however I do know -X also supports GUS in Quake (amung other corner-cases, such as from the demo scene).

Can you see any valuable aspects in this commit that can either simplify or improve upon the -X implementation?

(The DOSBox Staging implementation imported and credited its DMA solution to yourself @joncampbell123 followed by some refactoring; so is functionally aligned with -X).

I just want to make sure we (the collective DOSBox effort) have the best implementation.

joncampbell123 commented 4 years ago

Here's what I remember about DOSBox SVN's GUS DMA support (as it existed in 2016 before I changed it).

SVN's GUS DMA emulation made the transfer immediate by just calling the DMA emulation to read in the data in one go.

They probably did that to support games or demoscene stuff that use DMA to upload the sample data before playing music.

I already changed that to do DMA transfers at GUS speeds (about 600KB/sec if I recall) in 1ms increments which better emulates the actual transfer speed. It also allows Quake to render audio and it allows Windows 3.1 GUS drivers to work as well.

DOSBox-X also preserves the always interpolate function of GUS sampling, while SVN some time back committed some code that skipped interpolation if playing the sample above 1x. Additional changes were made based on DOSLIB test code and observed behavior of an actual Gravis Ultrasound MAX card including the odd undocumented sample rates that occur if less than 14 channels are enabled.

Finally, DOSBox-X by default will emulate the actual sample generation rates of the GUS given the number of channels, while SVN always adapts the audio synthesis to render at a fixed sample rate (usually 44100). DOSBox-X's design better emulates the actual DAC out of a GUS including artifacts at 32 channels. It also allows a suggested hack in GUS documentation to work regarding interleaved stereo samples in GUS RAM. The suggestion is to use two voices, panned L and R, and set the playback rate to exactly 2.00 (skip every other sample). See http://hackipedia.org/browse.cgi/Computer/Platform/PC%2c%20IBM%20compatible/Sound%20and%20Music/Gravis/Ultrasound/UltraSound%20Lowlevel%20ToolKit%20%281994%2d12%2d21%29%20v2%2e22%2epdf section 3.12 Stereo Playback / Hardware control.

kcgen commented 4 years ago

Thanks for the detailed history @joncampbell123 !

SVN always adapts the audio synthesis to render at a fixed sample rate (usually 44100)

(DOSBox-X DMA implementation) also allows Quake to render audio

Commit 4387 is brand new (just a day or so ago), https://sourceforge.net/p/dosbox/code-0/4387/#diff-2, and I'm raising this question because these two defects have been fixed.

The commit is very small; and given Quake is now happy w/ DMA. Curious if you can re-assess it (just the DMA aspect of this code.. certainly the other areas you've mentioned have not been addressed).

kcgen commented 4 years ago

jmarsh from the vogons.org forums offered these interesting details:

I originally wrote that patch over 18 months ago. Afair the only part relevant to making Quake (and Q2dos) produce audio was fixing one bit in a register that represents different things depending on if it's being read vs. being written. The DMA implementation wasn't touched.

Thanks jmarsh.

joncampbell123 commented 4 years ago

Ah, I get it.

DOSBox-X happens to work because it sets the bit on read for ANY pending IRQ.

http://hackipedia.org/browse.cgi/Computer/Platform/PC%2c%20IBM%20compatible/Sound%20and%20Music/Gravis/Ultrasound/UltraSound%20Lowlevel%20ToolKit%20%281994%2d12%2d21%29%20v2%2e22%2epdf

Firefox_Screenshot_2020-10-22T15-57-00 266Z

joncampbell123 commented 4 years ago

Hey, on a related note, Gravis's PLAYMIDI is unable to enumerate anything in the current directory, even though directories are there. If I force it to navigate the MIDI subdirectory it can see the MIDI files. playmidi_000

joncampbell123 commented 4 years ago

Testing the change from SVN in DOSBox-X... it seems to have fixed the hang in Warcraft II without having to use the "clear DMA IRQ if polling too much" hack.

joncampbell123 commented 4 years ago

I just did some testing with some demoscene entries known to support the GUS and NOT work in SVN.

The patch does not break anything in DOSBox-X and it fixes some hangs and issues with some of the demos when run in SVN.

Wengier commented 4 years ago

@joncampbell123 I have checked that the issue with PLAYMIDI listing directories seems to be mostly Linux-specific. Because on non-Windows systems the Archive attribute was always applied to both files and directories, but PLAYMIDI seems to only list directories without any attributes. This is likely a bug of PLAYMIDI itself, but I have already fixed the code to not apply Archive attribute for directories for non-Windows systems so that PLAYMIDI should now list directories correctly. Please check out the new code.

kcgen commented 4 years ago

The patch does not break anything in DOSBox-X and it fixes some hangs and issues with some of the demos when run in SVN.

Fantastic news! I'm sure the DOSBox team will also be happy to hear of these improvements given your GUS knowledge and corner-cases regarding WC2 and various demos. Sounds like this is an overall improvement.

I plan to run tests as well; will post results when I get through that.

Wengier commented 4 years ago

@kcgen I agree that it is definitely a big progress. Please let us know when you get results on your end too. Thanks!

kcgen commented 4 years ago

Passing everything I've thrown at it! - https://github.com/dosbox-staging/dosbox-staging/pull/685#issuecomment-716171743

Thanks again @joncampbell123 for taking a look at this and folding it into your DMA implementation.