[chuck-dev] LiSa revised - fixed many bugs (PATCH)

[Robin Haberkorn]

Hi Kassen,

hope I can clarify some of my motivations.

On 13/09/12 06:22, Kassen wrote:
> 
> I like this. I like it better than reversing under such conditions,
> which can also be done with a negative rate. It seems consistent with
> LiSa's "loopy" nature.
> 
>> Also, it allows all durations (like loopStart/loopEnd) to be negative or
>> larger than the buffer meaning duration from buffer end and duration
>> from buffer start respectively. I find this useful for calculations but
>> it breaks code like: lisa.duration() => lisa.loopEnd;
>> Since duration() is actually greater than the last addressable byte in
>> the buffer (which is at duration() - samp), so the expression sets
>> loopEnd to the first byte in the buffer. Lines like the above would have
>> to be changed into either:
>> lisa.duration() - samp => lisa.loopEnd;
>> or
>> -1*samp => lisa.loopEnd;
>>
> 
> Sorry, but I don't like this at all. I don't like it breaking that
> line which I suspect to be very common in LiSa-using code and I don't
> see the logic. IMHO this "-1" isn't beautiful or self-evident. 
> 

The line only worked previously because the value you give as loopEnd()
was automatically clipped to duration()-samp. Only loopEnd() was
clipped. On loopStart() you could specify arbitrarily large values,
likely crashing your VM... The idea was to standardize how buffer
positions are checked/processed.
Negative values are allowed so you can do calculations like this:
lisa.loopEnd() - second => lisa.loopStart;
and it will always mean 1 second before loopEnd, even if that duration
is negative.
E.g. if duration() == 2::second and loopEnd() == 500::ms, loopEnd() -
second == -500::ms was previously clipped to 0::samp when setting it as
loopStart(), so you only actually got a 500::ms loop. Of course
loopStart() had to be smaller than loopEnd(). If you had set it larger,
this effectively crashed your VM!!!
Now when using -500::ms => lisa.loopStart(), -500::ms is translated to a
position "from the right edge of the circular buffer", i.e. to 2::second
- 500::ms == 1.5::second. When you query lisa.loopStart() afterwards you
will get loopStart() == 1.5::second.

IMHO, allowing negative values is not that unintuitive. Popular
programming languages like Python interpret negative array indices in a
similar manner (this was taken from the Icon programming language...).

And yes, I also don't like that these changes break existing code.
That's why I asked for your opinion. Luckily the index-normalization
stuff can be separated from the looping-changes. Perhaps it's best to
"normalize" all durations uniformly by clipping to [0::samp,
duration()-samp] and if you want to do calculations like the above you
can still do them on your own (in ChucK code)...

lisa.loopEnd() - second => dur start;
if (start < 0::samp)
    lisa.duration() +=> start;
start => lisa.loopStart();

...which is not quite as powerful as my patch' normalization, but it
will work for this example. We could also add the "normalization" which
is nevertheless required for the internal getNextSamp() method, as a
LiSa member function, so you could write:

lisa.loopEnd() - second => lisa.normalize => lisa.loopStart;

Sounds like a good compromise, doesn't it? Even if you don't like that I
could live with it since I can still add that method to my own LiSa
derivation (written in ChucK).
If you agree I'll adapt my patch accordingly.

> If we simplify it all to a 2::samp buffer then the
> issue becomes clear; we have 3 values. What if we would make the end
> value (the 3rd, as programmers we call it "2") identical to the value
> at 0? When looping that causes no new issues. It also causes no issues
> that were not there anyway when not looping; if that would cause
> discontinuities  then you had non-0 points at the beginning or end
> anyway (or at least a non-identical beginning and end).
> 
I think you misunderstood. LiSa already worked that way: 2::samp =>
lisa.duration() meant you got 2 samples of data in your buffer, with
valid positions (e.g. for loopStart/loopEnd) 0::samp and 1::samp, in
other words 0::samp to duration()-samp.
My patch doesn't change that. The only change regarding buffer positions
is (or should be) that when setting parameters like loopStart, they are
*normalized* to values/durations between 0::samp and duration()-samp
*before* setting them and returning the normalized values.

> That would clear out this "-1" and it sounds like looping over a
> n::samp buffer would take (n+1)::samp of time with your plan (correct
> me if wrong) which will cause issues when dealing with guitarists
> and/or drummers.
> 
> With your solution I could make a square wave with duration/period of
> 1::samp (actually taking 2). With all due respect to your work that
> seems a bit silly to me. On the other hand; I'm open to my own
> suggestion here potentially causing more glitches in edge-cases like a
> LiSa used as a CV-signal ramp; we can't tell people to use SndBuf
> there as SndBuf can't be set procedurally as LiSa can, I think. 
> 
> Checks for looping or non-looping behaviour on the other hand add more
> complexity to LiSa herself and hardly will win any beauty awards
> either....
> 
The patch indeed introduces a little bit of overhead, even though the
looping/non-looping checks were (necessarily) already there, there is
now another check for the nature of looping. If loopStart > loopEnd, a
few more calculations and logic is required because of the possible
looping over the buffer end. On the other hand I think I optimized a
little bit, e.g. by caching the loop length which is required multiple
times in tick/tickf at sample rate but changes very infrequently (only
if loopStart(), loopEnd() or duration() changes). I also cleaned up the
code, at least where I touched it so it should be more readable now if
you would like to have a look.

> Erm... Dan? :-)
> 
> Kas.

What about the unary-minus-durations.ck (it's a patch, I accidentally
named it .ck)? It's a very simple change that seems consistent to me.

Best regards,
robin
> _______________________________________________
> chuck-dev mailing list
> chuck-dev at lists.cs.princeton.edu
> https://lists.cs.princeton.edu/mailman/listinfo/chuck-dev
>