VOGONS

Common searches


Reply 80 of 145, by Qbix

User metadata
Rank DOSBox Author
Rank
DOSBox Author

I commited as is and then a little change, as not all compilers that we use, support the c++11 array bracket in constructors. I hope the new code is fine as well

Water flows down the stream
How to ask questions the smart way!

Reply 82 of 145, by Yesterplay80

User metadata
Rank Oldbie
Rank
Oldbie

krcroft, do you have an updated patch? Version 9 of your patch doesn't apply any longer after r4223.

My full-featured DOSBox SVN builds for Windows & Linux: Vanilla DOSBox and DOSBox ECE (Google Drive Mirror)

Reply 83 of 145, by krcroft

User metadata
Rank Oldbie
Rank
Oldbie
Yesterplay80 wrote:

krcroft, do you have an updated patch? Version 9 of your patch doesn't apply any longer after r4223.

Yesterplay80, thanks for the heads up. Sync'd up patch is posted (download)

Reply 84 of 145, by Yesterplay80

User metadata
Rank Oldbie
Rank
Oldbie

Great, thank you! A new build of ECE is released as well.

My full-featured DOSBox SVN builds for Windows & Linux: Vanilla DOSBox and DOSBox ECE (Google Drive Mirror)

Reply 85 of 145, by krcroft

User metadata
Rank Oldbie
Rank
Oldbie

Version 11 of the patch is now attached to the original post. It passes all build and functional tests on Linux, but I no longer have my Windows VMs to test with (need to rebuild them).
Yesterplay80, DosFreak, if you have time to give it a whirl on Windows, I would greatly appreciate your feedback!

Changelog:

  • CUE handling: reverted MSF handling to that of the current SVN implementation, plus one change per https://sourceforge.net/p/dosbox/bugs/497/ from RibShark, which allows both Rayman and Destruction Derby to play with MSF values of 00:00:00 (or any value, for that matter). I believe this now eliminates the need for MSF control; let me know otherwise.
  • Diagnostics: Added CDROM control status, which can be shown by enabling DEBUG=1 in src/dos/cdrom_image.cpp. An example of the output can be seen in the attachment here: https://sourceforge.net/p/dosbox/bugs/509/
  • FLAC: Replaced the now deprecated PCM-sample-based API in favour of its new frame-based API, which allows time and decode calculations to be independent of the number of channels
  • Updated all of the builtin decoders to their latest stable and current versions; details below

Builtin FLAC decoder dr_flac updated to 0.12.0:

  • API changes including changes to the existing API and the removal of deprecated APIs
  • Add a more flexible way of implementing custom memory allocation routines
  • Seeking performance has been greatly improved. A new binary search based seeking algorithm has been introduced which significantly improves performance over the brute force method which was used when no seek table was present. Seek table based seeking also takes advantage of the new binary search seeking system to further improve performance there as well. Note that this depends on CRC which means it will be disabled when DR_FLAC_NO_CRC is used
  • The SSE4.1 pipeline has been cleaned up and optimized. You should see some improvements with decoding speed of 24-bit files in particular. 16-bit streams should also see some improvement
  • drflac_read_pcm_frames_s16() has been optimized. Previously this sat on top of drflac_read_pcm_frames_s32() and performed it's s32 to s16 conversion in a second pass. This is now all done in a single pass
  • Added an SSE2 optimized code path, such that circa 2000 Pentium4's and 2003 AMD CPUs can benefit from very low CPU usage
  • A minor optimization has been implemented for drflac_read_pcm_frames_s32(). This will now use an SSE2 optimized pipeline for stereo channel reconstruction which is the last part of the decoding process

Builtin MP3 decoder dr_mp3 updated to 0.4.7:

  • Fix two errors emitted from obscure compilers
  • Bring up to date with minimp3, as follows:
  • rename test vector extension to match other files
  • skip ID3v1 in mp3dec_iterate_buf as well
  • Fix compile with MinGW64 7.3 & MSVC 2017
  • handle extended ID3v1 tag
  • add optional ID3v1 skip
  • mp3dec_load: return error if no frames readed
  • L3_huffman: special case for linbits=0, ~10% speedup
  • add -U_FORTIFY_SOURCE to get rid of __memcpy_chk

Builtin Vorbis decoder stb_vorbis updated to 1.17:

  • fix CVE-2019-13217..CVE-2019-13223 (by ForAllSecure)
  • fix clang-cl issues with swprintf
  • fix warnings
  • improve random numbers on Linux (stb_intcmprev, stb_uidict)

Builtin Opus decoder's ogg handler xiph_ogg updated to 1.3.4:

  • Faster slice-by-8 CRC32 implementation. see https://lwn.net/Articles/453931/ for motivation
  • Various build fixes
  • Fix an issue when handling corrupted continue-packets

Reply 86 of 145, by dreamer_

User metadata
Rank Member
Rank
Member

Hi, krcroft - you contacted me in the past by direct message, but I couldn't respond because I had no privileges to send PMs on the forum back then.

I created new Git repo with intention of curating/testing patches and preparing them for eventual submission/re-submission for SVN upstream:
dosbox-staging. I was going through various patches/threads I am interested in and when I reached yours I got reminded of the message 😀. I figured you might be interested in cooperating on it, so it might be more productive than importing your patches manually.

You will find several branches in there already (representing various patches), but there are two important ones: svn/trunk, which is a direct mirror of ^/dosbox/trunk/ path in SVN, preserving SVN history (I will be updating this branch whenever new commits will land in SVN) and master, which includes few changes required to keep the repo usable as Git repo and maintain sync with SVN going forward (I intend to merge-in future changes from svn/trunk branch). Please note every SVN commit is imported as a separate Git commit and it's easy to correlate them, e.g. to find out git commit representing SVN revision 4000 you can do:

$ git log --grep=@4000 --all
commit 7cd00b3d845df5790fa2b83e6b90ef3bbad0c22c
Author: ripsaw8080 <ripsaw8080@users.sourceforge.net>
Date: Mon Oct 3 09:49:59 2016 +0000

Implement an obscure behavior of the VGA DAC, similar to what was done for Star Control 2. Fixes wrong colors in Planet Soccer/Football.

Imported-from: https://svn.code.sf.net/p/dosbox/code-0/dosbox/trunk@4000

Because of that, rebasing your patch will be as easy as simple "git cherry-pick" and creating a patch will be simple with "git format-patch" 😀 I am working on preparing proper CI for this repo at the moment.

| ← Ceci n'est pas une pipe
dosbox-staging

Reply 87 of 145, by Yesterplay80

User metadata
Rank Oldbie
Rank
Oldbie
krcroft wrote:

Version 11 of the patch is now attached to the original post. It passes all build and functional tests on Linux, but I no longer have my Windows VMs to test with (need to rebuild them).
Yesterplay80, DosFreak, if you have time to give it a whirl on Windows, I would greatly appreciate your feedback!

And starting with r4259 it's included in DOSBox ECE as well! Thanks, krcroft!

My full-featured DOSBox SVN builds for Windows & Linux: Vanilla DOSBox and DOSBox ECE (Google Drive Mirror)

Reply 88 of 145, by dreamer_

User metadata
Rank Member
Rank
Member

Hm, I couldn't make it to compile on Linux with GCC 9.2 (it was an error in dr_flac 0.12.0, which seems to be an unreleased version?) - but patch v9 applied cleanly on top of trunk@4258, so I am using it for the time being. Overall I think I found a small bug related to Track field in .cue preventing it from working with Steam version of Tomb Raider, but I need to investigate it further.

| ← Ceci n'est pas une pipe
dosbox-staging

Reply 89 of 145, by krcroft

User metadata
Rank Oldbie
Rank
Oldbie
dreamer_ wrote:

I figured you might be interested in cooperating on it, so it might be more productive than importing your patches manually.

Excellent! I will be happy to progress this patch on a branch. I work in git and use gitlab, but happy to push to github.

Besides the usual push - pull model, does github allow multiple people to work within a repo, such that other developers (like myself) could be given ownership at the branch-level, directly in your repo?
I like the idea of tighter collaboration with people atleast being able to to own branches. In any case, I'm happy with either model if we're limited.

dreamer_ wrote:

I am working on preparing proper CI for this repo at the moment.

Awesome! A CI chain that auto-patches, builds, and generates binaries for testers is exactly what the community of people wanting to help DOSBox needs.
As time and people/talent permit, the number of compilers and architectures supported in the CI chain could be expanded upon too.

Last edited by krcroft on 2019-09-19, 13:55. Edited 2 times in total.

Reply 90 of 145, by krcroft

User metadata
Rank Oldbie
Rank
Oldbie
dreamer_ wrote:

Hm, I couldn't make it to compile on Linux with GCC 9.2 (it was an error in dr_flac 0.12.0, which seems to be an unreleased version?) - but patch v9 applied cleanly on top of trunk@4258, so I am using it for the time being. Overall I think I found a small bug related to Track field in .cue preventing it from working with Steam version of Tomb Raider, but I need to investigate it further.

I know you're still digging into this. If it helps isolate things on your side, I built and tested the patch with the following:

  • gcc version 7.4.0 (Ubuntu 7.4.0-8ubuntu1)
  • gcc version 8.3.0 (Ubuntu 8.3.0-6ubuntu1)
  • gcc version 9.1.0 (Ubuntu 9.1.0-2ubuntu2~19.04)
  • gcc version 9.2.0 (GCC) (custom build)

The patch adds the very strict "-Wall -Wpedantic" arguments in its own Makefile (within the src/libs/decoders tree), and all files added or modified by the patch build without warnings.

For Tomb Raider, I encoded the 59 audio tracks into OPUS, Vorbis, MP3, and FLAC from the flat bin/cue file; both mono and stereo versions.
These mount clean and play in default mode, PSX-mode (tomb -audio 1), and "Tomb Raider II mode" (tomb -audio 2).
Let me know if I can narrow this down; feel free to PM too if that's working for you.

Last edited by krcroft on 2019-09-19, 14:00. Edited 3 times in total.

Reply 92 of 145, by dreamer_

User metadata
Rank Member
Rank
Member
krcroft wrote:

Excellent! I will be happy to progress this patch on a branch. I work in git and use gitlab, but happy to push to github.

I split projects between GitHub and GitLab, depending on several factors - in this case, I think GitHub will attract more interest (and hopefully collaborators - e.g. through easy cross-linking issues to DOSBox-X and duganchen). Also, GitLab CI does not offer Windows builds in the free plan, while GitHub Actions do (and I am quite sure Travis can be set up for Windows builds as well). I develop on Linux and have no way of testing on Windows, so from my POV, Windows-capable CI is needed for this project.

krcroft wrote:

Besides the usual push - pull model, does github allow multiple people to work within a repo, such that other developers (like myself) could be given ownership at the branch-level, directly in your repo?

Yes 😀 In the past I only prepared such rules for Gerrit hosting, but just glancing through GitHub documentation indicates that it's possible to set up rules per-branch (or branch group). I will dig in to learn how to properly set this up and ping you when ready.

krcroft wrote:

Let me know if I can narrow this down; feel free to PM too if that's working for you.

In this case, I think game distribution is at fault - .cue file was not prepared by me, but by someone uploading the game to Steam (I suspect GOG version might be affected as well). I attached the file, so you could inspect it (it has extension .DAT, I added .txt so forum would allow upload; yes, the formatting is broken) - you will notice that File 02 is marked as Track 02 and Index 02, File 03 - Track 03 - Index 03, etc. This lead cue parser to decide that File 10 is next to File 01 and sanity check (`prev.number + 1 != curr.number` in AddTrack function) indicated an error during imgmount. Replacing Indexes for all Tracks to be Index 01 (following .cue files from other games) prevented this from happening. Perhaps .cue parser could be less strict to work around such issues? DOSBox 0.74-3 simply continues here (but music is not playing - although it's more likely to be SDL_sound issue).

Otherwise, earlier today I hacked together your patch with a slightly modified version of SDL2 from other thread (sans Android support) and got nicely working build of DOSBox r4258+SDL2+working music (tested several games using ogg and mp3) 😀 I will need to clean it up and split into small patches, that could be reviewed and (hopefully), some of them included in SVN.

One more thing, that I discovered just yesterday - Ryan actually started work on SDL2_sound, but it hasn't reached releasable state (and there's no guarantee it will) - new version is covered by zlib license, co can be included with DOSBox code, just like your patch does it.

Attachments

  • Filename
    GAME.DAT.txt
    File size
    592 Bytes
    Downloads
    66 downloads
    File comment
    Tomb Raider (Steam edition) .cue file
    File license
    Fair use/fair dealing exception

| ← Ceci n'est pas une pipe
dosbox-staging

Reply 93 of 145, by krcroft

User metadata
Rank Oldbie
Rank
Oldbie

Dreamer_, all sounds good! I'm also Linux-based, so that's really handy if Microsoft's thrown in free Windows-based CI into GitHub (the more people who stay on Windows, the better for them I guess..)

