FLAC support

Developer's Forum, for discussion of bugs, code, and other developmental aspects of DOSBox.

Re: FLAC support

Postby Yesterplay80 » 2016-9-23 @ 07:02

Would you mind sharing your source and/or build with us, NewRisingSun?
My full-featured DOSBox SVN builds (without debugger) for Windows: Vanilla DOSBox and DOSBox ECE (Enhanced Community Edition)
User avatar
Yesterplay80
Member
 
Posts: 407
Joined: 2016-2-23 @ 11:02
Location: Germany

Re: FLAC support

Postby NewRisingSun » 2016-9-23 @ 07:51

I will post a source diff later. I'm still having problems seeking to a particular point within a flac file.
NewRisingSun
Oldbie
 
Posts: 844
Joined: 2005-9-02 @ 02:26

Re: FLAC support

Postby NewRisingSun » 2016-9-23 @ 09:11

First, we need to remove two problems from SDL_Sound's decoders/flac.c itself. First, the fact that it forces the caller to deal with the FLAC file's "block size", which the caller should not have to care about. DOSBox wants 2,325 byte sized morsels, not the entire FLAC block size, which is why there is so much skipping. flac.c will now buffer the decoded frame and return exactly as many bytes as the caller desires. Not elegant, but it seems to do the job. Second, FLAC_seek did not always seek as intended, producing an integer overflow when seeking a couple of minutes into a track.
Code: Select all
--- flac.c.old   2014-04-18 10:47:05 +0000
+++ flac.c   2016-09-23 09:02:30 +0000
@@ -168,11 +168,18 @@
     Uint32 frame_size;
     Uint8 is_flac;
     Uint32 stream_length;
+    unsigned char *frameBuffer;
+    unsigned long int frameBufferSize;
+    unsigned long int frameBufferPos;
 } flac_t;
 
 
 static void free_flac(flac_t *f)
 {
+    if (f->frameBuffer) {
+   free(f->frameBuffer);
+   f->frameBuffer = NULL;
+    }
     d_finish(f->decoder);
     d_delete(f->decoder);
     free(f);
@@ -231,10 +238,16 @@
     f->frame_size = frame->header.channels * frame->header.blocksize
         * frame->header.bits_per_sample / 8;
 
-    if (f->frame_size > f->sample->buffer_size)
-        Sound_SetBufferSize(f->sample, f->frame_size);
-
-    dst = f->sample->buffer;
+    if (f->frameBuffer == NULL) {
+   f->frameBuffer = malloc(f->frame_size);
+   f->frameBufferSize = f->frame_size;
+    }
+    if (f->frameBufferSize != f->frame_size) {
+   realloc(f->frameBuffer, f->frame_size);
+   f->frameBufferSize = f->frame_size;
+    }
+    dst = f->frameBuffer;
+    f->frameBufferPos = 0;
 
         /* If the sample is neither exactly 8-bit nor 16-bit, it will have to
          * be converted. Unfortunately the buffer is read-only, so we either
@@ -478,6 +491,9 @@
     f->decoder = decoder;
     f->sample->actual.format = 0;
     f->is_flac = 0 /* !!! FIXME: should be "has_extension", not "0". */;
+    f->frameBuffer = NULL;
+    f->frameBufferSize = 0;
+    f->frameBufferPos = 0;
 
     internal->decoder_private = f;
     /* really should check the init return value here: */
@@ -541,24 +557,32 @@
     Sound_SampleInternal *internal = (Sound_SampleInternal *) sample->opaque;
     flac_t *f = (flac_t *) internal->decoder_private;
     Uint32 len;
+    unsigned char *dst;
 
-    if (!d_process_one_frame(f->decoder))
-    {
-        sample->flags |= SOUND_SAMPLEFLAG_ERROR;
-        BAIL_MACRO("FLAC: Couldn't decode frame.", 0);
-    } /* if */
-
-    if (d_get_state(f->decoder) == D_END_OF_STREAM)
-    {
-        sample->flags |= SOUND_SAMPLEFLAG_EOF;
-        return(0);
-    } /* if */
-
-        /* An error may have been signalled through the error callback. */   
-    if (sample->flags & SOUND_SAMPLEFLAG_ERROR)
-        return(0);
-
-    return(f->frame_size);
+    len = 0;
+    dst = sample->buffer;
+    while (len < f->sample->buffer_size) {
+   if (f->frameBufferPos >= f->frameBufferSize) {
+      if (!d_process_one_frame(f->decoder))
+      {
+         sample->flags |= SOUND_SAMPLEFLAG_ERROR;
+         BAIL_MACRO("FLAC: Couldn't decode frame.", 0);
+      } /* if */
+      
+      if (d_get_state(f->decoder) == D_END_OF_STREAM)
+      {
+         sample->flags |= SOUND_SAMPLEFLAG_EOF;
+         break;
+      } /* if */
+      
+         /* An error may have been signalled through the error callback. */   
+      if (sample->flags & SOUND_SAMPLEFLAG_ERROR)
+         break;
+   }
+   if (f->frameBufferSize == 0) break;
+   dst[len++] = f->frameBuffer [f->frameBufferPos++];   
+    }
+    return(len);
 } /* FLAC_read */
 
 
