WIP: upgrade to SDL3 #103
Open
WIP: upgrade to SDL3 #103
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
superzazu
commented
Dec 21, 2024
superzazu
commented
Dec 21, 2024
superzazu
commented
Dec 21, 2024
superzazu
commented
Dec 21, 2024
superzazu
force-pushed
the
SDL3
branch
3 times, most recently
from
2c3e1bb to
c35a531
Compare
|
Here is a patch in And no, I don't think it is complete.. P.S.: The patch is incremental, i.e. it is against your branch.. |
Thank you! I added your commit to the branch |
You're welcome. Minor nit: --- a/src/SDL_sound.h
+++ b/src/SDL_sound.h
@@ -72,5 +72,5 @@
-#define SOUND_VER_MAJOR 2
+#define SOUND_VER_MAJOR 3
#define SOUND_VER_MINOR 0
-#define SOUND_VER_PATCH 3
+#define SOUND_VER_PATCH 0
#endif |
|
For the tls thing, maybe do what sdl2-compat does? Like the following? (not tested) diff --git a/src/SDL_sound.c b/src/SDL_sound.c
index bacfc02..461e374 100644
--- a/src/SDL_sound.c
+++ b/src/SDL_sound.c
@@ -94,2 +94,15 @@ typedef struct
+static SDL_TLSID SDL_TLSCreate(void)
+{
+ SDL_TLSID tlsid;
+ tlsid.value = 0; /* zero, so that SDL3 creates a new TLSID. */
+ /* this will create the new ID when it sees a zero, and just set a
+ harmless NULL in the slot for this thread, which is what SDL_TLSGet
+ will return if nothing was ever set. */
+ if (!SDL_SetTLS(&tlsid, NULL, NULL)) {
+ return 0; /* probably out of memory. */
+ }
+ return tlsid.value;
+}
+
static Sound_Sample *sample_list = NULL; /* this is a linked list. */
@@ -129,3 +142,3 @@ int Sound_Init(void)
- // tlsid_errmsg = SDL_TLSCreate();
+ tlsid_errmsg = SDL_TLSCreate();
@@ -176,3 +189,3 @@ int Sound_Quit(void)
- // tlsid_errmsg = 0;
+ tlsid_errmsg = 0;
|
superzazu
commented
Dec 22, 2024
|
P.S.: I created a new |
Good idea! It does not compile, but this does: diff --git a/src/SDL_sound.c b/src/SDL_sound.c
index bacfc02..aa829b2 100644
--- a/src/SDL_sound.c
+++ b/src/SDL_sound.c
@@ -127,7 +127,8 @@ int Sound_Init(void)
SDL_InitSubSystem(SDL_INIT_AUDIO);
- // tlsid_errmsg = SDL_TLSCreate();
+ tlsid_errmsg.value = 0;
+ BAIL_IF_MACRO(!SDL3_SetTLS(&tlsid_errmsg, NULL, NULL), ERR_OUT_OF_MEMORY, 0);
samplelist_mutex = SDL_CreateMutex();
@@ -174,7 +175,7 @@ int Sound_Quit(void)
SDL_free((void *) available_decoders);
available_decoders = NULL;
- // tlsid_errmsg = 0;
+ tlsid_errmsg.value = 0;
return 1;
} /* Sound_Quit */ |
|
For MSVC build failures, try this: diff --git a/src/libmodplug/load_mt2.c b/src/libmodplug/load_mt2.c
index c48d9f3..3e22129 100644
--- a/src/libmodplug/load_mt2.c
+++ b/src/libmodplug/load_mt2.c
@@ -358,8 +358,8 @@ BOOL CSoundFile_ReadMT2(CSoundFile *_this, LPCBYTE lpStream, DWORD dwMemLength)
}
}
// Load Instruments
- SDL_memset(InstrMap, 0, sizeof(InstrMap));
_this->m_nInstruments = (pfh->wInstruments < MAX_INSTRUMENTS) ? pfh->wInstruments : MAX_INSTRUMENTS-1;
+ for (j=0; j< 255; j++) InstrMap[j]=NULL;
for (j=1; j<=255; j++)
{
const MT2INSTRUMENT *pmi;
@@ -493,8 +493,8 @@ BOOL CSoundFile_ReadMT2(CSoundFile *_this, LPCBYTE lpStream, DWORD dwMemLength)
dwMemPos += 36;
}
}
- SDL_memset(SampleMap, 0, sizeof(SampleMap));
_this->m_nSamples = (pfh->wSamples < MAX_SAMPLES) ? pfh->wSamples : MAX_SAMPLES-1;
+ for (j=0; j< 256; j++) SampleMap[j]=NULL;
for (j=1; j<=256; j++)
{
const MT2SAMPLE *pms; |
I tried it ; but it looks like I did something wrong in the workflow file for ARM64 & x86: |
@madebr to rescue.. |
|
I guess the problem is with the cross-compilation to x86/ARM64: when building the project, it looks like CMake still tries to build for x64 (it finds the x86 SDL lib but says it can't find a compatible version) |
|
@superzazu: Can you please rebase to current main: I hand-picked three commits, so only the conflicts will be (1) the UTF-8'ization in SDL_sound_aiff.c which is now in, (2) the memset change to libmodplug/load_mt2.c which is now in, and (3) the cmake_minimum_required() statement which had a change |
Done! EDIT: I had to add |
Do the disabled msvc builds work now? |
Ugh, looks like they don't. Let's keep it that way though, so that @madebr might help. |
|
This seems to revert changes to load_mt2.c. May I force-push to your branch a clean & squashed version of this? |
Ah yes, it's because of the commit 0193ce4 I've made before the rebase to revert the changes. You can either revert this commit or force-push, as you'd like! |
|
OK, squashed and force-pushed: please remember to refresh your local source (or do a fresh clone.) |
Do you want it? (CC: @icculus)
Applied the patch and force-pushed |
Hm, failing again: |
|
Try also replacing |
madebr
reviewed
Dec 22, 2024
madebr
reviewed
Dec 22, 2024
madebr
reviewed
Dec 22, 2024
madebr
reviewed
Dec 22, 2024
madebr
reviewed
Dec 22, 2024
madebr
reviewed
Dec 22, 2024
|
Failing again (Didi I apply your suggestions correctly?) |
madebr
reviewed
Dec 22, 2024
| set_target_properties(SDL2_sound PROPERTIES MACOSX_RPATH 1) | ||
| set_target_properties(SDL2_sound PROPERTIES VERSION ${SDLSOUND_VERSION}) | ||
| set_target_properties(SDL2_sound PROPERTIES SOVERSION ${SDLSOUND_SOVERSION}) | ||
| add_library(SDL3_sound SHARED ${SDLSOUND_ALL_SRCS} ${SDLSOUND_SHARED_SRCS}) |
There was a problem hiding this comment.
SDL3 and all SDL3 satellite libraries create a SDL_xxx::SDL_xxx-shared and SDL_xxx::SDL_xxx-static target,
with SDL_xxx::SDL_xxx an alias to one of these (depending on what is available).
Should SDL_sound do the same?
There was a problem hiding this comment.
Really don't know. I'm not much of a cmake guy as you know :)
There was a problem hiding this comment.
No problem, you can do a lot with Makefiles ;)
#103 (comment) is missing |
Oops, done. |
.. and CI is green. Yey !!! |
|
OK, there are still unresolved suggestions from you: I or @superzazu can do those later, or if you get write permission from @icculus we can leave those to you. Thanks! |
|
I sent an invite. I've not caught up on this thread yet, but I'll try to soon. |
sezero
force-pushed
the
SDL3
branch
2 times, most recently
from
f207042 to
ec6d222
Compare
|
I cleaned up several stuff, squashed and rebased to current main: Removed old SDL2 version checks, replaced SIMDRealloc from the one in sdl2-compat, replaced rand/srand use with new SDL3 calls. (Maybe a few other things..) |
|
For diff --git a/src/SDL_sound_shn.c b/src/SDL_sound_shn.c
index d2e0b79..e705a2b 100644
--- a/src/SDL_sound_shn.c
+++ b/src/SDL_sound_shn.c
@@ -402,11 +402,11 @@ static SDL_INLINE Uint16 cvt_shnftype_to_sdlfmt(Sint16 shntype)
case SHN_TYPE_S16LH:
return SDL_AUDIO_S16LE;
- // case SHN_TYPE_U16HL:
- // return AUDIO_U16MSB;
+ case SHN_TYPE_U16HL:
+ return SDL_AUDIO_S16BE;
- // case SHN_TYPE_U16LH:
- // return AUDIO_U16LSB;
+ case SHN_TYPE_U16LH:
+ return SDL_AUDIO_S16LE;
} /* switch */
return 0;
@@ -919,18 +919,18 @@ static Uint32 put_to_buffers(Sound_Sample *sample, Uint32 bw)
case SHN_TYPE_U16HL:
case SHN_TYPE_U16LH:
{
- Uint16 *writebufp = (Uint16 *) shn->backBuffer;
+ Sint16 *writebufp = (Sint16 *) shn->backBuffer;
if (shn->nchan == 1)
{
for (i = 0; i < nitem; i++)
- *writebufp++ = CAPMAXUSHORT(data0[i]);
+ *writebufp++ = CAPMAXUSHORT(data0[i]) ^ 0x8000;
} /* if */
else
{
for (i = 0; i < nitem; i++)
{
for (chan = 0; chan < shn->nchan; chan++)
- *writebufp++ = CAPMAXUSHORT(shn->buffer[chan][i]);
+ *writebufp++ = CAPMAXUSHORT(shn->buffer[chan][i]) ^ 0x8000;
} /* for */
} /* else */
} /* case */ |
OK, pushed my shn patch for now. |
|
About timidity.c, do_song_load() buffer_size: @icculus did the same thing in SDL_mixer, so we should be OK I guess. |
Co-authored-by: Anonymous Maarten <[email protected]> Co-authored-by: Ozkan Sezer <[email protected]>
|
@icculus: This should be ready for review. WAV, AIFF, MP3 & OGG files briefly tested; VOC should work after #104 fix which this PR is already rebased to. Remaining are, if I'm not missing anything, AU and SHN testing. Edit: MIDI files briefly tested too (remember to configure with |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hello!
I worked on this for one of my projects, and I figured I might as well open a PR, even if the work is not finished. For now, the focus was to get it to work with SDL3 while minimising changes (that means API changes such as returning bools instead of ints etc. was not done).
To do list:
SHN_TYPE_U16**?timidity.c,do_song_load(), buffer size is incorrect (was taken from AudioSpec), I've hardcoded it to 4096 (to make it compile)MSVC (x86)andMSVC (ARM64)builds do not workSome things I would like to point out:
SDL_TLS**&SDL_SIMD**functions, so there's a good chance I made some mistakes thereSDL_RWFromFPhas been removed)Have a good day!