Package: release.debian.org
Severity: normal
User: release.debian....@packages.debian.org
Usertags: unblock
X-Debbugs-Cc: t...@mirbsd.de
Please unblock package musescore2 and musescore3
[ Reason ]
In #985129 a soundfont-dependent crash was reported. Upon further
debugging and inspection of the code as well as discussion with
upstream, multiple failures in the handling of soundfonts was found
which I fixed, adapting the code to honour the SoundFont specification
in the ways I found possible. Some of the issues had been known, but
not fixed yet. (Mostly the mismatch between end and end-1.)
Further discussion with the developers of FluidSynth, an old version
of which the MuseScore built-in synthesiser “fluid” is based on, and
with the developer of MuseScore’s principal soundfont resulted in
further refining the handling of bad {,loop}{start,end} values, as
to better match Fluid’s playback capabilities as well as real-existing
soundfonts.
There were also places where unsigned integers need to be used as
soundfonts can be up to 4 GiB in size. (One place uses size_t instead,
this is a backport of an upstream change; size_t is neither less nor
more correct here.)
This is a patch that applies to both source packages identically
(with very slight changes in the context of some lines only).
[ Impact ]
Rare arbitrary unreproducible crashes, and incorrect playback of some
real-existing but not spec-compliant soundfonts.
[ Tests ]
I’ve tested the patched versions myself for a bit and had developers
of both upstream and FluidSynth look at it. An upstream developer was
also testing it but had mixed feedback because of a bug he himself had
accidentally introduced in his tree, but other than that the feedback
was good.
[ Risks ]
The diff looks complex but in actuality simplifies the code in question
and makes it more easy to read (modulo upstream’s weird indentation
style). The code is duplicated, because SF3 soundfonts handle the checks
at a different point in the application lifecycle than SF2 ones due to
compression coming into play, but I ensured it’s a text editor block copy,
indent level increase, and the addition of p-> everywhere *only*.
These are leaf packages.
The PR has not yet been merged upstream, but I consider it low risk for
Debian nevertheless.
[ Checklist ]
[x] all changes are documented in the d/changelog
[x] I reviewed all changes and I approve them
[x] attach debdiff against the package in testing
[ Other info ]
unblock musescore2/2.3.2+dfsg4-15
unblock musescore3/3.2.3+dfsg2-11
diff -Nru musescore2-2.3.2+dfsg4/debian/changelog
musescore2-2.3.2+dfsg4/debian/changelog
--- musescore2-2.3.2+dfsg4/debian/changelog 2021-03-05 17:57:30.000000000
+0100
+++ musescore2-2.3.2+dfsg4/debian/changelog 2021-06-05 18:04:54.000000000
+0200
@@ -1,3 +1,9 @@
+musescore2 (2.3.2+dfsg4-15) unstable; urgency=medium
+
+ * Fix soundfont-related crashes or audible artefacts (Closes: #985129)
+
+ -- Thorsten Glaser <t...@mirbsd.de> Sat, 05 Jun 2021 18:04:54 +0200
+
musescore2 (2.3.2+dfsg4-14) unstable; urgency=medium
* Fix possible error cause in m-common.prerm (Closes: #984592)
diff -Nru
musescore2-2.3.2+dfsg4/debian/patches/experiments/valid-soundfont.diff
musescore2-2.3.2+dfsg4/debian/patches/experiments/valid-soundfont.diff
--- musescore2-2.3.2+dfsg4/debian/patches/experiments/valid-soundfont.diff
1970-01-01 01:00:00.000000000 +0100
+++ musescore2-2.3.2+dfsg4/debian/patches/experiments/valid-soundfont.diff
2021-06-05 18:04:54.000000000 +0200
@@ -0,0 +1,430 @@
+Description: Fix multiple possible causes of crashes or audible artefacts
+ - Track sample name so we can issue proper warning messages, show filename
+ - On read errors, issue an error message and mark sample as invalid
+ - Mark sample as invalid if Ogg Vorbis decompression (SF3) fails
+ - Do all sanity checks on {,loop}{start,end} with SF2 semantics for end;
+ only switch end to point to the last sample afterwards in only one place
+ - Adapt sanity checks and corrections to current FluidSynth, which matches
+ real-existing soundfonts better
+ - Add sanity check provided by the SoundFont spec as extra diagnostic
+ - Do not crash if there is no data[]
+ - Issue diagnostics if disabling a sample
+ - Swap two members to improve structure packing/alignment while there
+ - Use unsigned integers for SoundFont element sizes properly
+Author: mirabilos <m...@mirbsd.org>
+Bug: https://musescore.org/en/node/89216 (and probably others)
+Bug-Debian: https://bugs.debian.org/985129
+Forwarded: https://github.com/musescore/MuseScore/pull/7728
+
+--- a/fluid/sfont.cpp
++++ b/fluid/sfont.cpp
+@@ -589,6 +589,7 @@ Sample::Sample(SFont* s)
+ data = 0;
+ amplitude_that_reaches_noise_floor_is_valid = false;
+ amplitude_that_reaches_noise_floor = 0.0;
++ name[0] = 0;
+ }
+
+ //---------------------------------------------------------
+@@ -625,11 +626,12 @@ void Sample::load()
+ #ifdef SOUNDFONT3
+ char* p = new char[size];
+ if (fd.read(p, size) != size) {
+- printf(" read %d failed\n", size);
+ delete[] p;
++ qWarning("SoundFont(%s) Sample(%s) read %u bytes failed",
qPrintable(sf->get_name()), name, size);
++ setValid(false);
+ return;
+ }
+- decompressOggVorbis(p, size);
++ setValid(decompressOggVorbis(p, size));
+ delete[] p;
+ #endif
+ }
+@@ -637,8 +639,11 @@ void Sample::load()
+ data = new short[size];
+ size *= sizeof(short);
+
+- if (fd.read((char*)data, size) != size)
++ if (fd.read((char*)data, size) != size) {
++ qWarning("SoundFont(%s) Sample(%s) read %u bytes failed",
qPrintable(sf->get_name()), name, size);
++ setValid(false);
+ return;
++ }
+
+ if (QSysInfo::ByteOrder == QSysInfo::BigEndian) {
+ unsigned char hi, lo;
+@@ -652,11 +657,34 @@ void Sample::load()
+ data[i] = s;
+ }
+ }
+- end -= (start + 1); // marks last sample, contrary to
SF spec.
++ end -= start;
+ loopstart -= start;
+ loopend -= start;
+ start = 0;
+ }
++
++ // sanity checks:
++ // - start < end in SFont::load_shdr(int)
++ // - start < loopstart < loopend < end for !SF3 ibidem
++ // - … for SF3 in Sample::decompressOggVorbis(char *, unsigned int) in
sfont3.cpp
++ // - most importantly, they are done *before* start is normalised to 0,
which it is at this point
++ //
++ // now add some extra sanity checks from the SF2 spec (biased so they
work with unsigned int);
++ // just a warning, they probably work with Fluid, possibly with audible
artefacts though...
++ if (!(start + 7 < loopstart) || !(loopstart + 31 < loopend) ||
!(loopend + 7 < end))
++ qDebug("SoundFont(%s) Sample(%s) start(%u) startloop(%u)
endloop(%u) end(%u) smaller than SoundFont 2.04 spec chapter 7.10
recommendation",
++ qPrintable(sf->get_name()), name, start, loopstart,
loopend, end);
++
++ // this used to cause a crash
++ if (!data) {
++ qWarning("SoundFont(%s) Sample(%s) data is nil",
qPrintable(sf->get_name()), name);
++ setValid(false);
++ }
++
++ // from here, end marks the last sample, not one past as in the SF2 spec
++ if (end > 0)
++ end -= 1;
++
+ optimize();
+ }
+
+@@ -725,6 +753,8 @@ bool SFont::load()
+ }
+ SFChunk chunk;
+
++ // so that any subsequent errors can be attributed to the right file
++ qDebug("Loading soundfont: %s", qPrintable(f.fileName()));
+ try {
+ readchunk(&chunk);
+ if (chunkid(chunk.id) != RIFF_ID)
+@@ -1599,9 +1629,7 @@ void SFont::load_shdr (int size)
+ for (int i = 0; i < size; i++) {
+ Sample* p = new Sample(this);
+ sample.append(p);
+- char buffer[21];
+- READSTR (buffer);
+- // READSTR (p->name);
++ READSTR (p->name);
+ READD (p->start);
+ READD (p->end); /* - end, loopstart and loopend */
+ READD (p->loopstart); /* - will be checked and turned into */
+@@ -1616,28 +1644,47 @@ void SFont::load_shdr (int size)
+ continue;
+ }
+ if ((p->end > getSamplesize()) || (p->start > (p->end - 4))) {
+- qWarning("Sample start/end file positions are invalid,
disabling");
++ qWarning("Sample(%s) start/end file positions are invalid,
disabling", p->name);
+ p->setValid(false);
+ continue;
+ }
+ p->setValid(true);
+ if (p->sampletype & FLUID_SAMPLETYPE_OGG_VORBIS) {
++ // done in Sample::decompressOggVorbis in sfont3.cpp
+ }
+ else {
+- // loop is fowled?? (cluck cluck :)
+- if (p->loopend > p->end || p->loopstart >= p->loopend ||
p->loopstart <= p->start) {
+- /* can pad loop by 8 samples and ensure at least 4
for loop (2*8+4) */
+- if ((p->end - p->start) >= 20) {
+- p->loopstart = p->start + 8;
+- p->loopend = p->end - 8;
+- }
+- else { // loop is fowled, sample is tiny (can't
pad 8 samples)
+- p->loopstart = p->start + 1;
+- p->loopend = p->end - 1;
+- }
++ // try to fixup start<loopstart<loopend<end similar to
recent FluidSynth;
++ // some of these are technically invalid but exist in the
field; see the
++ // SoundFont 2.04 spec-related diagnostic about this in
sfont.cpp
++ if (p->loopstart == p->loopend) {
++ // normalise to ensure they are within the sample,
even non-looped
++ p->loopstart = p->loopend = p->start;
+ }
+- if ((p->end - p->start) < 8)
++ else if (p->loopstart > p->loopend) {
++ unsigned int looptmp;
++
++ qWarning("Sample(%s) swapping reversed
loopstart<=>loopend",
++ p->name);
++ looptmp = p->loopstart;
++ p->loopstart = p->loopend;
++ p->loopend = looptmp;
++ }
++ // ensure they are at least within the sample
++ if (p->loopstart < p->start || p->loopstart > p->end) {
++ qWarning("Sample(%s) loopstart(%u) out of bounds
[start=%u end=%u], setting to start",
++ p->name, p->loopstart, p->start, p->end);
++ p->loopstart = p->start;
++ }
++ if (p->loopend < p->start || p->loopend > p->end) {
++ qWarning("Sample(%s) loopend(%u) out of bounds
[start=%u end=%u], setting to end",
++ p->name, p->loopend, p->start, p->end);
++ p->loopstart = p->end;
++ }
++ // ensure it can even play
++ if ((p->end - p->start) < 8) {
++ qWarning("Sample(%s) too small, disabling", p->name);
+ p->setValid(false);
++ }
+ }
+ }
+ FSKIP (SFSHDRSIZE); /* skip terminal shdr */
+--- a/fluid/sfont.h
++++ b/fluid/sfont.h
+@@ -147,8 +147,10 @@ class Sample {
+ filled out automatically */
+ /* Set this to zero, when submitting a new sample. */
+
+- bool amplitude_that_reaches_noise_floor_is_valid;
+ double amplitude_that_reaches_noise_floor;
++ bool amplitude_that_reaches_noise_floor_is_valid;
++
++ char name[21];
+
+ Sample(SFont*);
+ ~Sample();
+@@ -159,7 +161,7 @@ class Sample {
+ bool valid() const { return _valid; }
+ void setValid(bool v) { _valid = v; }
+ #ifdef SOUNDFONT3
+- bool decompressOggVorbis(char* p, int size);
++ bool decompressOggVorbis(char* p, unsigned int size);
+ #endif
+ };
+
+--- a/fluid/sfont3.cpp
++++ b/fluid/sfont3.cpp
+@@ -10,7 +10,7 @@ namespace FluidS {
+ // decompressOggVorbis
+ //---------------------------------------------------------
+
+-bool Sample::decompressOggVorbis(char* src, int size)
++bool Sample::decompressOggVorbis(char* src, unsigned int size)
+ {
+ AudioFile af;
+ QByteArray ba(src, size);
+@@ -18,32 +18,51 @@ bool Sample::decompressOggVorbis(char* s
+ start = 0;
+ end = 0;
+ if (!af.open(ba)) {
+- qDebug("Sample::decompressOggVorbis: open failed: %s",
af.error());
++ qDebug("SoundFont(%s) Sample(%s) decompressOggVorbis: open
failed: %s", qPrintable(sf->get_name()), name, af.error());
+ return false;
+ }
+- int frames = af.frames();
++ unsigned int frames = af.frames();
+ data = new short[frames * af.channels()];
+- if (frames != af.readData(data, frames)) {
+- qDebug("Sample read failed: %s", af.error());
++ if (frames != (unsigned int)af.readData(data, frames)) {
++ qDebug("SoundFont(%s) Sample(%s) read failed: %s",
qPrintable(sf->get_name()), name, af.error());
+ delete[] data;
+ data = 0;
++ return false;
++ }
++ // cf. https://musescore.org/en/node/89216#comment-1068379 and following
++ end = frames;
++
++ // try to fixup start<loopstart<loopend<end similar to recent
FluidSynth;
++ // some of these are technically invalid but exist in the field; see the
++ // SoundFont 2.04 spec-related diagnostic about this in sfont.cpp
++ if (loopstart == loopend) {
++ // normalise to ensure they are within the sample, even non-looped
++ loopstart = loopend = start;
+ }
+- end = frames - 1;
++ else if (loopstart > loopend) {
++ unsigned int looptmp;
+
+- if (loopend > end ||loopstart >= loopend || loopstart <= start) {
+- /* can pad loop by 8 samples and ensure at least 4 for loop
(2*8+4) */
+- if ((end - start) >= 20) {
+- loopstart = start + 8;
+- loopend = end - 8;
+- }
+- else { // loop is fowled, sample is tiny (can't pad 8 samples)
+- loopstart = start + 1;
+- loopend = end - 1;
+- }
++ qWarning("SoundFont(%s) Sample(%s) swapping reversed
loopstart<=>loopend",
++ qPrintable(sf->get_name()), name);
++ looptmp = loopstart;
++ loopstart = loopend;
++ loopend = looptmp;
+ }
++ // ensure they are at least within the sample
++ if (loopstart < start || loopstart > end) {
++ qWarning("SoundFont(%s) Sample(%s) loopstart(%u) out of bounds
[start=%u end=%u], setting to start",
++ qPrintable(sf->get_name()), name, loopstart, start, end);
++ loopstart = start;
++ }
++ if (loopend < start || loopend > end) {
++ qWarning("SoundFont(%s) Sample(%s) loopend(%u) out of bounds
[start=%u end=%u], setting to end",
++ qPrintable(sf->get_name()), name, loopend, start, end);
++ loopstart = end;
++ }
++ // ensure it can even play
+ if ((end - start) < 8) {
+- qDebug("invalid sample");
+- setValid(false);
++ qWarning("SoundFont(%s) Sample(%s) too small, disabling",
qPrintable(sf->get_name()), name);
++ return false;
+ }
+
+ return true;
+--- a/fluid/voice.cpp
++++ b/fluid/voice.cpp
+@@ -1170,8 +1170,8 @@ void Voice::update_param(int _gen)
+ case GEN_STARTADDRCOARSEOFS: /* SF2.01 section 8.1.3 # 4 */
+ if (sample != 0) {
+ start = (sample->start
+- + (int) GEN(GEN_STARTADDROFS)
+- + 32768 * (int) GEN(GEN_STARTADDRCOARSEOFS));
++ + (unsigned int) GEN(GEN_STARTADDROFS)
++ + 32768U * (unsigned int)
GEN(GEN_STARTADDRCOARSEOFS));
+ check_sample_sanity_flag = FLUID_SAMPLESANITY_CHECK;
+ }
+ break;
+@@ -1180,8 +1180,8 @@ void Voice::update_param(int _gen)
+ case GEN_ENDADDRCOARSEOFS: /* SF2.01 section 8.1.3 # 12
*/
+ if (sample != 0) {
+ end = (sample->end
+- + (int) GEN(GEN_ENDADDROFS)
+- + 32768 * (int) GEN(GEN_ENDADDRCOARSEOFS));
++ + (unsigned int) GEN(GEN_ENDADDROFS)
++ + 32768U * (unsigned int)
GEN(GEN_ENDADDRCOARSEOFS));
+ check_sample_sanity_flag = FLUID_SAMPLESANITY_CHECK;
+ }
+ break;
+@@ -1190,8 +1190,8 @@ void Voice::update_param(int _gen)
+ case GEN_STARTLOOPADDRCOARSEOFS: /* SF2.01 section 8.1.3 # 45
*/
+ if (sample != 0) {
+ loopstart = (sample->loopstart
+- + (int) GEN(GEN_STARTLOOPADDROFS)
+- + 32768 * (int)
GEN(GEN_STARTLOOPADDRCOARSEOFS));
++ + (unsigned int) GEN(GEN_STARTLOOPADDROFS)
++ + 32768U * (unsigned int)
GEN(GEN_STARTLOOPADDRCOARSEOFS));
+ check_sample_sanity_flag = FLUID_SAMPLESANITY_CHECK;
+ }
+ break;
+@@ -1200,8 +1200,8 @@ void Voice::update_param(int _gen)
+ case GEN_ENDLOOPADDRCOARSEOFS: /* SF2.01 section 8.1.3 # 50
*/
+ if (sample != 0) {
+ loopend = (sample->loopend
+- + (int) GEN(GEN_ENDLOOPADDROFS)
+- + 32768 * (int)
GEN(GEN_ENDLOOPADDRCOARSEOFS));
++ + (unsigned int) GEN(GEN_ENDLOOPADDROFS)
++ + 32768U * (unsigned int)
GEN(GEN_ENDLOOPADDRCOARSEOFS));
+ check_sample_sanity_flag = FLUID_SAMPLESANITY_CHECK;
+ }
+ break;
+@@ -1636,22 +1636,22 @@ float Voice::get_lower_boundary_for_atte
+ */
+ void Voice::check_sample_sanity()
+ {
+- int min_index_nonloop=(int) sample->start;
+- int max_index_nonloop=(int) sample->end;
++ unsigned int min_index_nonloop = sample->start;
++ unsigned int max_index_nonloop = sample->end;
+
+ /* make sure we have enough samples surrounding the loop */
+- int min_index_loop=(int) sample->start + FLUID_MIN_LOOP_PAD;
+- int max_index_loop=(int) sample->end - FLUID_MIN_LOOP_PAD;
++ unsigned int min_index_loop = sample->start + FLUID_MIN_LOOP_PAD;
++ unsigned int max_index_loop = sample->end - FLUID_MIN_LOOP_PAD;
+ fluid_check_fpe("voice_check_sample_sanity start");
+
+ if (!check_sample_sanity_flag)
+ return;
+
+ #if 0
+- printf("Sample from %i to %i\n", sample->start, sample->end);
+- printf("Sample loop from %i %i\n", sample->loopstart, sample->loopend);
+- printf("Playback from %i to %i\n", start, end);
+- printf("Playback loop from %i to %i\n", loopstart, loopend);
++ printf("Sample from %u to %u\n", sample->start, sample->end);
++ printf("Sample loop from %u to %u\n", sample->loopstart,
sample->loopend);
++ printf("Playback from %u to %u\n", start, end);
++ printf("Playback loop from %u to %u\n", loopstart, loopend);
+ #endif
+
+ /* Keep the start point within the sample data */
+@@ -1668,7 +1668,7 @@ void Voice::check_sample_sanity()
+
+ /* Keep start and end point in the right order */
+ if (start > end) {
+- int temp = start;
++ unsigned int temp = start;
+ start = end;
+ end = temp;
+ }
+@@ -1694,7 +1694,7 @@ void Voice::check_sample_sanity()
+
+ /* Keep loop start and end point in the right order */
+ if (loopstart > loopend){
+- int temp = loopstart;
++ unsigned int temp = loopstart;
+ loopstart = loopend;
+ loopend = temp;
+ }
+@@ -1706,7 +1706,7 @@ void Voice::check_sample_sanity()
+ /* The loop points may have changed. Obtain a new estimate for
the loop volume. */
+ /* Is the voice loop within the sample loop?
+ */
+- if ((int)loopstart >= (int)sample->loopstart && (int)loopend <=
(int)sample->loopend){
++ if (loopstart >= sample->loopstart && loopend <= sample->loopend){
+ /* Is there a valid peak amplitude available for the loop?
*/
+ if (sample->amplitude_that_reaches_noise_floor_is_valid) {
+ amplitude_that_reaches_noise_floor_loop =
sample->amplitude_that_reaches_noise_floor;
+@@ -1745,13 +1745,13 @@ void Voice::check_sample_sanity()
+ * the sample, enter the loop and proceed as expected => no
+ * actions required.
+ */
+- int index_in_sample = phase.index();
++ unsigned int index_in_sample = (unsigned int)phase.index();
+ if (index_in_sample >= loopend) {
+ /* FLUID_LOG(FLUID_DBG, "Loop / sample sanity check: Phase
after 2nd loop point!"); */
+ phase.setInt(loopstart);
+ }
+ }
+-/* FLUID_LOG(FLUID_DBG, "Loop / sample sanity check: Sample from %i to %i,
loop from %i to %i", voice->start, voice->end, voice->loopstart,
voice->loopend); */
++/* FLUID_LOG(FLUID_DBG, "Loop / sample sanity check: Sample from %u to %u,
loop from %u to %u", voice->start, voice->end, voice->loopstart,
voice->loopend); */
+
+ /* Sample sanity has been assured. Don't check again, until some
+ sample parameter is changed by modulation.
+@@ -1795,7 +1795,6 @@ void Sample::optimize()
+ signed short peak;
+ float normalized_amplitude_during_loop;
+ double result;
+- int i;
+
+ /* ignore ROM and other(?) invalid samples */
+ if (!s->valid())
+@@ -1803,7 +1802,7 @@ void Sample::optimize()
+
+ if (!s->amplitude_that_reaches_noise_floor_is_valid) { /* Only once */
+ /* Scan the loop */
+- for (i = (int)s->loopstart; i < (int) s->loopend; i ++) {
++ for (size_t i = s->loopstart; i < s->loopend; i++) {
+ signed short val = s->data[i];
+ if (val > peak_max)
+ peak_max = val;
+--- a/fluid/voice.h
++++ b/fluid/voice.h
+@@ -134,10 +134,10 @@ public:
+ float root_pitch, root_pitch_hz;
+
+ /* sample and loop start and end points (offset in sample memory). */
+- int start;
+- int end;
+- int loopstart;
+- int loopend;
++ unsigned int start;
++ unsigned int end;
++ unsigned int loopstart;
++ unsigned int loopend;
+
+ /* vol env */
+ fluid_env_data_t volenv_data[FLUID_VOICE_ENVLAST];
diff -Nru musescore2-2.3.2+dfsg4/debian/patches/series
musescore2-2.3.2+dfsg4/debian/patches/series
--- musescore2-2.3.2+dfsg4/debian/patches/series 2021-03-05
16:15:58.000000000 +0100
+++ musescore2-2.3.2+dfsg4/debian/patches/series 2021-06-05
18:04:54.000000000 +0200
@@ -30,6 +30,7 @@
upstream/statusline-pitch-onofftime.diff
upstream/improve-mscore-font.diff
upstream/5507.patch
+experiments/valid-soundfont.diff
debian-specific/fixup-AppData.diff
experiments/upstream-backend-fixes.diff
experiments/lyrics-hyphen-5sp.diff
diff -Nru musescore3-3.2.3+dfsg2/debian/changelog
musescore3-3.2.3+dfsg2/debian/changelog
--- musescore3-3.2.3+dfsg2/debian/changelog 2021-03-05 17:59:50.000000000
+0100
+++ musescore3-3.2.3+dfsg2/debian/changelog 2021-06-05 18:18:36.000000000
+0200
@@ -1,3 +1,9 @@
+musescore3 (3.2.3+dfsg2-11) unstable; urgency=medium
+
+ * Fix soundfont-related crashes or audible artefacts (Closes: #985129)
+
+ -- Thorsten Glaser <t...@mirbsd.de> Sat, 05 Jun 2021 18:18:36 +0200
+
musescore3 (3.2.3+dfsg2-10) unstable; urgency=medium
* Merge musescore2 (2.3.2+dfsg4-14) unstable; urgency=medium
@@ -287,6 +293,12 @@
-- Thorsten Glaser <t...@mirbsd.de> Sun, 27 Jan 2019 17:06:00 +0100
+musescore2 (2.3.2+dfsg4-15) unstable; urgency=medium
+
+ * Fix soundfont-related crashes or audible artefacts (Closes: #985129)
+
+ -- Thorsten Glaser <t...@mirbsd.de> Sat, 05 Jun 2021 18:04:54 +0200
+
musescore2 (2.3.2+dfsg4-14) unstable; urgency=medium
* Fix possible error cause in m-common.prerm (Closes: #984592)
diff -Nru
musescore3-3.2.3+dfsg2/debian/patches/experiments/valid-soundfont.diff
musescore3-3.2.3+dfsg2/debian/patches/experiments/valid-soundfont.diff
--- musescore3-3.2.3+dfsg2/debian/patches/experiments/valid-soundfont.diff
1970-01-01 01:00:00.000000000 +0100
+++ musescore3-3.2.3+dfsg2/debian/patches/experiments/valid-soundfont.diff
2021-06-05 18:18:36.000000000 +0200
@@ -0,0 +1,428 @@
+Description: Fix multiple possible causes of crashes or audible artefacts
+ - Track sample name so we can issue proper warning messages, show filename
+ - On read errors, issue an error message and mark sample as invalid
+ - Mark sample as invalid if Ogg Vorbis decompression (SF3) fails
+ - Do all sanity checks on {,loop}{start,end} with SF2 semantics for end;
+ only switch end to point to the last sample afterwards in only one place
+ - Adapt sanity checks and corrections to current FluidSynth, which matches
+ real-existing soundfonts better
+ - Add sanity check provided by the SoundFont spec as extra diagnostic
+ - Do not crash if there is no data[]
+ - Issue diagnostics if disabling a sample
+ - Swap two members to improve structure packing/alignment while there
+ - Use unsigned integers for SoundFont element sizes properly
+Author: mirabilos <m...@mirbsd.org>
+Bug: https://musescore.org/en/node/89216 (and probably others)
+Bug-Debian: https://bugs.debian.org/985129
+Forwarded: https://github.com/musescore/MuseScore/pull/7728
+
+--- a/fluid/sfont.cpp
++++ b/fluid/sfont.cpp
+@@ -610,6 +610,7 @@ Sample::Sample(SFont* s)
+ data = 0;
+ amplitude_that_reaches_noise_floor_is_valid = false;
+ amplitude_that_reaches_noise_floor = 0.0;
++ name[0] = 0;
+ }
+
+ //---------------------------------------------------------
+@@ -647,18 +648,22 @@ void Sample::load()
+ std::vector<char> p;
+ p.resize(size);
+ if (fd.read(p.data(), size) != size) {
+- qDebug("read %d failed", size);
++ qWarning("SoundFont(%s) Sample(%s) read %u bytes failed",
qPrintable(sf->get_name()), name, size);
++ setValid(false);
+ return;
+ }
+- decompressOggVorbis(p.data(), size);
++ setValid(decompressOggVorbis(p.data(), size));
+ #endif
+ }
+ else {
+ data = new short[size];
+ size *= sizeof(short);
+
+- if (fd.read((char*)data, size) != size)
++ if (fd.read((char*)data, size) != size) {
++ qWarning("SoundFont(%s) Sample(%s) read %u bytes failed",
qPrintable(sf->get_name()), name, size);
++ setValid(false);
+ return;
++ }
+
+ if (QSysInfo::ByteOrder == QSysInfo::BigEndian) {
+ unsigned char hi, lo;
+@@ -672,11 +677,34 @@ void Sample::load()
+ data[i] = s;
+ }
+ }
+- end -= (start + 1); // marks last sample, contrary to
SF spec.
++ end -= start;
+ loopstart -= start;
+ loopend -= start;
+ start = 0;
+ }
++
++ // sanity checks:
++ // - start < end in SFont::load_shdr(int)
++ // - start < loopstart < loopend < end for !SF3 ibidem
++ // - … for SF3 in Sample::decompressOggVorbis(char *, unsigned int) in
sfont3.cpp
++ // - most importantly, they are done *before* start is normalised to 0,
which it is at this point
++ //
++ // now add some extra sanity checks from the SF2 spec (biased so they
work with unsigned int);
++ // just a warning, they probably work with Fluid, possibly with audible
artefacts though...
++ if (!(start + 7 < loopstart) || !(loopstart + 31 < loopend) ||
!(loopend + 7 < end))
++ qDebug("SoundFont(%s) Sample(%s) start(%u) startloop(%u)
endloop(%u) end(%u) smaller than SoundFont 2.04 spec chapter 7.10
recommendation",
++ qPrintable(sf->get_name()), name, start, loopstart,
loopend, end);
++
++ // this used to cause a crash
++ if (!data) {
++ qWarning("SoundFont(%s) Sample(%s) data is nil",
qPrintable(sf->get_name()), name);
++ setValid(false);
++ }
++
++ // from here, end marks the last sample, not one past as in the SF2 spec
++ if (end > 0)
++ end -= 1;
++
+ optimize();
+ }
+
+@@ -745,6 +773,8 @@ bool SFont::load()
+ }
+ SFChunk chunk;
+
++ // so that any subsequent errors can be attributed to the right file
++ qDebug("Loading soundfont: %s", qPrintable(f.fileName()));
+ try {
+ readchunk(&chunk);
+ if (chunkid(chunk.id) != RIFF_ID)
+@@ -1624,9 +1654,7 @@ void SFont::load_shdr (int size)
+ for (int i = 0; i < size; i++) {
+ Sample* p = new Sample(this);
+ sample.append(p);
+- char buffer[21];
+- READSTR (buffer);
+- // READSTR (p->name);
++ READSTR (p->name);
+ READD (p->start);
+ READD (p->end); /* - end, loopstart and loopend */
+ READD (p->loopstart); /* - will be checked and turned into */
+@@ -1641,28 +1669,47 @@ void SFont::load_shdr (int size)
+ continue;
+ }
+ if ((p->end > getSamplesize()) || (p->start > (p->end - 4))) {
+- qWarning("Sample start/end file positions are invalid,
disabling");
++ qWarning("Sample(%s) start/end file positions are invalid,
disabling", p->name);
+ p->setValid(false);
+ continue;
+ }
+ p->setValid(true);
+ if (p->sampletype & FLUID_SAMPLETYPE_OGG_VORBIS) {
++ // done in Sample::decompressOggVorbis in sfont3.cpp
+ }
+ else {
+- // loop is fowled?? (cluck cluck :)
+- if (p->loopend > p->end || p->loopstart >= p->loopend ||
p->loopstart <= p->start) {
+- /* can pad loop by 8 samples and ensure at least 4
for loop (2*8+4) */
+- if ((p->end - p->start) >= 20) {
+- p->loopstart = p->start + 8;
+- p->loopend = p->end - 8;
+- }
+- else { // loop is fowled, sample is tiny (can't
pad 8 samples)
+- p->loopstart = p->start + 1;
+- p->loopend = p->end - 1;
+- }
++ // try to fixup start<loopstart<loopend<end similar to
recent FluidSynth;
++ // some of these are technically invalid but exist in the
field; see the
++ // SoundFont 2.04 spec-related diagnostic about this in
sfont.cpp
++ if (p->loopstart == p->loopend) {
++ // normalise to ensure they are within the sample,
even non-looped
++ p->loopstart = p->loopend = p->start;
+ }
+- if ((p->end - p->start) < 8)
++ else if (p->loopstart > p->loopend) {
++ unsigned int looptmp;
++
++ qWarning("Sample(%s) swapping reversed
loopstart<=>loopend",
++ p->name);
++ looptmp = p->loopstart;
++ p->loopstart = p->loopend;
++ p->loopend = looptmp;
++ }
++ // ensure they are at least within the sample
++ if (p->loopstart < p->start || p->loopstart > p->end) {
++ qWarning("Sample(%s) loopstart(%u) out of bounds
[start=%u end=%u], setting to start",
++ p->name, p->loopstart, p->start, p->end);
++ p->loopstart = p->start;
++ }
++ if (p->loopend < p->start || p->loopend > p->end) {
++ qWarning("Sample(%s) loopend(%u) out of bounds
[start=%u end=%u], setting to end",
++ p->name, p->loopend, p->start, p->end);
++ p->loopstart = p->end;
++ }
++ // ensure it can even play
++ if ((p->end - p->start) < 8) {
++ qWarning("Sample(%s) too small, disabling", p->name);
+ p->setValid(false);
++ }
+ }
+ }
+ FSKIP (SFSHDRSIZE); /* skip terminal shdr */
+--- a/fluid/sfont.h
++++ b/fluid/sfont.h
+@@ -150,8 +150,10 @@ class Sample {
+ filled out automatically */
+ /* Set this to zero, when submitting a new sample. */
+
+- bool amplitude_that_reaches_noise_floor_is_valid;
+ double amplitude_that_reaches_noise_floor;
++ bool amplitude_that_reaches_noise_floor_is_valid;
++
++ char name[21];
+
+ Sample(SFont*);
+ ~Sample();
+@@ -162,7 +164,7 @@ class Sample {
+ bool valid() const { return _valid; }
+ void setValid(bool v) { _valid = v; }
+ #ifdef SOUNDFONT3
+- bool decompressOggVorbis(char* p, int size);
++ bool decompressOggVorbis(char* p, unsigned int size);
+ #endif
+ };
+
+--- a/fluid/sfont3.cpp
++++ b/fluid/sfont3.cpp
+@@ -10,7 +10,7 @@ namespace FluidS {
+ // decompressOggVorbis
+ //---------------------------------------------------------
+
+-bool Sample::decompressOggVorbis(char* src, int size)
++bool Sample::decompressOggVorbis(char* src, unsigned int size)
+ {
+ AudioFile af;
+ QByteArray ba(src, size);
+@@ -18,32 +18,51 @@ bool Sample::decompressOggVorbis(char* s
+ start = 0;
+ end = 0;
+ if (!af.open(ba)) {
+- qDebug("Sample::decompressOggVorbis: open failed: %s",
af.error());
++ qDebug("SoundFont(%s) Sample(%s) decompressOggVorbis: open
failed: %s", qPrintable(sf->get_name()), name, af.error());
+ return false;
+ }
+- int frames = af.frames();
++ unsigned int frames = af.frames();
+ data = new short[frames * af.channels()];
+- if (frames != af.readData(data, frames)) {
+- qDebug("Sample read failed: %s", af.error());
++ if (frames != (unsigned int)af.readData(data, frames)) {
++ qDebug("SoundFont(%s) Sample(%s) read failed: %s",
qPrintable(sf->get_name()), name, af.error());
+ delete[] data;
+ data = 0;
++ return false;
++ }
++ // cf. https://musescore.org/en/node/89216#comment-1068379 and following
++ end = frames;
++
++ // try to fixup start<loopstart<loopend<end similar to recent
FluidSynth;
++ // some of these are technically invalid but exist in the field; see the
++ // SoundFont 2.04 spec-related diagnostic about this in sfont.cpp
++ if (loopstart == loopend) {
++ // normalise to ensure they are within the sample, even non-looped
++ loopstart = loopend = start;
+ }
+- end = frames - 1;
++ else if (loopstart > loopend) {
++ unsigned int looptmp;
+
+- if (loopend > end ||loopstart >= loopend || loopstart <= start) {
+- /* can pad loop by 8 samples and ensure at least 4 for loop
(2*8+4) */
+- if ((end - start) >= 20) {
+- loopstart = start + 8;
+- loopend = end - 8;
+- }
+- else { // loop is fowled, sample is tiny (can't pad 8 samples)
+- loopstart = start + 1;
+- loopend = end - 1;
+- }
++ qWarning("SoundFont(%s) Sample(%s) swapping reversed
loopstart<=>loopend",
++ qPrintable(sf->get_name()), name);
++ looptmp = loopstart;
++ loopstart = loopend;
++ loopend = looptmp;
+ }
++ // ensure they are at least within the sample
++ if (loopstart < start || loopstart > end) {
++ qWarning("SoundFont(%s) Sample(%s) loopstart(%u) out of bounds
[start=%u end=%u], setting to start",
++ qPrintable(sf->get_name()), name, loopstart, start, end);
++ loopstart = start;
++ }
++ if (loopend < start || loopend > end) {
++ qWarning("SoundFont(%s) Sample(%s) loopend(%u) out of bounds
[start=%u end=%u], setting to end",
++ qPrintable(sf->get_name()), name, loopend, start, end);
++ loopstart = end;
++ }
++ // ensure it can even play
+ if ((end - start) < 8) {
+- qDebug("invalid sample");
+- setValid(false);
++ qWarning("SoundFont(%s) Sample(%s) too small, disabling",
qPrintable(sf->get_name()), name);
++ return false;
+ }
+
+ return true;
+--- a/fluid/voice.cpp
++++ b/fluid/voice.cpp
+@@ -1171,8 +1171,8 @@ void Voice::update_param(int _gen)
+ case GEN_STARTADDRCOARSEOFS: /* SF2.01 section 8.1.3 # 4 */
+ if (sample != 0) {
+ start = (sample->start
+- + (int) GEN(GEN_STARTADDROFS)
+- + 32768 * (int) GEN(GEN_STARTADDRCOARSEOFS));
++ + (unsigned int) GEN(GEN_STARTADDROFS)
++ + 32768U * (unsigned int)
GEN(GEN_STARTADDRCOARSEOFS));
+ check_sample_sanity_flag = FLUID_SAMPLESANITY_CHECK;
+ }
+ break;
+@@ -1181,8 +1181,8 @@ void Voice::update_param(int _gen)
+ case GEN_ENDADDRCOARSEOFS: /* SF2.01 section 8.1.3 # 12
*/
+ if (sample != 0) {
+ end = (sample->end
+- + (int) GEN(GEN_ENDADDROFS)
+- + 32768 * (int) GEN(GEN_ENDADDRCOARSEOFS));
++ + (unsigned int) GEN(GEN_ENDADDROFS)
++ + 32768U * (unsigned int)
GEN(GEN_ENDADDRCOARSEOFS));
+ check_sample_sanity_flag = FLUID_SAMPLESANITY_CHECK;
+ }
+ break;
+@@ -1191,8 +1191,8 @@ void Voice::update_param(int _gen)
+ case GEN_STARTLOOPADDRCOARSEOFS: /* SF2.01 section 8.1.3 # 45
*/
+ if (sample != 0) {
+ loopstart = (sample->loopstart
+- + (int) GEN(GEN_STARTLOOPADDROFS)
+- + 32768 * (int)
GEN(GEN_STARTLOOPADDRCOARSEOFS));
++ + (unsigned int) GEN(GEN_STARTLOOPADDROFS)
++ + 32768U * (unsigned int)
GEN(GEN_STARTLOOPADDRCOARSEOFS));
+ check_sample_sanity_flag = FLUID_SAMPLESANITY_CHECK;
+ }
+ break;
+@@ -1201,8 +1201,8 @@ void Voice::update_param(int _gen)
+ case GEN_ENDLOOPADDRCOARSEOFS: /* SF2.01 section 8.1.3 # 50
*/
+ if (sample != 0) {
+ loopend = (sample->loopend
+- + (int) GEN(GEN_ENDLOOPADDROFS)
+- + 32768 * (int)
GEN(GEN_ENDLOOPADDRCOARSEOFS));
++ + (unsigned int) GEN(GEN_ENDLOOPADDROFS)
++ + 32768U * (unsigned int)
GEN(GEN_ENDLOOPADDRCOARSEOFS));
+ check_sample_sanity_flag = FLUID_SAMPLESANITY_CHECK;
+ }
+ break;
+@@ -1646,22 +1646,22 @@ float Voice::get_lower_boundary_for_atte
+ */
+ void Voice::check_sample_sanity()
+ {
+- int min_index_nonloop=(int) sample->start;
+- int max_index_nonloop=(int) sample->end;
++ unsigned int min_index_nonloop = sample->start;
++ unsigned int max_index_nonloop = sample->end;
+
+ /* make sure we have enough samples surrounding the loop */
+- int min_index_loop=(int) sample->start + FLUID_MIN_LOOP_PAD;
+- int max_index_loop=(int) sample->end - FLUID_MIN_LOOP_PAD;
++ unsigned int min_index_loop = sample->start + FLUID_MIN_LOOP_PAD;
++ unsigned int max_index_loop = sample->end - FLUID_MIN_LOOP_PAD;
+ fluid_check_fpe("voice_check_sample_sanity start");
+
+ if (!check_sample_sanity_flag)
+ return;
+
+ #if 0
+-printf("Sample from %i to %i\n", sample->start, sample->end);
+-printf("Sample loop from %i %i\n", sample->loopstart, sample->loopend);
+-printf("Playback from %i to %i\n", start, end);
+-printf("Playback loop from %i to %i\n", loopstart, loopend);
++printf("Sample from %u to %u\n", sample->start, sample->end);
++printf("Sample loop from %u to %u\n", sample->loopstart, sample->loopend);
++printf("Playback from %u to %u\n", start, end);
++printf("Playback loop from %u to %u\n", loopstart, loopend);
+ #endif
+
+ /* Keep the start point within the sample data */
+@@ -1678,7 +1678,7 @@ printf("Playback loop from %i to %i\n",
+
+ /* Keep start and end point in the right order */
+ if (start > end) {
+- int temp = start;
++ unsigned int temp = start;
+ start = end;
+ end = temp;
+ }
+@@ -1704,7 +1704,7 @@ printf("Playback loop from %i to %i\n",
+
+ /* Keep loop start and end point in the right order */
+ if (loopstart > loopend){
+- int temp = loopstart;
++ unsigned int temp = loopstart;
+ loopstart = loopend;
+ loopend = temp;
+ }
+@@ -1716,7 +1716,7 @@ printf("Playback loop from %i to %i\n",
+ /* The loop points may have changed. Obtain a new estimate for
the loop volume. */
+ /* Is the voice loop within the sample loop?
+ */
+- if ((int)loopstart >= (int)sample->loopstart && (int)loopend <=
(int)sample->loopend){
++ if (loopstart >= sample->loopstart && loopend <= sample->loopend){
+ /* Is there a valid peak amplitude available for the loop?
*/
+ if (sample->amplitude_that_reaches_noise_floor_is_valid) {
+ amplitude_that_reaches_noise_floor_loop =
sample->amplitude_that_reaches_noise_floor;
+@@ -1755,13 +1755,13 @@ printf("Playback loop from %i to %i\n",
+ * the sample, enter the loop and proceed as expected => no
+ * actions required.
+ */
+- int index_in_sample = phase.index();
++ unsigned int index_in_sample = (unsigned int)phase.index();
+ if (index_in_sample >= loopend) {
+ /* FLUID_LOG(FLUID_DBG, "Loop / sample sanity check: Phase
after 2nd loop point!"); */
+ phase.setInt(loopstart);
+ }
+ }
+-/* FLUID_LOG(FLUID_DBG, "Loop / sample sanity check: Sample from %i to %i,
loop from %i to %i", voice->start, voice->end, voice->loopstart,
voice->loopend); */
++/* FLUID_LOG(FLUID_DBG, "Loop / sample sanity check: Sample from %u to %u,
loop from %u to %u", voice->start, voice->end, voice->loopstart,
voice->loopend); */
+
+ /* Sample sanity has been assured. Don't check again, until some
+ sample parameter is changed by modulation.
+@@ -1805,7 +1805,6 @@ void Sample::optimize()
+ signed short peak;
+ float normalized_amplitude_during_loop;
+ double result;
+- int i;
+
+ /* ignore ROM and other(?) invalid samples */
+ if (!s->valid())
+@@ -1813,7 +1812,7 @@ void Sample::optimize()
+
+ if (!s->amplitude_that_reaches_noise_floor_is_valid) { /* Only once */
+ /* Scan the loop */
+- for (i = (int)s->loopstart; i < (int) s->loopend; i ++) {
++ for (size_t i = s->loopstart; i < s->loopend; i++) {
+ signed short val = s->data[i];
+ if (val > peak_max)
+ peak_max = val;
+--- a/fluid/voice.h
++++ b/fluid/voice.h
+@@ -134,10 +134,10 @@ public:
+ float root_pitch, root_pitch_hz;
+
+ /* sample and loop start and end points (offset in sample memory). */
+- int start;
+- int end;
+- int loopstart;
+- int loopend;
++ unsigned int start;
++ unsigned int end;
++ unsigned int loopstart;
++ unsigned int loopend;
+
+ /* vol env */
+ fluid_env_data_t volenv_data[FLUID_VOICE_ENVLAST];
diff -Nru musescore3-3.2.3+dfsg2/debian/patches/series
musescore3-3.2.3+dfsg2/debian/patches/series
--- musescore3-3.2.3+dfsg2/debian/patches/series 2021-03-05
17:59:28.000000000 +0100
+++ musescore3-3.2.3+dfsg2/debian/patches/series 2021-06-05
18:18:36.000000000 +0200
@@ -91,6 +91,7 @@
upstream/fix-accidental-paren-pos.diff
upstream/smufl-fixes.diff
upstream/sprintf.diff
+experiments/valid-soundfont.diff
experiments/element-barbeat.diff
experiments/revert-restriking-patch.diff
experiments/log-collisions.diff