@@ -573,7 +597,14 @@
     Sound_SampleInternal *internal = (Sound_SampleInternal *) sample->opaque;
     flac_t *f = (flac_t *) internal->decoder_private;
 
-    d_seek_absolute(f->decoder, (ms * sample->actual.rate) / 1000);
+    d_seek_absolute(f->decoder, (long long) ms * sample->actual.rate / 1000);
+    /* Invalidate frame buffer */
+    if (f->frameBuffer) {
+   free(f->frameBuffer);
+   f->frameBuffer = NULL;
+    }
+    f->frameBufferSize = 0;
+    f->frameBufferPos = 0;
     return(1);
 } /* FLAC_seek */
 

Next, in src/dos/cdrom_image.cpp, DOSBox should just report the track length as returned by SDL_Sound, not try to determine by itself how far into the track it can seek. I also replaced the floating-point arithmetic with integer arithmetic that shows why the constant is 176.4 (a sample rate of 44,100 samples per second multiplied by two channels per sample multiplied by 16 bits per sample divided by 8 bits per byte divided by 1000 milliseconds per second).
Code: Select all
--- cdrom_image.cpp.old   2015-01-08 22:10:02 +0000
+++ cdrom_image.cpp   2016-09-23 08:01:50 +0000
@@ -91,7 +91,7 @@
       if (!success) return false;
    }
    if (lastSeek != (seek - count)) {
-      int success = Sound_Seek(sample, (int)((double)(seek) / 176.4f));
+      int success = Sound_Seek(sample, (long long) seek * 1000 / (44100 * 2 * (16/8)) );
       if (!success) return false;
    }
    lastSeek = seek;
@@ -108,21 +108,9 @@
 
 int CDROM_Interface_Image::AudioFile::getLength()
 {
-   int time = 1;
-   int shift = 0;
    if (!(sample->flags & SOUND_SAMPLEFLAG_CANSEEK)) return -1;
    
-   while (true) {
-      int success = Sound_Seek(sample, (unsigned int)(shift + time));
-      if (!success) {
-         if (time == 1) return lround((double)shift * 176.4f);
-         shift += time >> 1;
-         time = 1;
-      } else {
-         if (time > ((numeric_limits<int>::max() - shift) / 2)) return -1;
-         time = time << 1;
-      }
-   }
+   return (long long) 44100 * 2 * (16/8) * Sound_GetDuration(sample) / 1000;
 }
 #endif
 
Note that SDL_Sound does not support sample-accurate seeking, because the seek position is specified in milliseconds and is an integer. The 1991 CD-ROM version of Mixed-up Mother Goose has all its speech lines on one long audio track, but it seems to seek and play well enough from the FLAC file that I just tried it with. You will never get perfectly seamless playing from the end of one track to the next track though. But again, that seems to be a problem of SDL_Sound in general, not just of its FLAC decoder.
NewRisingSun
Oldbie
 
