VOGONS


First post, by gilgamesh

User metadata
Rank Newbie
Rank
Newbie

Hello,

I think I fond a logical bug in src/dos/cdrom_image.cpp.

When I save a whole CD with data and audio tracks in one big BIN-image (for mounting a BIN/CUE comination) everything works just fine. CDROM_Interface_Image::GetAudioTrackInfo returns the correct offset (minutes, seconds, frames) for each audio track. When I specify compressed single files for each track via the MP3 tag in my cue-file, then the offsets are a mess.

I think that dosbox calculates the offsets by using the size in bytes of each file, which doesn't make sense with compressed audio tracks (see CDROM_Interface_Image::AddTrack):

/*...*/
// current track uses a different file as the previous track
} else {
int tmp = prev.file->getLength() - prev.skip;
prev.length = tmp / prev.sectorSize;
if (tmp % prev.sectorSize != 0) prev.length++; // padding

curr.start += prev.start + prev.length + currPregap;
curr.skip = skip * curr.sectorSize;
shift += prev.start + prev.length;
totalPregap = currPregap;
}
/*...*/

It should rather retrieve the length from SDL_audio or use the data explicitly specified via the INDEX tag which is processed in dos/cdrom_image.cpp: CDROM_Interface_Image::LoadCueSheet.

I know of at least one game (Realms of Arkania3 / Das schwarze Auge 3) which checks the offsets before playing them.

Are my thoughts concerning this right and does anybody know of a hack to get around this problem?

Reply 1 of 16, by gilgamesh

User metadata
Rank Newbie
Rank
Newbie

I just changed

if(prev.file == curr.file) {

into

if(true) {

in CDROM_Interface_Image::AddTrack and then got the offsets right. This seems to be the only correct behavoir to me.

(Anyway, that didn't solve my problem. There seems to be still another issue 😢)

Reply 2 of 16, by Qbix

User metadata
Rank DOSBox Author
Rank
DOSBox Author

you are right that it makes no sense to use filelength with compressed files. I recall though that there was something with the length with sdl_sound. Think it requires a relatively new version of it.

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

Reply 3 of 16, by gilgamesh

User metadata
Rank Newbie
Rank
Newbie

I recall though that there was something with the length with sdl_sound.

SDL_sound doesn't seem to be neccessary any more, since making the if-statement unconditional (so that the quoted code snippet becomes inaccessible, see my last posting) gives the correct results. Cuesheets with several files should always be treated as if they were one cd image (at least when track offsets are concerned). I wonder what made you developers choose a different (probably incorrect) approach.

Maybe this could be corrected in the next release?

Reply 5 of 16, by gilgamesh

User metadata
Rank Newbie
Rank
Newbie

Well,II made a mistake there.

AudioFile::getLength() returns the length in some time unit and not in bytes.
It repeats Sound_Seek() until the end is reached and determines thus the play length.
Nevertheless the results are not correct.

My hack worked most of the time because it made dosbox take the data from the cuesheet. It failed for the last track, because cuesheets store no data for lead out tracks.

I wrote a small program based on the dosbox sources that computes the correct lengths of the files in frames: I changed the factor 176.4f [? / millisecond] to 75/1000 [frames / millisecond]. The macro FRAMES_TO_MSF returns the correct length for each track. The algo is ok then.
When I dump the info that MSCDEX returns as track info, the results for compressed audio tracks differ from the (correct) results for non-compressed tracks.

I wonder what the factor 176.4f means. If that is correct, then I assume the bug is in CDROM_Interface_Image::AddTrack(), although I can't spot it. Maybe I'll have to dig into MSCDEX's details then.

Any other suggestions?

Reply 6 of 16, by gilgamesh

User metadata
Rank Newbie
Rank
Newbie

Found three errors in CDROM_Interface_Image::AddTrack():

	//Track &prev = *(tracks.end() - 1);
Track prev = tracks.back();

and

	//prev.length = prev.file->getLength() / prev.sectorSize;
if(prev.number == 1) {
prev.length = prev.file->getLength() / prev.sectorSize;
} else {
prev.length = tmp;
}

and

	//curr.start += prev.start + prev.length + currPregap;
curr.start = prev.start + prev.length + currPregap;

The result is not perfect, but lies within a few frames from the correct result and not half hours. That's sufficient to run the game. I'll post a patch soon. Maybe I should do some more testing, first.

EDIT: forgot the central part, in CDROM_Interface_Image::AudioFile::getLength():

	    //if (time == 1) return lround((double)shift * 176.4f);
if (time == 1) return lround((double)shift * 75/1000);

Reply 7 of 16, by prompt

User metadata
Rank Newbie
Rank
Newbie

Hi,

It is been a long time since I wrote this code, but here is some info:

1) AudioFile::getLength is supposed to return the length in bytes. SDL_sound uses ms. So I multiply it with a factor. I should have put a comment on how I calculated it. In principle this should be sample_rate (44100 Hz = 1/s) * bit_depth (2 bytes) * channels (2) / ms->s (1000). This will never be 100% accurate as the length of a cd audio track is always a multiple of a frame (1/75s) and there will be some rounding.

2) I used an exponential search algorithm since SDL_sound did not provide a getLength at the time I wrote the code. The algorithm should work though. Feel free to replace it if a stable version of SDL_sound now provides a getLength.

3) As for your changes: I had a very hard time getting it right for every single exception. Please make sure you run many regression tests for your changes.

