pure-data / pddp

Pure Data Documentation Project
9 stars 2 forks source link

4.data.structures/09.sequencer.pd regression bug in 0.55 test #208

Closed maftkd closed 4 months ago

maftkd commented 4 months ago

Hello Pd Community,

Patch 09 creates a scalar-based sequencer, where the position of the scalar on the canvas dictates pitch, amp, & timing.

I believe there may be a timing issue in the patch, but I'm not sure. I've attached an image of the subpatch with scalars below:

image

The easiest way to detect what I believe to be the timing error, is listen for the shape at "A" almost a whip-lash sound. And then listen for "B" a low-pitched kick-like sound. The way it is laid out makes it appear that I should expect "B" to play right after "A", but in truth there is a significant delay.

I will take a closer look later, but being fairly new to Pd still, it will take me some time to investigate. If anyone with more experience has a chance to jump in and check it out, that would be much appreciated!

porres commented 4 months ago

hey are you testing this with pd version 0.55test? Cause I saw the delay over there, but not in 0.54-1!!! I know I changed things in 0.53 so I went to check there first, but it turns out things are good (phew).

Now, I don't remember changing things for 0.55, but I might and, wow, nice catch! As there's still time to fix it... let me see

EDIT: Ok, now I remember and I obviously ruined it, thanks again for the catch!

I reverted things in https://github.com/pure-data/pure-data/pull/2316 and it's all good now, I will try to make the same changes I did without ruining it before merging into the documentation branch

porres commented 4 months ago

Ok, so, this is what I did that ruined things, and I can't make any sense of it... in [voice] I had removed this completely redundanct [s] and [r] pair and just connected things straight. Note that the name doesn't matter, I can rename to whatever so no, this send/receive name is not used anywhere else. It just makes no sense at all and I still can't for the life of me say how this ruins things... so yeah, I missed the bug.

Screen Shot 2024-05-18 at 16 50 46

I will keep investigating how this change ruins everything.... I suspected that the send and receive interferes with the execution order of messages, so the patch has a bug elsewhere. The thing is that substituting send/receive for [pipe 0] also ruined it... will keep looking.

porres commented 4 months ago

hmmmm.... so, added some [print] objects to debug.

Screen Shot 2024-05-18 at 17 09 50

So, with the [s]/[r] I get

dif: 22; time-of-last-evt: 22; time-of-last-evt: 22; time-of-last-evt: 22; time-of-last-evt: 22; bang: bang; dif: 11; time-of-last-evt: 33; time-of-last-evt: 33; time-of-last-evt: 33; time-of-last-evt: 33; bang: bang; dif: 24; time-of-last-evt: 57; time-of-last-evt: 57; time-of-last-evt: 57; time-of-last-evt: 57; bang: bang; dif: 36; time-of-last-evt: 93; time-of-last-evt: 93; time-of-last-evt: 93; time-of-last-evt: 93; bang: bang; dif: 68; time-of-last-evt: 161; time-of-last-evt: 161; time-of-last-evt: 161; time-of-last-evt: 161; bang: bang; dif: 177; time-of-last-evt: 338; time-of-last-evt: 338; time-of-last-evt: 338; time-of-last-evt: 338; bang: bang;

And connecting it straight (as in the screenshot above) I get

dif: 22; time-of-last-evt: 22; bang: bang; dif: 33; time-of-last-evt: 33; bang: bang; dif: 57; time-of-last-evt: 57; bang: bang; dif: 93; time-of-last-evt: 93; bang: bang; dif: 139v time-of-last-evt: 161; bang: bang; dif: 305; time-of-last-evt: 338; bang: bang;

porres commented 4 months ago

So, fits thing I realized... [voice] is an abstraction loaded in [clone], so maybe that's why I'm getting repeated values... and with '$0' in send/receive I get the same 'wrong' values... and for the record, the 'wrong' version does not get the differences right... it just accumulates all values.

So yeah, the original version does work but it is bugged... hmmmm.....

I still can't see why, but I'll get there, only now I gotta go, see ya!

maftkd commented 4 months ago

Thanks so much for checking it out!

So just to clarify the problem is not the note-duration, but the delay between notes - or at least that's what the prints are indicating.

I think I see the issue. And as you said, it would be because the voice is a cloned abstraction with 4 instances. We are trying to get the difference between the current scalar's X and the previous's X via the [-] block. The problem is that we aren't actually persisting the previous X in the cold inlet due to the clone 4

image

I see that for the first 4 notes (since 4 clones) the cold inlet will be 0, which is why diff = time-of-last-evt for the first 4 print-outs.

What do you think would be a good way to work around this? One thought I had: Does PD support global floats? If so, could we have one global float for cur-time, and then compare against that in each voice? Alternatively, within [pd sequence], could we simply step-back through the pointer to get the previous template's X? Instead of relying on it persisting in the cold inlet mentioned above?

maftkd commented 4 months ago

Actually I think I get why you used the send / receive initially. Was it to make the previous note's X sort of a "global" float, so all the voices had access to it?

If so, should we just revert to the send/receive approach for previous note's X?

porres commented 4 months ago

Actually I think I get why you used the send / receive initially.

I didn't create this example but I adapted it to use [clone] with the intent to modernize it and I just wrapped the abstraction and tested and it did work. Now I was attempting to edit it in a way I thought made sense for the sake of code elegancy and things broke, but the previous version is quite flawed anyway, despite working

porres commented 4 months ago

I didn't create this example but I adapted it to use [clone] with the intent to modernize it and I just wrapped the abstraction and tested and it did work.

It did seem to work, but I now suspected and I obviously ruined it a bit then. Getting the output from good old pd-extended we have

dif: 3 time-of-last-evt: 3 bang: bang dif: 11 time-of-last-evt: 14 bang: bang dif: 24 time-of-last-evt: 38 bang: bang dif: 26 time-of-last-evt: 64 bang: bang dif: 78 time-of-last-evt: 142 bang: bang dif: 177 time-of-last-evt: 319 bang: bang

porres commented 4 months ago

Ok, using a [value] object as a global value fixes things and makes it all elegant

porres commented 4 months ago

[pd sequencer] doesn't seem to be necessary inside [clone], it might be much more elegant to have it outside

porres commented 4 months ago

it might be much more elegant to have it outside

It certainly is, and by doing so I can keep using [t f f] as I was doing and no [value] object is needed to keep track of a global variable shared by all instances

porres commented 4 months ago

If you don't mind I am rebooting this issue

maftkd commented 4 months ago

Not sure what that means, but I trust you @porres Reboot away!