Posts: 844
Joined: 2005-9-02 @ 02:26

Re: FLAC support

Postby Yesterplay80 » 2016-9-23 @ 11:51

Thanks, NewRisingSun. I just set everything up and tried to complie DOSBox, but got the following error:
Code: Select all
cdrom_image.cpp: In member function 'virtual int CDROM_Interface_Image::AudioFile::getLength()':
cdrom_image.cpp:113:66: error: 'Sound_GetDuration' was not declared in this scope
  return (long long) 44100 * 2 * (16/8) * Sound_GetDuration(sample) / 1000;
                                                                  ^
My full-featured DOSBox SVN builds (without debugger) for Windows: Vanilla DOSBox and DOSBox ECE (Enhanced Community Edition)
User avatar
Yesterplay80
Member
 
Posts: 407
Joined: 2016-2-23 @ 11:02
Location: Germany

Re: FLAC support

Postby NewRisingSun » 2016-9-23 @ 12:01

You need the development branch of SDL_sound (last updated in 2012), not the stable branch (last updated in 2008). See the bottom of this page for instructions.
NewRisingSun
Oldbie
 
Posts: 844
Joined: 2005-9-02 @ 02:26

Re: FLAC support

Postby Yesterplay80 » 2016-9-23 @ 13:07

I see. The problem, my problem, to be precise, with the development branch is that is seems you have to compile it with Visual Studio, instead of MinGW. And there it looks like it depends on SDL and even can't find some of it's own files. At the moment I don't have the time to get deeper into that matter. Maybe someone else would like to try his luck?
My full-featured DOSBox SVN builds (without debugger) for Windows: Vanilla DOSBox and DOSBox ECE (Enhanced Community Edition)
User avatar
Yesterplay80
Member
 
Posts: 407
Joined: 2016-2-23 @ 11:02
Location: Germany

Re: FLAC support

Postby NewRisingSun » 2016-9-23 @ 13:26

I had almost no problems compiling it on MingW32, providing that I used the --prefix option and removed the "#include <math.h>" from "decoders\timidity\tables.h". Maybe just one: I did have to copy SDL_sound.h manually to the include directory, because "make install" somehow would not do it.
NewRisingSun
Oldbie
 
Posts: 844
Joined: 2005-9-02 @ 02:26

Re: FLAC support

Postby Dominus » 2016-9-23 @ 13:32

Would be a good case for a bug report on SDL's bugzilla. Also might be worthwhile to look through there whether someone has alreadypatched the FLAC seeking.
User avatar
Dominus
DOSBox Moderator
 
Posts: 7910
Joined: 2002-10-03 @ 09:54
Location: Ludwigsburg

Re: FLAC support

Postby NewRisingSun » 2016-9-23 @ 13:43

I think SDL_sound is a separate project from SDL. And its repository has not been updated since 2012.
NewRisingSun
Oldbie
 
Posts: 844
Joined: 2005-9-02 @ 02:26

Re: FLAC support

Postby Dominus » 2016-9-23 @ 13:54

I think it's also "maintained" by the SDL devs but I'd need to look it up again.
User avatar
Dominus
DOSBox Moderator
 
Posts: 7910
Joined: 2002-10-03 @ 09:54
Location: Ludwigsburg

Re: FLAC support

Postby Yesterplay80 » 2016-9-23 @ 14:11

NewRisingSun wrote:I had almost no problems compiling it on MingW32, providing that I used the --prefix option and removed the "#include <math.h>" from "decoders\timidity\tables.h". Maybe just one: I did have to copy SDL_sound.h manually to the include directory, because "make install" somehow would not do it.
Sorry, my bad, I forgot to run "./bootstrap" before trying to run "./configure". The rest worked like a charm, even sdl_sound.h was copied as intended.

So, basically, FLAC works with my build now, too. I tested again with a DOS cd player and an ISO/FLAC of the game "World Rally Fever". The cd player played all songs correctly, in the game the tracks all start maybe a half second, a second into the track. I assume this has to do with how a programm seeks and/or accesses a track on a disc (image).