4) I remember SDL_sound having a bug where it would where SoundSeek did not work, you could check if it does work for your Soundfile type.

5) You could also post your cue file so we can check if it is correct.

Cheers,
Martin

Reply 8 of 16, by gilgamesh

User metadata
Rank Newbie
Rank
Newbie

Thanks for the detailed answer.

prompt wrote:

1) AudioFile::getLength is supposed to return the length in bytes. SDL_sound uses ms.

Something goes wrong with this. For my program with a CUE/BIN pair the unpatched dosbox returns via mscdex the correct values:

  • Trackinfo 1: min0 sec2 fr0 attr64
    Trackinfo 2: min35 sec21 fr41 attr0
    Trackinfo 3: min37 sec41 fr70 attr0
    Trackinfo 4: min39 sec6 fr74 attr0
    Trackinfo 5: min41 sec38 fr17 attr0
    Trackinfo 6: min43 sec47 fr29 attr0
    Trackinfo 7: min46 sec39 fr11 attr0
    Trackinfo 8: min48 sec51 fr52 attr0
    Trackinfo 9: min51 sec33 fr30 attr0
    Trackinfo 10: min53 sec47 fr54 attr0
    Trackinfo 11: min56 sec32 fr53 attr0
    Trackinfo 12: min58 sec57 fr44 attr0
    Trackinfo 13: min60 sec57 fr66 attr0

When I use compressed tracks as CUE/ISO/OGGs the according values become:

  • Trackinfo 1: min0 sec2 fr0 attr64
    Trackinfo 2: min66 sec12 fr0 attr0
    Trackinfo 3: min106 sec10 fr24 attr0
    Trackinfo 4: min146 sec38 fr27 attr0
    Trackinfo 5: min190 sec43 fr62 attr0
    Trackinfo 6: min236 sec36 fr28 attr0
    Trackinfo 7: min30 sec3 fr21 attr0
    Trackinfo 8: min81 sec3 fr39 attr0
    Trackinfo 9: min135 sec14 fr47 attr0
    Trackinfo 10: min191 sec12 fr50 attr0
    Trackinfo 11: min250 sec26 fr27 attr0
    Trackinfo 12: min55 sec44 fr62 attr0
    Trackinfo 13: min118 sec39 fr0 attr0

My version of getLength now returns the value in msec, and comes very near the original values.
It would be cleaner to have it returning the length in bytes, but I have the feeling that this was the critical point and I don't quite understand, what exactly went wrong.
(And it does what I want it to do, this way).

prompt wrote:

2) The algorithm should work though. Feel free to replace it if a stable version of SDL_sound now provides a getLength.

It works, and I didn't find anything better.

(Btw, the CUE-file is correct, since it works for CUE/BIN. When compression comes into play, the values from the CUE are simply overridden with those from getLength.)

Reply 9 of 16, by prompt

User metadata
Rank Newbie
Rank
Newbie

(Btw, the CUE-file is correct, since it works for CUE/BIN. When compression comes into play, the values from the CUE are simply overridden with those from getLength.)

The cue file itself does not contain explicit track lengths. The length of a track is determined by the index of the following track or the end of a file. Knowledge about the file length is essential. After a new file is set, the index starts at 0 again.
Btw: The precision of the length is already lost in SDL_Sound as they do convert it to ms