Regarding the CUE file, yes - you've nailed the issue regarding the incorrect INDEX values. The patch doesn't change CUE handling, so SVN would behave similarly but like you noted that code isn't getting far enough without SDL_Sound. The INDEX 00 minute:second:frame (MSF) is the pregap duration, or amount of time to skip at the start of the track (perhaps where either hidden or potentially garbage content might live), where as INDEX 01 is the offset, relative to the start of the FILE, at which to start playback.

I suppose the code could ignore the invalid INDEX numbers and carry on, but given it's a broken CUE, I would be inclined to throw a helpful error message with the offending CUE line (but will leave that for the core devs to weight; I'm trying to minimally touch the baseline code to reduce the review cycles if/when they want to pull this in).

That's great news if SDL2_Sound is back in the works. Ryan also prefers the smaller builtin codecs like stb_ and dr_ and wants to pull in my Opus support. Transitioning from my patch to SDL2_Sound should be straight forward when DOSBox formally moves to SDL2, and would let me reduce this patch to just the runtime audio handling improvements in DOSBox itself (in cdrom_image.cpp); although I'm hopefully those will be pulled in at some point when QBix has cycles to review.

Reply 94 of 145, by Kisai

User metadata
Rank Member
Rank
Member
DosFreak wrote:

I've never been happy with people converting their DOS audio to lossy but I could see it being useful in some circumstances, don't really see any reason for mp3 support but I'm sure someone will bitch of course. Probably someone using a game package they downloaded or unwisely converting all their DOS games to mp3 for some reason. I'm sure they'll be fine with converting their mp3s to ogg. heh

If I could suggest something, throw a scare warning when using lossy input/detectable lossy input that they should try to re-image their game disc and use lossless compression. It's very discernible when cd-audio is ripped to lossy audio due to the stereo separation suddenly going from distinct left and right channels to appearing in the center channel. Someone who is playing a game with lossy-audio likely is playing a pirated version of the game, as it was a common thing back in the late 90's, early 2000's for pirates to convert cd-audio and wav files to lossy audio to make their rips smaller.

krcroft wrote:
Dreamer_, all sounds good! I'm also Linux-based, so that's really handy if Microsoft's thrown in free Windows-based CI into Git […]
Show full quote

Dreamer_, all sounds good! I'm also Linux-based, so that's really handy if Microsoft's thrown in free Windows-based CI into GitHub (the more people who stay on Windows, the better for them I guess..)

Regarding the CUE file, yes - you've nailed the issue regarding the incorrect INDEX values. The patch doesn't change CUE handling, so SVN would behave similarly but like you noted that code isn't getting far enough without SDL_Sound. The INDEX 00 minute:second:frame (MSF) is the pregap duration, or amount of time to skip at the start of the track (perhaps where either hidden or potentially garbage content might live), where as INDEX 01 is the offset, relative to the start of the FILE, at which to start playback.

I suppose the code could ignore the invalid INDEX numbers and carry on, but given it's a broken CUE, I would be inclined to throw a helpful error message with the offending CUE line (but will leave that for the core devs to weight; I'm trying to minimally touch the baseline code to reduce the review cycles if/when they want to pull this in).

That's great news if SDL2_Sound is back in the works. Ryan also prefers the smaller builtin codecs like stb_ and dr_ and wants to pull in my Opus support. Transitioning from my patch to SDL2_Sound should be straight forward when DOSBox formally moves to SDL2, and would let me reduce this patch to just the runtime audio handling improvements in DOSBox itself (in cdrom_image.cpp); although I'm hopefully those will be pulled in at some point when QBix has cycles to review.

The SDL2 patches used right now just re-implement/rip the SDL1 physical cd-rom handling.

The only practical reason to use a lossy compression in the present day is to fit the game onto a device (eg a smartphone/smarttv/nintendo wii/wiiu/switch) where the flash memory primary storage is at a premium. Even then, that's still the venue of the pirates trying to profit off of low quality emulator boxes online, like all the retroarch based pirate boxes seen on ebay.

So if any of this ends up in mainline dosbox, I'd probably suggest making the lossy compression support off-by-default, but the lossless compression on-by-default. When dealing with fluidsynth, the option to use SF2/SFZ/SF3 files that have lossy compression came up and I was like "then I'd have to add all these support libraries, and the output would ultimately be inferior" and opted to not look into it further. But if the stb/dr header's are already in use by one part of dosbox, then using it with another part becomes at least an option without having to compile it in twice in separate libraries which I loathe.