For all who are interested, here's the exe:
dosbox.7z
DOSBox enhanced with FLAC support
(1.19 MiB) Downloaded 79 times
My full-featured DOSBox SVN builds (without debugger) for Windows: Vanilla DOSBox and DOSBox ECE (Enhanced Community Edition)
User avatar
Yesterplay80
Member
 
Posts: 407
Joined: 2016-2-23 @ 11:02
Location: Germany

Re: FLAC support

Postby krcroft » 2018-7-20 @ 01:51

NewRisingSun,
I manually applied your FLAC buffer patch to the latest dev branch of SDL_sound (and then built without issue) and similarly added the getDuration change to cdrom_image.cpp in SVN dosbox (which also built cleanly).

Finally, I confirmed the dosbox binary was linked to the new SDL_sound library and tried playing a game with a FLAC-encoded track loaded via CUE. It mounted cleanly and loaded the track, confirmed with:

Code: Select all
DOSBox version SVN
Copyright 2002-2017 DOSBox Team, published under GNU GPL.
---
CONFIG:Loading primary settings from config file /home/kcroft/.dosbox/dosbox-SVN.conf
MIXER: Got different values from SDL: freq 48000, blocksize 2048
ALSA:Client initialised [128:0]
MIDI: Opened device:alsa
AudioFile audio-flac/audio02.flac is 2-channel@44100 Hz
AudioFile audio-flac/audio02.flac is 2-channel@44100 Hz


Unfortunately the CDDA sound is absent during the game in Dosbox. When the same track is encoded as OGG or MP3, the CDDA plays without issue.
Tested on Linux; I haven't tried Windows yet.

Yesterplay80,
You mentioned your build now contains this (or maybe did at one point?), but I'm no longer seeing the changes to cdrom_image.cpp using getDuration in
Code: Select all
https://dosbox.yesterplay80.net/DOSBox%20ECE%20r4132%20(source).7z

Do either of you still have this working?
Last edited by krcroft on 2018-7-20 @ 02:40, edited 1 time in total.
User avatar
krcroft
Member
 
Posts: 240
Joined: 2017-4-29 @ 15:07
Location: Ogden's Retreat

Re: FLAC support

Postby krcroft » 2018-7-20 @ 02:33

This post previous contained one of my first patches to add FLAC support to DOSBox.

Please see this post for the latest patch: viewtopic.php?f=32&t=62203
Last edited by krcroft on 2019-7-11 @ 14:18, edited 2 times in total.
User avatar
krcroft
Member
 
Posts: 240
Joined: 2017-4-29 @ 15:07
Location: Ogden's Retreat

Re: FLAC support

Postby Yesterplay80 » 2018-7-20 @ 07:21

krcroft wrote:Yesterplay80,
You mentioned your build now contains this (or maybe did at one point?), but I'm no longer seeing the changes to cdrom_image.cpp using getDuration in
Code: Select all
https://dosbox.yesterplay80.net/DOSBox%20ECE%20r4132%20(source).7z

Do either of you still have this working?

While FLAC playback worked, the patch(es) caused issues with some other games, which didnt playback any cd audio music any more, so I dropped FLAC support again.
My full-featured DOSBox SVN builds (without debugger) for Windows: Vanilla DOSBox and DOSBox ECE (Enhanced Community Edition)
User avatar
Yesterplay80
Member
 
Posts: 407
Joined: 2016-2-23 @ 11:02
Location: Germany

Re: FLAC support

Postby krcroft » 2018-7-20 @ 18:20

While FLAC playback worked, the patch(es) caused issues with some other games, which didnt playback any cd audio music any more, so I dropped FLAC support again.


Thanks for the heads up!
User avatar
krcroft
Member
 
Posts: 240
Joined: 2017-4-29 @ 15:07
Location: Ogden's Retreat

Previous

Return to DOSBox Development

Who is online

Users browsing this forum: No registered users and 1 guest