Trackinfo 1: min0 sec2 fr0 attr64
Trackinfo 2: min66 sec12 fr0 attr0

Did you actually shorten the binary file? You have to extract the first track.

In a short search I found this, it contain some more examples:
http://www.pcnineoneone.com/howto/cdburnadv4.html

I am as well attaching a sample cue file that I made for monkey island.

Reply 10 of 16, by gilgamesh

User metadata
Rank Newbie
Rank
Newbie

Yes, I copied the data track's contents into a new ISO image. I think the difference between the lengths of the first (and second, third, ...) track are a real bug.
(Otherwise I wouldn't have written the patch.)

Most games won't have noticed the bug because they don't explicitly check for track lengths.

Anyway, I'll post the two CUEs.
(Original:)

FILE "dsa3.bin" BINARY
TRACK 01 MODE1/2352
INDEX 01 00:00:00
TRACK 02 AUDIO
PREGAP 00:02:00
INDEX 01 35:17:41
TRACK 03 AUDIO
INDEX 01 37:37:70
TRACK 04 AUDIO
INDEX 01 39:02:74
TRACK 05 AUDIO
INDEX 01 41:34:17
TRACK 06 AUDIO
INDEX 01 43:43:29
TRACK 07 AUDIO
INDEX 01 46:35:11
TRACK 08 AUDIO
INDEX 01 48:47:52
TRACK 09 AUDIO
INDEX 01 51:29:30
TRACK 10 AUDIO
INDEX 01 53:43:54
TRACK 11 AUDIO
INDEX 01 56:28:53
TRACK 12 AUDIO
INDEX 01 58:53:44
TRACK 13 AUDIO
INDEX 01 60:53:66

(With compression:)

FILE "dsa3.iso" BINARY
TRACK 01 MODE1/2048
INDEX 01 00:00:00
FILE "track02.ogg" MP3
TRACK 02 AUDIO
PREGAP 00:02:00
INDEX 01 35:17:41
FILE "track03.ogg" MP3
TRACK 03 AUDIO
INDEX 01 37:37:70
FILE "track04.ogg" MP3
TRACK 04 AUDIO
INDEX 01 39:02:74
FILE "track05.ogg" MP3
TRACK 05 AUDIO
INDEX 01 41:34:17
FILE "track06.ogg" MP3
TRACK 06 AUDIO
INDEX 01 43:43:29
FILE "track07.ogg" MP3
TRACK 07 AUDIO
INDEX 01 46:35:11
FILE "track08.ogg" MP3
TRACK 08 AUDIO
INDEX 01 48:47:52
FILE "track09.ogg" MP3
TRACK 09 AUDIO
INDEX 01 51:29:30
FILE "track10.ogg" MP3
TRACK 10 AUDIO
INDEX 01 53:43:54
FILE "track11.ogg" MP3
TRACK 11 AUDIO
INDEX 01 56:28:53
FILE "track12.ogg" MP3
TRACK 12 AUDIO
INDEX 01 58:53:44
FILE "track13.ogg" MP3
TRACK 13 AUDIO
INDEX 01 60:53:66

Reply 11 of 16, by prompt

User metadata
Rank Newbie
Rank
Newbie
FILE "track02.ogg" MP3 TRACK 02 AUDIO PREGAP 00:02:00 INDEX 01 35:17:41 […]
Show full quote

FILE "track02.ogg" MP3
TRACK 02 AUDIO
PREGAP 00:02:00
INDEX 01 35:17:41

This is your problem: As I stated already after a new file command the count starts again at 0. Please see the linked web page above. You set the index 35+ minutes *after* the start of your sound file...

See here my monkey island example:

FILE /home/emulation/dos/monkey/monkey.iso BINARY
TRACK 01 MODE1/2352
INDEX 01 00:00:00
FILE /home/emulation/dos/monkey/tracks/track2.ogg MP3
TRACK 02 AUDIO
PREGAP 00:04:00
INDEX 01 00:00:00
FILE /home/emulation/dos/monkey/tracks/track3.ogg MP3
TRACK 03 AUDIO
INDEX 01 00:00:00
FILE /home/emulation/dos/monkey/tracks/track4.ogg MP3
TRACK 04 AUDIO
INDEX 01 00:00:00
FILE /home/emulation/dos/monkey/tracks/track5.ogg MP3
TRACK 05 AUDIO
INDEX 01 00:00:00
FILE /home/emulation/dos/monkey/tracks/track6.ogg MP3
TRACK 06 AUDIO
INDEX 01 00:00:00
FILE /home/emulation/dos/monkey/tracks/track7.ogg MP3
TRACK 07 AUDIO
INDEX 01 00:00:00
FILE /home/emulation/dos/monkey/tracks/track8.ogg MP3
TRACK 08 AUDIO
INDEX 01 00:00:00
FILE /home/emulation/dos/monkey/tracks/track9.ogg MP3
TRACK 09 AUDIO
INDEX 01 00:00:00
FILE /home/emulation/dos/monkey/tracks/track10.ogg MP3
TRACK 10 AUDIO
INDEX 01 00:00:00
FILE /home/emulation/dos/monkey/tracks/track11.ogg MP3
TRACK 11 AUDIO
INDEX 01 00:00:00
FILE /home/emulation/dos/monkey/tracks/track12.ogg MP3
TRACK 12 AUDIO
INDEX 01 00:00:00
FILE /home/emulation/dos/monkey/tracks/track13.ogg MP3
TRACK 13 AUDIO
INDEX 01 00:00:00
FILE /home/emulation/dos/monkey/tracks/track14.ogg MP3
TRACK 14 AUDIO
INDEX 01 00:00:00
FILE /home/emulation/dos/monkey/tracks/track15.ogg MP3
TRACK 15 AUDIO
INDEX 01 00:00:00
FILE /home/emulation/dos/monkey/tracks/track16.ogg MP3
TRACK 16 AUDIO
INDEX 01 00:00:00
FILE /home/emulation/dos/monkey/tracks/track17.ogg MP3
TRACK 17 AUDIO
INDEX 01 00:00:00
FILE /home/emulation/dos/monkey/tracks/track18.ogg MP3
TRACK 18 AUDIO
INDEX 01 00:00:00
FILE /home/emulation/dos/monkey/tracks/track19.ogg MP3
TRACK 19 AUDIO
INDEX 01 00:00:00
FILE /home/emulation/dos/monkey/tracks/track20.ogg MP3
TRACK 20 AUDIO
Show last 17 lines
    INDEX 01 00:00:00
FILE /home/emulation/dos/monkey/tracks/track21.ogg MP3
TRACK 21 AUDIO
INDEX 01 00:00:00
FILE /home/emulation/dos/monkey/tracks/track22.ogg MP3
TRACK 22 AUDIO
INDEX 01 00:00:00
FILE /home/emulation/dos/monkey/tracks/track23.ogg MP3
TRACK 23 AUDIO
INDEX 01 00:00:00
FILE /home/emulation/dos/monkey/tracks/track24.ogg MP3
TRACK 24 AUDIO
INDEX 01 00:00:00
FILE /home/emulation/dos/monkey/tracks/track25.ogg MP3
TRACK 25 AUDIO
INDEX 01 00:00:00

Reply 12 of 16, by gilgamesh

User metadata
Rank Newbie
Rank
Newbie

Oh my. It really works with “INDEX 01 00:00:00” for all audio tracks without patching. (I feel stupid now 😳 )

Was I supposed to know this? Wouldn't it be more logical and intuitive if it worked with more or less the same cuesheet as in the cue/bin pair?

Maybe all this info should find its way into the dosbox reference material? (I would write something for the wiki, but obviously I don't know enough for that.)

Reply 13 of 16, by MiniMax

User metadata
Rank Moderator
Rank
Moderator

gilgamesh - I think you should thrown something about this into the wiki. It is always easier for the next guy to edit/expand on something that is already there than having to start a new page. Wiki's are always a work in progress, so it is never supposed to be perfect. Add a link to this thread if you think parts of what you write is not correct.

DOSBox 60 seconds guide | How to ask questions
_________________
Lenovo M58p | Core 2 Quad Q8400 @ 2.66 GHz | Radeon R7 240 | LG HL-DT-ST DVDRAM GH40N | Fedora 32

Reply 15 of 16, by MiniMax

User metadata
Rank Moderator
Rank
Moderator

Looks great to me! 😀 Thanks gilgamesh.

DOSBox 60 seconds guide | How to ask questions
_________________
Lenovo M58p | Core 2 Quad Q8400 @ 2.66 GHz | Radeon R7 240 | LG HL-DT-ST DVDRAM GH40N | Fedora 32