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: Re: Bug#718129: Re: Re: Bug#715461: libsdl-mixer1.2: no sf2 sound fonts loaded by default
On Monday 12 August 2013 00:29:34 Manuel A. Fernandez Montecelo wrote: So... does this look OK to both of you (I didn't actually upload, waiting for your confirmation)? Looks good. SDL-perl tests are fine with your updated patch All the best signature.asc Description: This is a digitally signed message part.
Bug#715461: Re: Bug#718129: Re: Re: Bug#715461: libsdl-mixer1.2: no sf2 sound fonts loaded by default
2013/8/12 Dominique Dumont d...@debian.org: On Monday 12 August 2013 00:29:34 Manuel A. Fernandez Montecelo wrote: So... does this look OK to both of you (I didn't actually upload, waiting for your confirmation)? Looks good. SDL-perl tests are fine with your updated patch All the best Updating then, thanks! -- 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
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
Bug#715461: Re: Re: Bug#715461: libsdl-mixer1.2: no sf2 sound fonts loaded by default
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 signature.asc Description: This is a digitally signed message part.
Bug#715461: Re: Bug#715461: libsdl-mixer1.2: no sf2 sound fonts loaded by default
On Wednesday 07 August 2013 22:13:49 you wrote: For example, one fix that comes to mind is to change the line in the first patch: char* soundfont_paths = /usr/share/sounds/sf2/TimGM6mb.sf2:/usr/share/sounds/sf2/FluidR3_GM.sf2; to this: char* soundfont_paths = SDL_strdup(/usr/share/sounds/sf2/TimGM6mb.sf2:/usr/share/sounds/sf2/FluidR3 _GM.sf2); What do you think? Feels less intrusive than having a second patch. ok to reduce the number of patches. But the SDL_strdup solution is needlessly complicated and will probably have some eyebrows raised very high in the future. I'd rather see bug-718129-rm-bad-free.patch merged into bug-715461- soundfont_paths.patch so as to have one simple, correct patch. All the best signature.asc Description: This is a digitally signed message part.
Bug#715461: Re: Bug#715461: libsdl-mixer1.2: no sf2 sound fonts loaded by default
2013/8/8 Dominique Dumont d...@debian.org: On Wednesday 07 August 2013 22:13:49 you wrote: For example, one fix that comes to mind is to change the line in the first patch: char* soundfont_paths = /usr/share/sounds/sf2/TimGM6mb.sf2:/usr/share/sounds/sf2/FluidR3_GM.sf2; to this: char* soundfont_paths = SDL_strdup(/usr/share/sounds/sf2/TimGM6mb.sf2:/usr/share/sounds/sf2/FluidR3 _GM.sf2); What do you think? Feels less intrusive than having a second patch. ok to reduce the number of patches. But the SDL_strdup solution is needlessly complicated and will probably have some eyebrows raised very high in the future. I'd rather see bug-718129-rm-bad-free.patch merged into bug-715461- soundfont_paths.patch so as to have one simple, correct patch. I don't know if my intentions were clear. 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. I think that this fits the simple, correct patch idea that you mention, and I don't see anything complicated about it -- it's just to assign the variable with dynamic memory, which is the way the rest of the code thinks that it should be (there are more instances trying to free memory from this varaible). 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). Still, if anybody thinks that other solutions are preferrable, it's OK by me -- I have no special interest in pushing this solution over others. I volunteer to fix this, no matter the solution chosen, if nobody else wants. 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. Cheers. [1] music.c, int Mix_SetSoundFonts(const char *paths) -- 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: AW: Re: Re: Bug#715461: libsdl-mixer1.2: no sf2 sound fonts loaded by default
As the one who introduced that bug: please go ahead, your solution looks right to me. ;-) Von Samsung Mobile gesendet Ursprüngliche Nachricht Von: Manuel A. Fernandez Montecelo manuel.montez...@gmail.com Datum: An: Dominique Dumont d...@debian.org Cc: Fabian Greffrath fab...@greffrath.com,715...@bugs.debian.org,718...@bugs.debian.org Betreff: Re: Re: Bug#715461: libsdl-mixer1.2: no sf2 sound fonts loaded by default 2013/8/8 Dominique Dumont d...@debian.org: On Wednesday 07 August 2013 22:13:49 you wrote: For example, one fix that comes to mind is to change the line in the first patch: char* soundfont_paths = /usr/share/sounds/sf2/TimGM6mb.sf2:/usr/share/sounds/sf2/FluidR3_GM.sf2; to this: char* soundfont_paths = SDL_strdup(/usr/share/sounds/sf2/TimGM6mb.sf2:/usr/share/sounds/sf2/FluidR3 _GM.sf2); What do you think? Feels less intrusive than having a second patch. ok to reduce the number of patches. But the SDL_strdup solution is needlessly complicated and will probably have some eyebrows raised very high in the future. I'd rather see bug-718129-rm-bad-free.patch merged into bug-715461- soundfont_paths.patch so as to have one simple, correct patch. I don't know if my intentions were clear. 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. I think that this fits the simple, correct patch idea that you mention, and I don't see anything complicated about it -- it's just to assign the variable with dynamic memory, which is the way the rest of the code thinks that it should be (there are more instances trying to free memory from this varaible). 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). Still, if anybody thinks that other solutions are preferrable, it's OK by me -- I have no special interest in pushing this solution over others. I volunteer to fix this, no matter the solution chosen, if nobody else wants. 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. Cheers. [1] music.c, int Mix_SetSoundFonts(const char *paths) -- Manuel A. Fernandez Montecelo manuel.montez...@gmail.com
Bug#715461: libsdl-mixer1.2: no sf2 sound fonts loaded by default
Hi, For reference, I think that bug-715461-soundfont_paths.patch created the recent problem in #718129. Instead of adding another patch (the one fixing #718129, included in revision -7 of the package), perhaps the pach to include soundfonts can be modified to avoid having the second one. I think that, as Dominique mentioned, the problem is that the SDL_free() frees memory which has not been malloc()ed (SDL_malloc() perhaps?). For example, one fix that comes to mind is to change the line in the first patch: char* soundfont_paths = /usr/share/sounds/sf2/TimGM6mb.sf2:/usr/share/sounds/sf2/FluidR3_GM.sf2; to this: char* soundfont_paths = SDL_strdup(/usr/share/sounds/sf2/TimGM6mb.sf2:/usr/share/sounds/sf2/FluidR3_GM.sf2); What do you think? Feels less intrusive than having a second patch. 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: libsdl-mixer1.2: no sf2 sound fonts loaded by default
Am Mittwoch, den 07.08.2013, 22:13 +0100 schrieb Manuel A. Fernandez Montecelo: char* soundfont_paths = SDL_strdup(/usr/share/sounds/sf2/TimGM6mb.sf2:/usr/share/sounds/sf2/FluidR3_GM.sf2); What do you think? Feels less intrusive than having a second patch. Good idea! Feels much cleaner than having a second patch remove unrelated code. - 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: libsdl-mixer1.2: no sf2 sound fonts loaded by default
Am Donnerstag, den 11.07.2013, 14:47 +0100 schrieb Manuel A. Fernandez Montecelo: I'm going to upload the fixes now, would you please test if it's working as you expect when you have some time availale? I'd say it works exactly as expected. Thanks for the prompt upload! :) - 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: libsdl-mixer1.2: no sf2 sound fonts loaded by default
2013/7/12 Fabian Greffrath fab...@greffrath.com: Am Donnerstag, den 11.07.2013, 14:47 +0100 schrieb Manuel A. Fernandez Montecelo: I'm going to upload the fixes now, would you please test if it's working as you expect when you have some time availale? I'd say it works exactly as expected. Thanks for the prompt upload! :) Thanks to you for the fix! -- 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: libsdl-mixer1.2: no sf2 sound fonts loaded by default
Hi Fabian, Thanks for the reports and the fixes. 2013/7/9 Fabian Greffrath fab...@greffrath.com: Recommends: musescore-soundfont-gm, fluid-soundfont-gm, freepats This again is alternative dependencies, right? Recommends: musescore-soundfont-gm | fluid-soundfont-gm | freepats 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: libsdl-mixer1.2: no sf2 sound fonts loaded by default
Am Donnerstag, den 11.07.2013, 12:37 +0100 schrieb Manuel A. Fernandez Montecelo: This again is alternative dependencies, right? Recommends: musescore-soundfont-gm | fluid-soundfont-gm | freepats Yes, sure, sorry. I am still not sure, however, if fluid-soundfont-gm should get precedence over freepats as it is quite chunky and some users will be happy to hear music at all - regardless of its quality. Please note that the patch is just a quick hack to demonstrate that music playback with sdl_mixer using fluidsynth and a packaged sound font *could* be easy. I am not sure if it is already ready for application in Debian, maybe could could check that with upstream first? However, I find the current situation of being forced to set an environment variable first in order to get music playback of reasonable quality completely unsatisfying. - 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: libsdl-mixer1.2: no sf2 sound fonts loaded by default
2013/7/11 Fabian Greffrath fab...@greffrath.com: Am Donnerstag, den 11.07.2013, 12:37 +0100 schrieb Manuel A. Fernandez Montecelo: This again is alternative dependencies, right? Recommends: musescore-soundfont-gm | fluid-soundfont-gm | freepats Yes, sure, sorry. I am still not sure, however, if fluid-soundfont-gm should get precedence over freepats as it is quite chunky and some users will be happy to hear music at all - regardless of its quality. Please note that the patch is just a quick hack to demonstrate that music playback with sdl_mixer using fluidsynth and a packaged sound font *could* be easy. I am not sure if it is already ready for application in Debian, maybe could could check that with upstream first? However, I find the current situation of being forced to set an environment variable first in order to get music playback of reasonable quality completely unsatisfying. There are patches doing something similar with freepats for ages, so I think that it's sensible to try this route with better quality sound. At some point I would like to disable the internal timidity completely. I think that libsdl2-mixer already got rid of the GPL version of timidity. I'm going to upload the fixes now, would you please test if it's working as you expect when you have some time availale? 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: libsdl-mixer1.2: no sf2 sound fonts loaded by default
Am Donnerstag, den 11.07.2013, 14:47 +0100 schrieb Manuel A. Fernandez Montecelo: There are patches doing something similar with freepats for ages, so I think that it's sensible to try this route with better quality sound. Yes, I know that patch. ;) I'm going to upload the fixes now, would you please test if it's working as you expect when you have some time availale? Sure thing, please go ahead! - 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: libsdl-mixer1.2: no sf2 sound fonts loaded by default
Package: libsdl-mixer1.2 Version: 1.2.12-5 Severity: normal Hello, the current version of libsdl-mixer1.2 is able to use fluidsynth for midi playback, which is far superior compared to the embedded copy of timidity which relies on sound fonts in the obsolete pat format (and thus requires the freepats package to play music). However, fluidsynth requires sound fonts in the sf2 format to be able to play music. To achieve this, two things have to be changed in the current Debian package: 1) Debian currently ships three sf2 sound font files in the fluid-soundfont-gs, musescore-soundfont-gm and fluid-soundfont-gm packages, respectively. These packages should get promoted to Recommends with a higher priority than freepats: - Recommends: freepats + Recommends: musescore-soundfont-gm, fluid-soundfont-gm, freepats I have given the musescore-soundfont-gm package higher priority, because it is a complete set but has only 6MB compared to the 145Mb of fluid-soundfont-gm. fluid-soundfont-gs is out of this list, because I did not get it working without its bigger brother fluid-soundfont-gm installed alongside. 2) The list of available sound font files is currently set to NULL in music.c:148 and relies on the SDL_SOUNDFONTS environment variable to contain a list of appropriate files. However, it should get initialized with the known locations of the sf2 sound font files available in Debian: - char* soundfont_paths = NULL; + char* soundfont_paths = /usr/share/sounds/sf2/TimGM6mb.sf2:/usr/share/sounds/sf2/FluidR3_GM.sf2; This will enable high quality music playback *by default* (at least higher quality than using the internal timidity copy with freepats). - Fabian -- System Information: Debian Release: jessie/sid APT prefers stable APT policy: (900, 'stable'), (800, 'testing'), (700, 'unstable') Architecture: amd64 (x86_64) Foreign Architectures: i386 Kernel: Linux 3.9-1-amd64 (SMP w/4 CPU cores) Locale: LANG=de_DE.UTF-8, LC_CTYPE=de_DE.UTF-8 (charmap=UTF-8) Shell: /bin/sh linked to /bin/dash Versions of packages libsdl-mixer1.2 depends on: ii libc6 2.17-7 ii libflac8 1.3.0-1 ii libmad00.15.1b-8 ii libmikmod2 3.1.12-5 ii libsdl1.2debian1.2.15-5 ii libvorbis0a1.3.2-1.3 ii libvorbisfile3 1.3.2-1.3 ii multiarch-support 2.17-7 Versions of packages libsdl-mixer1.2 recommends: ii freepats 20060219-1 libsdl-mixer1.2 suggests no packages. -- no debconf information -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#715461: libsdl-mixer1.2: no sf2 sound fonts loaded by default
Am Dienstag, den 09.07.2013, 11:07 +0200 schrieb Fabian Greffrath: + Recommends: musescore-soundfont-gm, fluid-soundfont-gm, freepats Recommends: musescore-soundfont-gm | freepats Of course, these should be alternative dependencies. I have dropped fluid-soundfont-gm, see below: + char* soundfont_paths = /usr/share/sounds/sf2/TimGM6mb.sf2:/usr/share/sounds/sf2/FluidR3_GM.sf2; char* soundfont_paths = /usr/share/sounds/sf2/TimGM6mb.sf2; The Mix_EachSoundFont() function return()s when one of the passed sound font files cannot be handled. So instead of requiring both sound fonts to be installed, let's concentrate on one instead (I have decided for the smaller sound font) and let the user explicitely select the other one via the SDL_SOUNDFONTS variable. - 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: libsdl-mixer1.2: no sf2 sound fonts loaded by default
tags -1 + patch
Am Dienstag, den 09.07.2013, 11:48 +0200 schrieb Fabian Greffrath:
The Mix_EachSoundFont() function return()s when one of the passed sound
font files cannot be handled. So instead of requiring both sound fonts
to be installed, let's concentrate on one instead (I have decided for
the smaller sound font) and let the user explicitely select the other
one via the SDL_SOUNDFONTS variable.
The attached patch/hack fixes this. It iterates over both given sound
font files and uses the last one it finds. That is, if both are present,
the latter overrides the former (in this case, the bigger FluidR3_GM.sf2
overrides the smaller TimGM6mb.sf2). If none is present, it gracefully
falls back to its own internal timidity sound renderer and freepats.
The correct package relationship with this patch is thus:
Recommends: musescore-soundfont-gm, fluid-soundfont-gm, freepats
Hope that helps!
- Fabian
--- a/music.c
+++ b/music.c
@@ -145,7 +145,7 @@ static int num_decoders = 0;
/* Semicolon-separated SoundFont paths */
#ifdef MID_MUSIC
-char* soundfont_paths = NULL;
+char* soundfont_paths = /usr/share/sounds/sf2/TimGM6mb.sf2:/usr/share/sounds/sf2/FluidR3_GM.sf2;
#endif
int Mix_GetNumMusicDecoders(void)
@@ -1567,6 +1567,7 @@ int Mix_EachSoundFont(int (*function)(co
{
char *context, *path, *paths;
const char* cpaths = Mix_GetSoundFonts();
+ int atleastone = 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 {
+ atleastone++;
}
}
SDL_free(paths);
- return 1;
+ if (atleastone 0)
+ return 1;
+ else
+ return 0;
}
#endif
