cdrom code #32
Comments
|
I was looking at this last night, because at some point the ScummVM team told me not having CD support was a deal breaker, but we can't just paste that code in, and in some cases, it wouldn't be helpful (for example, Apple doesn't sell Macs with CD-ROM drives anymore, and for years before, the audio cable wasn't hooked up, so you couldn't just tell an audio CD to play in one...iTunes would play CDs by reading the raw data from the disc and playing it through CoreAudio, instead of just sending a command to the drive like these SDL APIs expect). Also it feels weird to think that implementing this properly means that half of sdl12-compat would be CD-ROM code for various platforms. :) Let's leave this open for now, but my suspicion is that modern hardware and a lack of readily-available discs is going to be the real problem in the future. Part of me thinks it could be cool if we could configure sdl12-compat to pretend a directory full of MP3s is an audio CD, though. |
|
Side note: libcdio could be used for cd-play, but it's GPLv3, not LGPL: |
I did it. :) It's WIP, but almost done. |
|
okay, I finished this work over the weekend. Tested with quakespasm and the original Quake 1 soundtrack ripped to mp3, and it seems to work pretty well. The only issue I see at the moment is that it doesn't have a way to specify data tracks, so for Quake to get the right music (which starts on track 2, past the game data at track 1), you have to copy a valid mp3 file to 0.mp3 to make sdl12-compat report tracks at the right place. This obviously should be fixed still, so I'm reopening this bug for now. |
|
Also we should probably start with 1.mp3 instead of 0.mp3 so it matches the track number, and maybe call these things "track1.mp3" instead. |
|
Are the static linkage issues fixed? (Haven't tested yet.)
I'd suggest using two-digits, like trackXX.mp3, starting with 01. |
Wrong choice of words - I meant windows linkage with -nostdlib |
|
Mingw builds using Makefile.mingw: Targeting i686: Targeting x86_64: Targeting i686 using really old mingw.org toolchain: Linkage succeeds for all three cases if I link to libgcc like: diff --git a/src/Makefile.mingw b/src/Makefile.mingw
index 085ffcb..b509c69 100644
--- a/src/Makefile.mingw
+++ b/src/Makefile.mingw
@@ -17,7 +17,7 @@ CPPFLAGS = -DDLL_EXPORT -DNDEBUG
CFLAGS = -O3 -Wall
LDFLAGS = -nostdlib -shared -Wl,--no-undefined -Wl,--enable-auto-image-base -Wl,--out-implib,$(LIB)
-LDLIBS = -lkernel32 -luser32
+LDLIBS = -lkernel32 -luser32 -static-libgcc -lgcc
LIB = libSDL.dll.a
DLL = SDL.dll |
|
If I use a gcc-7.5 / mingw-w64-v5.0.4 based toolchain, the missing symbol |
This is only safe because we only decode one mp3 at a time, but cleaning this up more properly would be good. Should prevent MSVC from inserting a call to _chkstk here.
Not yet. The chkstk thing happens if you have more than 4 kilobytes of local variables, so I’m cleaning out some big scratch arrays still.
Okay, we can do that. |
|
Using Makefile.vc, with VS2017-v15.9.36, Targeting i686: Targeting x86_64: Using Makefile.vc, with VS2005-sp1, Targeting i686: Targeting x86_64: Linking to |
|
As of commit bbebe7c, MinGW x64 builds seems to link. 32 bits (x86) is still trouble: Using a mingw-w64 toolchain: Using a really old mingw.org-based toolchain: |
|
Same with VS2017 x86 builds: |
|
So most (or all) of these are for SSE implementations of various math and casting operations, inserted by MSVC with the assumption that you're linking against their C runtime anyhow. Since we've seen that SDL 1.2 linked against the C runtime on Windows by default, we should probably just not link with /NODEFAULTLIB which I think will solve this. (But I am almost entirely ignorant of the politics of Windows C runtime compatibility, except that sometimes it's pretty complicated...but using whatever SDL 1.2 used here should be safe, right?) |
Doing that might allow linkage, yes. But, how does SDL2 x86 builds handle As for mingw, linking with libgcc fixes linkage (as I noted at #32 (comment))
A new msvcrXX.dll for every new iteration of MSVC since version 2003. |
See: #32 (comment) (CMake would need the same adjustment.)
|
FWIW, I tried linking with libcmt.lib with MSVC 2017 to satisfy missing |
Windows development in a nutshell. :) I'll fight with this more soon. |
|
My solution to msvc/x86 builds is making it not emit sse2 instructions, See the following patch: diff --git a/src/Makefile.vc b/src/Makefile.vc
index bdfe142..8a2cd8e 100644
--- a/src/Makefile.vc
+++ b/src/Makefile.vc
@@ -1,5 +1,8 @@
# Makefile for Win32 using MSVC:
# nmake /f Makefile.vc
+#
+# If you specifically want to build for x86:
+# nmake /f Makefile.vc CPU=x86
# change INCLUDES so it points to SDL2 headers directory:
INCLUDES = -Iinclude
@@ -14,6 +17,10 @@ CFLAGS = /nologo /O2 /MD /W3 /GS-
LDFLAGS = /nologo /DLL /NODEFAULTLIB /RELEASE
LDLIBS = kernel32.lib user32.lib
+!if "$(CPU)" == "x86"
+CFLAGS = $(CFLAGS) /arch:SSE
+!endif
+
DLLNAME = SDL.dll
IMPNAME = SDL.lib
diff --git a/CMakeLists.txt b/CMakeLists.txt
index ea181a9..874f89f 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -38,6 +38,8 @@ set(SDL12COMPAT_SRCS
)
add_library(SDL SHARED ${SDL12COMPAT_SRCS})
+include(CheckCSourceCompiles)
+
include(GNUInstallDirs)
include("cmake/modules/FindSDL2.cmake")
target_include_directories(SDL PRIVATE ${SDL2_INCLUDE_DIRS})
@@ -83,7 +85,16 @@ if(MINGW)
endif()
if(MSVC)
# Don't try to link with the default set of libraries.
- set_target_properties(SDL PROPERTIES COMPILE_FLAGS "/GS-")
+ set(MSVC_FLAGS "/GS-")
+ check_c_source_compiles("int main(void) {
+#ifndef _M_IX86
+#error not x86
+#endif
+return 0; }" IS_X86)
+ if(IS_X86) # don't emit SSE2 in x86 builds
+ set(MSVC_FLAGS "${MSVC_FLAGS} /arch:SSE")
+ endif()
+ set_target_properties(SDL PROPERTIES COMPILE_FLAGS ${MSVC_FLAGS})
set_target_properties(SDL PROPERTIES LINK_FLAGS "/NODEFAULTLIB")
# Make sure /RTC1 is disabled: (from SDL2 CMake)
foreach(flag_var
diff --git a/src/SDL12_compat.c b/src/SDL12_compat.c
index a3c3d3a..e17c1b5 100644
--- a/src/SDL12_compat.c
+++ b/src/SDL12_compat.c
@@ -5940,6 +5940,10 @@ SDL_LoadWAV_RW(SDL12_RWops *rwops12, int freerwops12,
an audio CD-ROM, and will decode these files and mix them into an audio
stream as if they were playing from a disc. */
+#if defined(_MSC_VER) && defined(_M_IX86)
+#include "x86_msvc.h"
+#endif
+
#define CDAUDIO_FPS 75 /* CD audio frames per second. */
/* public domain, single-header MP3 decoder for fake CD-ROM audio support! */And here is the Tested build using both Makefile.vc and cmake, using VS2005 and VS2017. |
|
Looks fine to me, commit it, please. |
|
Pushed as d1f3c6e |
|
So the only thing still pending on this is to let people specify fake data tracks (quake 1 had the audio start at track 2 for example), I think. |
IMO, we shouldn't concern ourselves with at which track the cdaudio As for what's left to do: I even had thought about modifying drmp3 |
|
One thing to make sure is 'close'ing audio does close cdrom properly, and Also, does it work if |
Is the following OK to apply? diff --git a/src/SDL12_compat.c b/src/SDL12_compat.c
index ee352a3..ee21404 100644
--- a/src/SDL12_compat.c
+++ b/src/SDL12_compat.c
@@ -6169,7 +6169,8 @@ SDL_CDNumDrives(void)
warned_once = SDL_TRUE;
SDL20_Log("This app is looking for CD-ROM drives, but no path was specified");
SDL20_Log("Set the SDL12COMPAT_FAKE_CDROM_PATH environment variable to a directory");
- SDL20_Log("of MP3 files named NUM.mp3, where NUM is a track number from 0 to 99");
+ SDL20_Log("of MP3 files named trackXX.mp3 where XX is a track number in two digits");
+ SDL20_Log("from 00 to 99");
}
}
@@ -6243,10 +6244,13 @@ SDL_CDOpen(int drive)
drmp3_uint64 pcmframes;
drmp3_uint32 samplerate;
SDL12_CDtrack *track;
+ char c0, c1;
/* we only report audio tracks, starting at 0... */
FIXME("Let there be fake data tracks (quake1's audio starts at track 2, etc).");
- SDL20_snprintf(fullpath, alloclen, "%s%s%d.mp3", CDRomPath, DIRSEP, retval->numtracks);
+ c0 = retval->numtracks / 10 + '0';
+ c1 = retval->numtracks % 10 + '0';
+ SDL20_snprintf(fullpath, alloclen, "%s%strack%c%c.mp3", CDRomPath, DIRSEP, c0, c1);
rw = SDL20_RWFromFile(fullpath, "rb");
if (!rw) {
break; /* ok, we're done looking for more. */ |
|
Let's start from 1 instead of zero, but yeah, apply this. |
Applied as ffc3d79 |
|
I seem to have screwed up with numtracks value. Will push a fix shortly. |
|
Pushed 67b5a5b to fix things. |
I think verifying this is the only pending item at this point. (I wouldn't risk removing 64-bit stuff from dr_mp3, as it's going to cause problems if we ever update that code. I need to send them patches for the changes I already made, in hopes of avoiding that problem.) |
|
This is the code I used to test the CD and OpenAudio stuff (and it did, in fact, reveal a bug)...it's basically loopwave.c with some changes. This is not super-robust, but basically make sure you have #include "SDL_config.h"
#include <stdio.h>
#include <stdlib.h>
#include "SDL.h"
#include "SDL_audio.h"
struct {
SDL_AudioSpec spec;
Uint8 *sound; /* Pointer to wave data */
Uint32 soundlen; /* Length of wave data */
int soundpos; /* Current play position */
} wave;
/* Call this instead of exit(), so we can clean up SDL: atexit() is evil. */
static void quit(int rc)
{
SDL_Quit();
exit(rc);
}
void SDLCALL fillerup(void *unused, Uint8 *stream, int len)
{
Uint8 *waveptr;
int waveleft;
/* Set up the pointers */
waveptr = wave.sound + wave.soundpos;
waveleft = wave.soundlen - wave.soundpos;
/* Go! */
while ( waveleft <= len ) {
SDL_memcpy(stream, waveptr, waveleft);
stream += waveleft;
len -= waveleft;
waveptr = wave.sound;
waveleft = wave.soundlen;
wave.soundpos = 0;
}
SDL_memcpy(stream, waveptr, len);
wave.soundpos += len;
}
static int done = 0;
void poked(int sig)
{
done = 1;
}
int main(int argc, char *argv[])
{
char name[32];
const char *file;
int done = 0;
SDL_CD *cdrom = NULL;
/* Load the SDL library */
if ( SDL_Init(SDL_INIT_AUDIO | SDL_INIT_VIDEO | SDL_INIT_CDROM) < 0 ) {
fprintf(stderr, "Couldn't initialize SDL: %s\n",SDL_GetError());
return(1);
}
printf("Using audio driver: %s\n", SDL_AudioDriverName(name, 32));
file = (argc < 2) ? "sample.wav" : argv[1];
/* Load the wave file into memory */
if ( SDL_LoadWAV(file, &wave.spec, &wave.sound, &wave.soundlen) == NULL ) {
fprintf(stderr, "Couldn't load %s: %s\n", file, SDL_GetError());
quit(1);
}
wave.spec.callback = fillerup;
if (!SDL_SetVideoMode(640, 480, 0, 0)) {
fprintf(stderr, "Couldn't SetVideoMode: %s\n", SDL_GetError());
quit(1);
}
while (!done) {
SDL_Event e;
while (SDL_PollEvent(&e)) {
if (e.type == SDL_QUIT) {
done = 1;
} else if (e.type == SDL_KEYDOWN) {
if (e.key.keysym.sym == SDLK_a) {
if ( SDL_OpenAudio(&wave.spec, NULL) < 0 ) {
fprintf(stderr, "Couldn't open audio: %s\n", SDL_GetError());
SDL_FreeWAV(wave.sound);
quit(2);
}
SDL_PauseAudio(0);
} else if (e.key.keysym.sym == SDLK_s) {
SDL_CloseAudio();
} else if (e.key.keysym.sym == SDLK_c) {
if (!cdrom) {
cdrom = SDL_CDOpen(0);
if (!cdrom) {
fprintf(stderr, "Couldn't open cdrom: %s\n", SDL_GetError());
SDL_FreeWAV(wave.sound);
quit(2);
}
if ( !CD_INDRIVE(SDL_CDStatus(cdrom)) ) {
fprintf(stderr, "No CD in drive: %s\n", SDL_GetError());
SDL_FreeWAV(wave.sound);
quit(2);
}
if ( SDL_CDPlayTracks(cdrom, 0, 0, 0, 0) < 0 ) {
fprintf(stderr, "Failed to play CD: %s\n", SDL_GetError());
SDL_FreeWAV(wave.sound);
quit(2);
}
}
} else if (e.key.keysym.sym == SDLK_v) {
SDL_CDClose(cdrom);
cdrom = NULL;
} else if (e.key.keysym.sym == SDLK_q) {
done = 1;
}
}
}
SDL_Delay(100);
}
/* Clean up on signal */
if (cdrom) { SDL_CDClose(cdrom); }
SDL_CloseAudio();
SDL_FreeWAV(wave.sound);
SDL_Quit();
return(0);
} |
We have stubs for cdrom code in sdl12-compat.
Should we import the code from SDL-1.2? If we ever do that,
would there be licence issues (can any of the authors still be
reached, etc)?
If we'll keep the stubs, close this.
The text was updated successfully, but these errors were encountered: