Bug#715461: Bug#718129: Re: Re: Bug#715461: libsdl-mixer1.2: no sf2 sound fonts loaded by default
Hi, I just uploaded another fix for this in -9, thanks Fabian. http://anonscm.debian.org/gitweb/?p=pkg-sdl/packages/sdl-mixer1.2.git;a=commitdiff;h=396542188f997d14a066d8ddcb4007ca72667509 If you could check it when you have the opportunity and report here that everything works as expected, it would be great. Cheers. -- Manuel -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#715461: Bug#718129: Re: Re: Bug#715461: libsdl-mixer1.2: no sf2 sound fonts loaded by default
2013/9/1 Fabian Greffrath fab...@greffrath.com: In the patch for debian/rules the quotation marks have to get escaped. This somehow got lost by attaching it to my mail. Sorry for the delay but I'm quite busy at the moment. I will try to review and get this new patch integrated soonish. Thanks. Cheers. -- Manuel A. Fernandez Montecelo manuel.montez...@gmail.com -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#715461: Bug#718129: Re: Re: Bug#715461: libsdl-mixer1.2: no sf2 sound fonts loaded by default
Am Freitag, den 30.08.2013, 11:50 +0100 schrieb Manuel A. Fernandez
Montecelo:
Do you think that it could be modified to be less system dependent, in
a way that it would be accepted upstream? Perhaps passing it at
compilation time, instead of having to patch or export at runtime?
How about the attached modified patch and also the one for debian/rules?
- Fabian
Description: no sf2 sound fonts loaded by default
Introduced in 1.2.12-6 (Thu, 11 Jul 2013 12:17:15 +0100)
Author: Fabian Greffrath fab...@greffrath.com
Last-Update: 2013-08-11
Bug-Debian: http://bugs.debian.org/715461
--- a/mixer.c
+++ b/mixer.c
@@ -148,6 +148,11 @@ int Mix_Init(int flags)
{
int result = 0;
+#ifdef MIX_INIT_SOUNDFONT_PATHS
+ if (!soundfont_paths)
+ soundfont_paths = SDL_strdup(MIX_INIT_SOUNDFONT_PATHS);
+#endif
+
if (flags MIX_INIT_FLUIDSYNTH) {
#ifdef USE_FLUIDSYNTH_MIDI
if ((initialized MIX_INIT_FLUIDSYNTH) || Mix_InitFluidSynth() == 0) {
--- a/music.c
+++ b/music.c
@@ -1567,6 +1567,7 @@ int Mix_EachSoundFont(int (*function)(co
{
char *context, *path, *paths;
const char* cpaths = Mix_GetSoundFonts();
+ int soundfonts_found = 0;
if (!cpaths) {
Mix_SetError(No SoundFonts have been requested);
@@ -1586,12 +1587,16 @@ int Mix_EachSoundFont(int (*function)(co
for (path = strtok_r(paths, :;, context); path; path = strtok_r(NULL, :;, context)) {
#endif
if (!function(path, data)) {
- SDL_free(paths);
- return 0;
+ continue;
+ } else {
+ soundfonts_found++;
}
}
SDL_free(paths);
- return 1;
+ if (soundfonts_found 0)
+ return 1;
+ else
+ return 0;
}
#endif
diff -Nru sdl-mixer1.2-1.2.12/debian/rules sdl-mixer1.2-1.2.12/debian/rules
--- sdl-mixer1.2-1.2.12/debian/rules 2013-07-11 13:33:46.0 +0200
+++ sdl-mixer1.2-1.2.12/debian/rules 2013-09-01 16:25:19.0 +0200
@@ -3,6 +3,7 @@
#export DH_VERBOSE=1
export DEB_CFLAGS_MAINT_APPEND = -pipe -Wall
+export DEB_CPPFLAGS_MAINT_APPEND = -DMIX_INIT_SOUNDFONT_PATHS=/usr/share/sounds/sf2/TimGM6mb.sf2:/usr/share/sounds/sf2/FluidR3_GM.sf2
export DEB_LDFLAGS_MAINT_APPEND = -Wl,--no-undefined -Wl,-Bsymbolic -Wl,--as-needed
Bug#715461: Bug#718129: Re: Re: Bug#715461: libsdl-mixer1.2: no sf2 sound fonts loaded by default
In the patch for debian/rules the quotation marks have to get escaped. This somehow got lost by attaching it to my mail. - Fabian -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#715461: Bug#718129: Re: Re: Bug#715461: libsdl-mixer1.2: no sf2 sound fonts loaded by default
2013/8/26 Fabian Greffrath fab...@greffrath.com: Am Dienstag, den 20.08.2013, 09:58 +0200 schrieb Fabian Greffrath: Yes, it does. However, I would have added a check if the pointer is already set prior to resetting it, e.g. if (!soundfont_paths) soundfont_paths = SDL_strdup(...); But this is really just nit-picking. Wait, does SDL_strdup() allocate new memory and isn't the current solution leaking memory when Mix_Init() is repeatedly called? Yes, I don't think that it will be a real issue except if programs do real weird things (even the tests are not likely to exhaust any memory doing this). But it can be fixed properly anyway, just didn't find the time to do that. What it concerns me more is to have to carry the patch around, modifying it for SDL2, etc. Do you think that it could be modified to be less system dependent, in a way that it would be accepted upstream? Perhaps passing it at compilation time, instead of having to patch or export at runtime? Would they be interested in something like this at all? Cheers. -- Manuel A. Fernandez Montecelo manuel.montez...@gmail.com -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#715461: Bug#718129: Re: Re: Bug#715461: libsdl-mixer1.2: no sf2 sound fonts loaded by default
Am Dienstag, den 20.08.2013, 09:58 +0200 schrieb Fabian Greffrath: Yes, it does. However, I would have added a check if the pointer is already set prior to resetting it, e.g. if (!soundfont_paths) soundfont_paths = SDL_strdup(...); But this is really just nit-picking. Wait, does SDL_strdup() allocate new memory and isn't the current solution leaking memory when Mix_Init() is repeatedly called? - Fabian -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#715461: Bug#718129: Re: Re: Bug#715461: libsdl-mixer1.2: no sf2 sound fonts loaded by default
Sorry for my late response, I have been on vacation last week. Am Montag, den 12.08.2013, 00:29 +0100 schrieb Manuel A. Fernandez Montecelo: So... does this look OK to both of you (I didn't actually upload, waiting for your confirmation)? Yes, it does. However, I would have added a check if the pointer is already set prior to resetting it, e.g. if (!soundfont_paths) soundfont_paths = SDL_strdup(...); But this is really just nit-picking. Thanks for the fix! - Fabian -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#715461: Bug#718129: Re: Re: Bug#715461: libsdl-mixer1.2: no sf2 sound fonts loaded by default
2013/8/9 Dominique Dumont d...@debian.org: On Thursday 08 August 2013 19:13:24 Manuel A. Fernandez Montecelo wrote: I meant to modify the first patch bug-715461-soundfont_paths.patch so when that variable soundfont_paths is assigned, it's done with SDL_strdup() (it's done in several places in the code --that's where I got the idea from--, so it fits), and remove the second patch altogether, bug-718129-rm-bad-free.patch. Understood. The variable can be set by users of the library to use dynamic memory [1], so removing that SDL_free() is theoretically incorrect -- if it gets assigned other content in runtime, it would not free it where the SDL_free() is removed (which is the end of the program, so actually it shoudn't be that important, bug e.g. valgrind would report it as a leak). I think soundfont_paths initialisation should be done in Mix_Init(). Otherwise a sequence of Mix_Init, Mix_Quit, Mix_Init and Mix_Quit will lead to a segfault. This sequence may not make sense from a user's point of view, but it may happen in test suites like SDL-perl's test suite. And Dominique, sorry that I didn't catch this when you asked me, I was busy at work and couldn't pay full attention to the issue. Don't worry about it. Been there, done that ;-) All the best So... does this look OK to both of you (I didn't actually upload, waiting for your confirmation)? The commit diff is needlessly long due to issues with dos/unix line ending, but I guess that you get the idea comparing previous and current versions of the patch. commit diff: http://anonscm.debian.org/gitweb/?p=pkg-sdl/packages/sdl-mixer1.2.git;a=commitdiff;h=ef1b4991e313ea2ef840af3291e6e8b77f9e60be current version of the patch: http://anonscm.debian.org/gitweb/?p=pkg-sdl/packages/sdl-mixer1.2.git;a=blob;f=debian/patches/bug-715461-soundfont_paths.patch;h=bf2fe632a3ea38de28a079138e7ab3627d7fdd57;hb=ef1b4991e313ea2ef840af3291e6e8b77f9e60be previous version: http://anonscm.debian.org/gitweb/?p=pkg-sdl/packages/sdl-mixer1.2.git;a=blob;f=debian/patches/bug-715461-soundfont_paths.patch;h=71faacc115df02713bae2e6211368bc24d2577cc;hb=06b5b0ea8a1417ca356e5055ce5855df8dbfcbd6 Cheers. -- Manuel A. Fernandez Montecelo manuel.montez...@gmail.com -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
