snd-add mem leak and snd-xform bugs EXPLAINED!

[Paul L]

I haven't yet built Audacity to test these fixes, but I think I found the causes of known bugs while reading some source code this weekend.

I refer to source code versions found here: http://code.google.com/p/audacity/sourc ... 253Dclosed

• The memory leak discussed in this thread: http://forum.audacityteam.org/viewtopic ... 39&t=75022
  I believe this is the fault of lines 471:

  Code: Select all

          susp->s1 = NULL;

  and 721:

  Code: Select all

          susp->s2 = NULL;

  of file add.c. I believe these lines can just be commented out without harm. I believe the bug in the addition of two sounds will happen whenever two sounds are added with snd-add, and the sum is completely evaluated, and one sound outlasts the physical stop of the other sound (by at least one internal block of samples), and this later-ending sound is not otherwise referenced: then it leaks. Note that if you add many sounds with sim or sum, the last two are added with snd-add, and the others accumulated right to left onto that result with more calls to snd-add. Perhaps nobody detected this common occurrence until I wrote effects that cause this to happen many thousands of times?
• The problems with results of snd-xform given to snd-add, with the resulting sound sometimes incorrect in a way dependent on the sequence of the summands, discussed here: http://forum.audacityteam.org/viewtopic ... 39&t=72398
  
  The fault is again in add.c, not in snd-xform. Change line 405 to:

  Code: Select all

              susp->s1->get_next == SND_get_next && susp->s1->stop == MAX_STOP &&

  and analogously at line 650:

  Code: Select all

              susp->s2->get_next == SND_get_next && susp->s2->stop == MAX_STOP &&
  

  Those changes will have the effect of disabling some "optimizations" in snd-add for more structure sharing, which were applied incorrectly to sounds that were shortened by snd-xform: the shortening was ignored and the underlying sound sequence copied to its physical stop. A better fix might be written that does not sacrifice sharing in this case, but it would not be as simple.
  
  To see the bug, generate 1 second of a 100Hz wave, do this in Nyquist prompt:

  Code: Select all

          (defun extract-global (snd start end)
            (let* ((t0 (snd-t0 snd))
              (result
               (shift-time (snd-xform snd (snd-srate snd) t0 start end 1.0) start)))
              result))
  
      (let* (
  
      (s1 (extract-global s 0.5 0.51))
      (s2 (extract-global s 0.6 0.61))
      (s3 (extract-global s 0.7 0.71))
  
      )
   (sum s3 s2 s1)
  )

  Whereas if you change the result to (sum s1 s2 s3), you get a different (and correct) result, which should just be three isolated samples of the original wave.

[steve]

Nyquist is developed and maintained by Roger Dannenberg at CMU. He rarely visits this forum, so you would probably do better to contact him directly.

[Paul L]

I send rbd a message via this board, which should appear in his email.

Is there an official source repository for Nyquist separate from Audacity? So we don't fork the library?

I know you were concerned to track the memory leak bug, and now I think I know the source code mistake.

[steve]

Is there an official source repository for Nyquist separate from Audacity? So we don't fork the library?

The Nyquist home page is here: http://www.cs.cmu.edu/~music/music.software.html

[Paul L]

What about Xlisp?

I was pondering how Xlisp might be made faster but maybe I should curb that enthusiasm.

[steve]

What about Xlisp?

http://www.xlisp.org/

[Paul L]

We use Xlisp 2.0, which is based on a subset of Common Lisp not Scheme, and besides 3.0 has not been updated in over ten years!

So can we feel free to fork it?

I was fantasizing about compiling Lisp, not necessarily to machine code but to a more rapidly interpreted form with optimizations applied. It could be someone's summer project.

[steve]

The problems with results of snd-xform given to snd-add, with the resulting sound sometimes incorrect in a way dependent on the sequence of the summands, discussed here: http://forum.audacityteam.org/viewtopic ... 39&t=72398

The fault is again in add.c, not in snd-xform. Change line 405 to:

Code: Select all
susp->s1->get_next == SND_get_next && susp->s1->stop != MAX_STOP &&



and analogously at line 650:

Code: Select all
susp->s2->get_next == SND_get_next && susp->s2->stop != MAX_STOP &&



Those changes will have the effect of disabling some "optimizations" in snd-add for more structure sharing, which were applied incorrectly to sounds that were shortened by snd-xform: the shortening was ignored and the underlying sound sequence copied to its physical stop. A better fix might be written that does not sacrifice sharing in this case, but it would not be as simple.

To see the bug, generate 1 second of a 100Hz wave, do this in Nyquist prompt:

Code: Select all
(defun extract-global (snd start end)
(let* ((t0 (snd-t0 snd))
(result
(shift-time (snd-xform snd (snd-srate snd) t0 start end 1.0) start)))
result))

(let* (

(s1 (extract-global s 0.5 0.51))
(s2 (extract-global s 0.6 0.61))
(s3 (extract-global s 0.7 0.71))

)
(sum s3 s2 s1)
)



Whereas if you change the result to (sum s1 s2 s3), you get a different (and correct) result, which should just be three isolated samples of the original wave.

I think you are close to a solution, but I tried that modification and your test case, but the result is still the same.

[Paul L]

Oh duhh, I wrote != where it should have been == and twice. I edited my previous posting.

Did you try the memory leak fix?

[Paul L]

At last! I built Audacity for Windows today, tried my fix, and verified that the sum does not depend on the sequence of the summands.

Now, to make a patch and publicize it in appropriate boards.