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

[Kassen]

On Thu, Sep 13, 2012 at 05:39:51PM +0200, Robin Haberkorn wrote:
> Hi Kassen,
> 

Hey Robin!

> hope I can clarify some of my motivations.
> 

Yes, let's go over it but let me first clarify that it was just the
"-1" that I disliked and that I love the work you are doing. I think I
sounded a bit harsher than I meant to.

> 
> 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.

I do seem to remember we previously had a LiSa issue with loops being
off by 1 sample under some conditions. Dan fixed that. So; right now
we at least have a fix to this loopStart() issue, that is nice.


> 
> 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...).
> 

Yes, there I agree, even values beyond the end would arguably make
sense, in that a "loop" is really a bit like a infinitely long (if
monotonous) recording. So far we still agree.

> 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.
> 

This example looks a lot better to me, without the arbitrary "samp" in
it. If I have a duration called "beat" then I want to be able to poll
LiSa for the current loopStart() add a beat to it, set loopEnd() to
the result and then I want the result to always be a loop to exactly
1::beat. Maybe I misread your post, but where I balked was the
implication that we had to subtract a sample, sometimes, to get the
correct loop length.

> 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.
> 

I think we are "in tune" again. Would I be correct to say that LiSa
was "cheating" to enable us to do what I wrote above, that you
extension exposed that and that we now have to deal with this?

Mind you; I quite like your take. I I would do the above I would
currently need to check that loopStart+beat was smaller than the end
of the buffer. With your take that would no longer be needed and the
result would make sense; we'd just wrap. I like that a lot.

> 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.

That we will survive, I was hoping to somehow simplify it all until
things became simple and "obvious". Maybe the problem is simply hard.

> 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.

I'm going to have a look. Still need to build and install the latest
greatest.

I hope Dan will chime in here and that we can have your improvements
while keeping the nice "straightforward" reasoning about time and
loops. IMHO ChucK's syntax is there to make us type in things as they
appear to us with as little "adapting to the underlying system" as
possible.

Thanks for the clarifications and sorry again that I sounded a bit
harsh going from "I like this" to "I don't like that".

Yours,
Kas.