The current SDL2 patch to dosbox doesn't use SDL2_Sound at all.

Reply 95 of 145, by jmarsh

User metadata
Rank Oldbie
Rank
Oldbie
Kisai wrote:

Someone who is playing a game with lossy-audio likely is playing a pirated version of the game, as it was a common thing back in the late 90's, early 2000's for pirates to convert cd-audio and wav files to lossy audio to make their rips smaller.

Practically all games sold on gog.com that have CD audio use lossy audio. Breaking those by default would be unacceptable.

Reply 96 of 145, by Kisai

User metadata
Rank Member
Rank
Member
jmarsh wrote:
Kisai wrote:

Someone who is playing a game with lossy-audio likely is playing a pirated version of the game, as it was a common thing back in the late 90's, early 2000's for pirates to convert cd-audio and wav files to lossy audio to make their rips smaller.

Practically all games sold on gog.com that have CD audio use lossy audio. Breaking those by default would be unacceptable.

Are those the one being sold with just SCUMMVM and without their setup/install utilities?

Just based on https://github.com/scummvm/scummvm-tools

It looks like many of those tools have the option to compress to flac, not just vorbis. And that's not the cd-audio, that's the game audio. If GoG is in fact selling games with lossy audio, they're not doing it with dosbox, and are selling inferior versions.

Anyway, notifying the user that they are playing using lossy audio and should use lossless instead would at least draw attention to the user that they're probably using bad version. Disabling the lossy audio in the standard build would either encourage the user to re-rip the disc, or the publisher to do so. Unfortunately since ScummVM allows lossy audio, that means those publishers might just use scummvm instead and throw ogg's out at the default wav2ogg settings.

Reply 97 of 145, by Yesterplay80

User metadata
Rank Oldbie
Rank
Oldbie
Kisai wrote:

If GoG is in fact selling games with lossy audio, they're not doing it with dosbox, and are selling inferior versions.

They are of course using DOSBox. And for me and most players, a high bandwidth OGG file is more than enough for background music. I also rip my physical games to just an ISO containing the data und the music to OGGs with 320kbit/s.

For gaming only that's good enough and it saves a lot of space on the hard drive. Since many people nowadays use SSDs, which often have less capacity than HDDs, that's the perfect compromise between sound quality and required space.

My full-featured DOSBox SVN builds for Windows & Linux: Vanilla DOSBox and DOSBox ECE (Google Drive Mirror)

Reply 98 of 145, by jmarsh

User metadata
Rank Oldbie
Rank
Oldbie
Kisai wrote:

If GoG is in fact selling games with lossy audio, they're not doing it with dosbox, and are selling inferior versions.

ScummVM doesn't support anywhere close to GoG's full library of DOS games.
If you want a freebie example go grab Shadow Warrior (classic).

Reply 99 of 145, by Kisai

User metadata
Rank Member
Rank
Member
Yesterplay80 wrote:
Kisai wrote:

If GoG is in fact selling games with lossy audio, they're not doing it with dosbox, and are selling inferior versions.

They are of course using DOSBox. And for me and most players, a high bandwidth OGG file is more than enough for background music. I also rip my physical games to just an ISO containing the data und the music to OGGs with 320kbit/s.

For gaming only that's good enough and it saves a lot of space on the hard drive. Since many people nowadays use SSDs, which often have less capacity than HDDs, that's the perfect compromise between sound quality and required space.

That's a weird angle to take when you also integrate "more accurate FM synth"'s into your build. Saying "the user can't really tell" is not a good reason to reduce the quality on them. One should first make efforts to preserve the original media and data on them, then let the user decide if they want to save that space, don't make that decision for them. Internet bandwidth and disks are fast and large enough that you don't need to destroy your DOS games this way. One could even suggest just making it so DOSBOX can read a zip-file of the game "disc" and save having to implement any of the compression codecs at all since zlib is already used for the video capture codec.

Anyway all I'm asking is that the user be made aware when they are using lossy audio and bad audio settings in general. Something like:
"Lossy audio used for CD track #, you might want to use lossless compression instead"
"(FM Synth|Tandy PSG|MT32|MIDI) initialized below output rate, this might not be desirable"

If they know they did this on purpose, then it doesn't matter to them, but it should encourage the publisher's to put out better versions.