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