280aa80a4728d36a5183bbe1b9581bae18991a03
[alioth/musescore.git] / debian / patches / experiments / valid-soundfont.diff
1 Description: Fix multiple possible causes of crashes or audible artefacts
2  - Track sample name so we can issue proper warning messages, show filename
3  - On read errors, issue an error message and mark sample as invalid
4  - Mark sample as invalid if Ogg Vorbis decompression (SF3) fails
5  - Do all sanity checks on {,loop}{start,end} with SF2 semantics for end;
6    only switch end to point to the last sample afterwards in only one place
7  - Adapt sanity checks and corrections to current FluidSynth, which matches
8    real-existing soundfonts better
9  - Add sanity check provided by the SoundFont spec as extra diagnostic
10  - Do not crash if there is no data[]
11  - Issue diagnostics if disabling a sample
12  - Swap two members to improve structure packing/alignment while there
13  - Use unsigned integers for SoundFont element sizes properly
14 Author: mirabilos <m@mirbsd.org>
15 Bug: https://musescore.org/en/node/89216 (and probably others)
16 Bug-Debian: https://bugs.debian.org/985129
17 Forwarded: https://github.com/musescore/MuseScore/pull/7728
18
19 --- a/fluid/sfont.cpp
20 +++ b/fluid/sfont.cpp
21 @@ -589,6 +589,7 @@ Sample::Sample(SFont* s)
22        data        = 0;
23        amplitude_that_reaches_noise_floor_is_valid = false;
24        amplitude_that_reaches_noise_floor = 0.0;
25 +      name[0]     = 0;
26        }
27  
28  //---------------------------------------------------------
29 @@ -625,11 +626,12 @@ void Sample::load()
30  #ifdef SOUNDFONT3
31              char* p = new char[size];
32              if (fd.read(p, size) != size) {
33 -                  printf("  read %d failed\n", size);
34                    delete[] p;
35 +                  qWarning("SoundFont(%s) Sample(%s) read %u bytes failed", qPrintable(sf->get_name()), name, size);
36 +                  setValid(false);
37                    return;
38                    }
39 -            decompressOggVorbis(p, size);
40 +            setValid(decompressOggVorbis(p, size));
41              delete[] p;
42  #endif
43              }
44 @@ -637,8 +639,11 @@ void Sample::load()
45              data = new short[size];
46              size *= sizeof(short);
47  
48 -            if (fd.read((char*)data, size) != size)
49 +            if (fd.read((char*)data, size) != size) {
50 +                  qWarning("SoundFont(%s) Sample(%s) read %u bytes failed", qPrintable(sf->get_name()), name, size);
51 +                  setValid(false);
52                    return;
53 +                  }
54  
55              if (QSysInfo::ByteOrder == QSysInfo::BigEndian) {
56                    unsigned char hi, lo;
57 @@ -652,11 +657,34 @@ void Sample::load()
58                          data[i] = s;
59                          }
60                    }
61 -            end       -= (start + 1);       // marks last sample, contrary to SF spec.
62 +            end       -= start;
63              loopstart -= start;
64              loopend   -= start;
65              start      = 0;
66              }
67 +
68 +      // sanity checks:
69 +      // - start < end in SFont::load_shdr(int)
70 +      // - start < loopstart < loopend < end for !SF3 ibidem
71 +      // - … for SF3 in Sample::decompressOggVorbis(char *, unsigned int) in sfont3.cpp
72 +      // - most importantly, they are done *before* start is normalised to 0, which it is at this point
73 +      //
74 +      // now add some extra sanity checks from the SF2 spec (biased so they work with unsigned int);
75 +      // just a warning, they probably work with Fluid, possibly with audible artefacts though...
76 +      if (!(start + 7 < loopstart) || !(loopstart + 31 < loopend) || !(loopend + 7 < end))
77 +            qDebug("SoundFont(%s) Sample(%s) start(%u) startloop(%u) endloop(%u) end(%u) smaller than SoundFont 2.04 spec chapter 7.10 recommendation",
78 +                     qPrintable(sf->get_name()), name, start, loopstart, loopend, end);
79 +
80 +      // this used to cause a crash
81 +      if (!data) {
82 +            qWarning("SoundFont(%s) Sample(%s) data is nil", qPrintable(sf->get_name()), name);
83 +            setValid(false);
84 +            }
85 +
86 +      // from here, end marks the last sample, not one past as in the SF2 spec
87 +      if (end > 0)
88 +            end -= 1;
89 +
90        optimize();
91        }
92  
93 @@ -725,6 +753,8 @@ bool SFont::load()
94              }
95        SFChunk chunk;
96  
97 +      // so that any subsequent errors can be attributed to the right file
98 +      qDebug("Loading soundfont: %s", qPrintable(f.fileName()));
99        try {
100              readchunk(&chunk);
101              if (chunkid(chunk.id) != RIFF_ID)
102 @@ -1599,9 +1629,7 @@ void SFont::load_shdr (int size)
103        for (int i = 0; i < size; i++) {
104              Sample* p = new Sample(this);
105              sample.append(p);
106 -            char buffer[21];
107 -            READSTR (buffer);
108 -            // READSTR (p->name);
109 +            READSTR (p->name);
110              READD (p->start);
111              READD (p->end);          /* - end, loopstart and loopend */
112              READD (p->loopstart);      /* - will be checked and turned into */
113 @@ -1616,28 +1644,47 @@ void SFont::load_shdr (int size)
114                    continue;
115                    }
116              if ((p->end > getSamplesize()) || (p->start > (p->end - 4))) {
117 -                  qWarning("Sample start/end file positions are invalid, disabling");
118 +                  qWarning("Sample(%s) start/end file positions are invalid, disabling", p->name);
119                    p->setValid(false);
120                    continue;
121                    }
122              p->setValid(true);
123              if (p->sampletype & FLUID_SAMPLETYPE_OGG_VORBIS) {
124 +                  // done in Sample::decompressOggVorbis in sfont3.cpp
125                    }
126              else {
127 -                  // loop is fowled?? (cluck cluck :)
128 -                  if (p->loopend > p->end || p->loopstart >= p->loopend || p->loopstart <= p->start) {
129 -                        /* can pad loop by 8 samples and ensure at least 4 for loop (2*8+4) */
130 -                        if ((p->end - p->start) >= 20) {
131 -                              p->loopstart = p->start + 8;
132 -                              p->loopend   = p->end - 8;
133 -                              }
134 -                        else {      // loop is fowled, sample is tiny (can't pad 8 samples)
135 -                              p->loopstart = p->start + 1;
136 -                              p->loopend   = p->end - 1;
137 -                              }
138 +                  // try to fixup start<loopstart<loopend<end similar to recent FluidSynth;
139 +                  // some of these are technically invalid but exist in the field; see the
140 +                  // SoundFont 2.04 spec-related diagnostic about this in sfont.cpp
141 +                  if (p->loopstart == p->loopend) {
142 +                        // normalise to ensure they are within the sample, even non-looped
143 +                        p->loopstart = p->loopend = p->start;
144                          }
145 -                  if ((p->end - p->start) < 8)
146 +                  else if (p->loopstart < p->loopend) {
147 +                        unsigned int looptmp;
148 +
149 +                        qWarning("Sample(%s) swapping reversed loopstart<=>loopend",
150 +                                 p->name);
151 +                        looptmp = p->loopstart;
152 +                        p->loopstart = p->loopend;
153 +                        p->loopend = looptmp;
154 +                        }
155 +                  // ensure they are at least within the sample
156 +                  if (p->loopstart < p->start || p->loopstart > p->end) {
157 +                        qWarning("Sample(%s) loopstart(%u) out of bounds [start=%u end=%u], setting to start",
158 +                                 p->name, p->loopstart, p->start, p->end);
159 +                        p->loopstart = p->start;
160 +                        }
161 +                  if (p->loopend < p->start || p->loopend > p->end) {
162 +                        qWarning("Sample(%s) loopend(%u) out of bounds [start=%u end=%u], setting to end",
163 +                                 p->name, p->loopend, p->start, p->end);
164 +                        p->loopstart = p->end;
165 +                        }
166 +                  // ensure it can even play
167 +                  if ((p->end - p->start) < 8) {
168 +                        qWarning("Sample(%s) too small, disabling", p->name);
169                          p->setValid(false);
170 +                        }
171                    }
172              }
173        FSKIP (SFSHDRSIZE);      /* skip terminal shdr */
174 --- a/fluid/sfont.h
175 +++ b/fluid/sfont.h
176 @@ -147,8 +147,10 @@ class Sample {
177            filled out automatically */
178        /* Set this to zero, when submitting a new sample. */
179  
180 -      bool amplitude_that_reaches_noise_floor_is_valid;
181        double amplitude_that_reaches_noise_floor;
182 +      bool amplitude_that_reaches_noise_floor_is_valid;
183 +
184 +      char name[21];
185  
186        Sample(SFont*);
187        ~Sample();
188 @@ -159,7 +161,7 @@ class Sample {
189        bool valid() const    { return _valid; }
190        void setValid(bool v) { _valid = v; }
191  #ifdef SOUNDFONT3
192 -      bool decompressOggVorbis(char* p, int size);
193 +      bool decompressOggVorbis(char* p, unsigned int size);
194  #endif
195        };
196  
197 --- a/fluid/sfont3.cpp
198 +++ b/fluid/sfont3.cpp
199 @@ -10,7 +10,7 @@ namespace FluidS {
200  //   decompressOggVorbis
201  //---------------------------------------------------------
202  
203 -bool Sample::decompressOggVorbis(char* src, int size)
204 +bool Sample::decompressOggVorbis(char* src, unsigned int size)
205        {
206        AudioFile af;
207        QByteArray ba(src, size);
208 @@ -18,32 +18,51 @@ bool Sample::decompressOggVorbis(char* s
209        start = 0;
210        end   = 0;
211        if (!af.open(ba)) {
212 -            qDebug("Sample::decompressOggVorbis: open failed: %s", af.error());
213 +            qDebug("SoundFont(%s) Sample(%s) decompressOggVorbis: open failed: %s", qPrintable(sf->get_name()), name, af.error());
214              return false;
215              }
216 -      int frames = af.frames();
217 +      unsigned int frames = af.frames();
218        data = new short[frames * af.channels()];
219 -      if (frames != af.readData(data, frames)) {
220 -            qDebug("Sample read failed: %s", af.error());
221 +      if (frames != (unsigned int)af.readData(data, frames)) {
222 +            qDebug("SoundFont(%s) Sample(%s) read failed: %s", qPrintable(sf->get_name()), name, af.error());
223              delete[] data;
224              data = 0;
225 +            return false;
226 +            }
227 +      // cf. https://musescore.org/en/node/89216#comment-1068379 and following
228 +      end = frames;
229 +
230 +      // try to fixup start<loopstart<loopend<end similar to recent FluidSynth;
231 +      // some of these are technically invalid but exist in the field; see the
232 +      // SoundFont 2.04 spec-related diagnostic about this in sfont.cpp
233 +      if (loopstart == loopend) {
234 +            // normalise to ensure they are within the sample, even non-looped
235 +            loopstart = loopend = start;
236              }
237 -      end = frames - 1;
238 +      else if (loopstart < loopend) {
239 +            unsigned int looptmp;
240  
241 -      if (loopend > end ||loopstart >= loopend || loopstart <= start) {
242 -            /* can pad loop by 8 samples and ensure at least 4 for loop (2*8+4) */
243 -            if ((end - start) >= 20) {
244 -                  loopstart = start + 8;
245 -                  loopend = end - 8;
246 -                  }
247 -            else { // loop is fowled, sample is tiny (can't pad 8 samples)
248 -                  loopstart = start + 1;
249 -                  loopend = end - 1;
250 -                  }
251 +            qWarning("SoundFont(%s) Sample(%s) swapping reversed loopstart<=>loopend",
252 +                     qPrintable(sf->get_name()), name);
253 +            looptmp = loopstart;
254 +            loopstart = loopend;
255 +            loopend = looptmp;
256              }
257 +      // ensure they are at least within the sample
258 +      if (loopstart < start || loopstart > end) {
259 +            qWarning("SoundFont(%s) Sample(%s) loopstart(%u) out of bounds [start=%u end=%u], setting to start",
260 +                     qPrintable(sf->get_name()), name, loopstart, start, end);
261 +            loopstart = start;
262 +            }
263 +      if (loopend < start || loopend > end) {
264 +            qWarning("SoundFont(%s) Sample(%s) loopend(%u) out of bounds [start=%u end=%u], setting to end",
265 +                     qPrintable(sf->get_name()), name, loopend, start, end);
266 +            loopstart = end;
267 +            }
268 +      // ensure it can even play
269        if ((end - start) < 8) {
270 -            qDebug("invalid sample");
271 -            setValid(false);
272 +            qWarning("SoundFont(%s) Sample(%s) too small, disabling", qPrintable(sf->get_name()), name);
273 +            return false;
274              }
275  
276        return true;
277 --- a/fluid/voice.cpp
278 +++ b/fluid/voice.cpp
279 @@ -1170,8 +1170,8 @@ void Voice::update_param(int _gen)
280              case GEN_STARTADDRCOARSEOFS:        /* SF2.01 section 8.1.3 # 4 */
281                    if (sample != 0) {
282                          start = (sample->start
283 -                                + (int) GEN(GEN_STARTADDROFS)
284 -                                + 32768 * (int) GEN(GEN_STARTADDRCOARSEOFS));
285 +                                + (unsigned int) GEN(GEN_STARTADDROFS)
286 +                                + 32768U * (unsigned int) GEN(GEN_STARTADDRCOARSEOFS));
287                          check_sample_sanity_flag = FLUID_SAMPLESANITY_CHECK;
288                          }
289                    break;
290 @@ -1180,8 +1180,8 @@ void Voice::update_param(int _gen)
291              case GEN_ENDADDRCOARSEOFS:           /* SF2.01 section 8.1.3 # 12 */
292                    if (sample != 0) {
293                          end = (sample->end
294 -                                + (int) GEN(GEN_ENDADDROFS)
295 -                                + 32768 * (int) GEN(GEN_ENDADDRCOARSEOFS));
296 +                                + (unsigned int) GEN(GEN_ENDADDROFS)
297 +                                + 32768U * (unsigned int) GEN(GEN_ENDADDRCOARSEOFS));
298                          check_sample_sanity_flag = FLUID_SAMPLESANITY_CHECK;
299                          }
300                    break;
301 @@ -1190,8 +1190,8 @@ void Voice::update_param(int _gen)
302              case GEN_STARTLOOPADDRCOARSEOFS:     /* SF2.01 section 8.1.3 # 45 */
303                    if (sample != 0) {
304                          loopstart = (sample->loopstart
305 -                                 + (int) GEN(GEN_STARTLOOPADDROFS)
306 -                                 + 32768 * (int) GEN(GEN_STARTLOOPADDRCOARSEOFS));
307 +                                 + (unsigned int) GEN(GEN_STARTLOOPADDROFS)
308 +                                 + 32768U * (unsigned int) GEN(GEN_STARTLOOPADDRCOARSEOFS));
309                          check_sample_sanity_flag = FLUID_SAMPLESANITY_CHECK;
310                          }
311                    break;
312 @@ -1200,8 +1200,8 @@ void Voice::update_param(int _gen)
313              case GEN_ENDLOOPADDRCOARSEOFS:       /* SF2.01 section 8.1.3 # 50 */
314                    if (sample != 0) {
315                          loopend = (sample->loopend
316 -                                  + (int) GEN(GEN_ENDLOOPADDROFS)
317 -                                  + 32768 * (int) GEN(GEN_ENDLOOPADDRCOARSEOFS));
318 +                                  + (unsigned int) GEN(GEN_ENDLOOPADDROFS)
319 +                                  + 32768U * (unsigned int) GEN(GEN_ENDLOOPADDRCOARSEOFS));
320                          check_sample_sanity_flag = FLUID_SAMPLESANITY_CHECK;
321                          }
322                    break;
323 @@ -1636,22 +1636,22 @@ float Voice::get_lower_boundary_for_atte
324   */
325  void Voice::check_sample_sanity()
326        {
327 -      int min_index_nonloop=(int) sample->start;
328 -      int max_index_nonloop=(int) sample->end;
329 +      unsigned int min_index_nonloop = sample->start;
330 +      unsigned int max_index_nonloop = sample->end;
331  
332        /* make sure we have enough samples surrounding the loop */
333 -      int min_index_loop=(int) sample->start + FLUID_MIN_LOOP_PAD;
334 -      int max_index_loop=(int) sample->end - FLUID_MIN_LOOP_PAD;
335 +      unsigned int min_index_loop = sample->start + FLUID_MIN_LOOP_PAD;
336 +      unsigned int max_index_loop = sample->end - FLUID_MIN_LOOP_PAD;
337        fluid_check_fpe("voice_check_sample_sanity start");
338  
339        if (!check_sample_sanity_flag)
340               return;
341  
342  #if 0
343 -      printf("Sample from %i to %i\n", sample->start, sample->end);
344 -      printf("Sample loop from %i %i\n", sample->loopstart, sample->loopend);
345 -      printf("Playback from %i to %i\n", start, end);
346 -      printf("Playback loop from %i to %i\n", loopstart, loopend);
347 +      printf("Sample from %u to %u\n", sample->start, sample->end);
348 +      printf("Sample loop from %u to %u\n", sample->loopstart, sample->loopend);
349 +      printf("Playback from %u to %u\n", start, end);
350 +      printf("Playback loop from %u to %u\n", loopstart, loopend);
351  #endif
352  
353        /* Keep the start point within the sample data */
354 @@ -1668,7 +1668,7 @@ void Voice::check_sample_sanity()
355  
356        /* Keep start and end point in the right order */
357        if (start > end) {
358 -            int temp = start;
359 +            unsigned int temp = start;
360              start    = end;
361              end      = temp;
362              }
363 @@ -1694,7 +1694,7 @@ void Voice::check_sample_sanity()
364  
365              /* Keep loop start and end point in the right order */
366              if (loopstart > loopend){
367 -                   int temp  = loopstart;
368 +                   unsigned int temp  = loopstart;
369                     loopstart = loopend;
370                     loopend   = temp;
371                    }
372 @@ -1706,7 +1706,7 @@ void Voice::check_sample_sanity()
373              /* The loop points may have changed. Obtain a new estimate for the loop volume. */
374              /* Is the voice loop within the sample loop?
375               */
376 -            if ((int)loopstart >= (int)sample->loopstart && (int)loopend <= (int)sample->loopend){
377 +            if (loopstart >= sample->loopstart && loopend <= sample->loopend){
378                     /* Is there a valid peak amplitude available for the loop? */
379                     if (sample->amplitude_that_reaches_noise_floor_is_valid) {
380                           amplitude_that_reaches_noise_floor_loop = sample->amplitude_that_reaches_noise_floor;
381 @@ -1745,13 +1745,13 @@ void Voice::check_sample_sanity()
382              * the sample, enter the loop and proceed as expected => no
383              * actions required.
384              */
385 -            int index_in_sample = phase.index();
386 +            unsigned int index_in_sample = (unsigned int)phase.index();
387              if (index_in_sample >= loopend) {
388                     /* FLUID_LOG(FLUID_DBG, "Loop / sample sanity check: Phase after 2nd loop point!"); */
389                     phase.setInt(loopstart);
390                    }
391              }
392 -/*    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); */
393 +/*    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); */
394  
395      /* Sample sanity has been assured. Don't check again, until some
396         sample parameter is changed by modulation.
397 @@ -1795,7 +1795,6 @@ void Sample::optimize()
398        signed short peak;
399        float normalized_amplitude_during_loop;
400        double result;
401 -      int i;
402  
403        /* ignore ROM and other(?) invalid samples */
404        if (!s->valid())
405 @@ -1803,7 +1802,7 @@ void Sample::optimize()
406  
407        if (!s->amplitude_that_reaches_noise_floor_is_valid) { /* Only once */
408              /* Scan the loop */
409 -            for (i = (int)s->loopstart; i < (int) s->loopend; i ++) {
410 +            for (size_t i = s->loopstart; i < s->loopend; i++) {
411                    signed short val = s->data[i];
412                    if (val > peak_max)
413                          peak_max = val;
414 --- a/fluid/voice.h
415 +++ b/fluid/voice.h
416 @@ -134,10 +134,10 @@ public:
417         float root_pitch, root_pitch_hz;
418  
419         /* sample and loop start and end points (offset in sample memory).  */
420 -       int start;
421 -       int end;
422 -       int loopstart;
423 -       int loopend;
424 +       unsigned int start;
425 +       unsigned int end;
426 +       unsigned int loopstart;
427 +       unsigned int loopend;
428  
429         /* vol env */
430         fluid_env_data_t volenv_data[FLUID_VOICE_ENVLAST];