rconstanzo / karma

The karma~ external (for Max) is a dynamically lengthed varispeed record/playback looper object with complex functionality.
http://www.rodrigoconstanzo.com/karma
103 stars 12 forks source link

List reporting output stops when executing a 'stop' message #63

Open menderchuck opened 6 years ago

menderchuck commented 6 years ago

List reporting output stops when executing a 'stop' message.

In a patch with a moderate use of scheduled clocks, sending a 'stop' message stops the transport but also stops the list output.

Steps to reproduce: 1) Send play message. 2) Send stop message.

Notice that the list output has stopped. Also, notice that the last reported 'play' state remains 1.

Taking a look at the source, it seems like the intent is for list output to stop when the transport is in stopped state. However, constant list output while in stopped state has been the behavior since 1.0. In this case, the clock is being set and then unset on each subsequent perform callback.

The problem that causes the list output to stop (the reporting clock is never rescheduled) is the fact that the scheduled clock ( clock_delay(x->clock, 0) ) is being unset before it fires the reporting callback.

Here's the test patch which is a little weird, but it was the first idea that reproduced the problem. I/O vector size should be set to 64 or less.


----------begin_max5_patcher----------
1637.3oc2btzaiaCDG+rymBA0asYCH4PpG8VaOrm5odnnXwh.YaZGsQVxPO1
MoK18ydoHoTbbhkGulxEQI.1xTOl4+73mXHjyWuZl+7hGjU9d+p2G7lM6qWM
aldn1AlY+7L+MIOrHKoReX94xuTL+S9Wa1Us7gZ8vYEIKmmjutaGU0OlI06o
aj7lMo4YxZ8kg9zfEM06O51j5E2klu91R4hZiywn2Pt1iBQsuEPZekwtg38Q
6ojtTaLkq8NJzYRykt9wsRyEwW6gdercue6pqZe4ZjxdUVgxWOr5VUTtIQ6q
A30K6v5kBhVMJDraDp2FVvT+9KTYxFYsr7VYdxbiKRNTvv+5yIfnTxbY4Ykt
GP9Lk7E8xGfAkOYxIeZLfV9w+en9WGBzjuMYw8dq7RU+tR+a5YEhBOdCBk1F
hh4CEhBNHPP0Vq5ZUwhz7ceqe3WtQ6Q3xXVsWl27QqNxDj3QZ9ALHEQD5ONE
KajUUIqkuP4J2rnbIRoyN0aXDpqH3DcuCWLH.IheXo6TIWUWrcrDrREscBVE
OLy.dhYNxJdaVxiilhsU2nTLOxwJ9.8y2mnlHv289Ixs0xp5yR5CzXaZn4l4
ELL7C3wGT4UoqySxZauyRqbKWadypUxx9.gGkn9Yr3bBtYZg55AJKb3tcD2N
3Gm1MuottHerlFrHHVqMHP+Fu60WEnyGg4AefrsZdGkEdTxX0p2oahN0JhF7
FYhKmt+4wWyTycx.1PZdfdbWOgkp7jsU2UT+8KfzM+4dQCR1EzizN6Rw+KJU
eyHq5XcMNLbMN6XLcWp57hzJ42GYll4OpmGLnrgQQ1iKylaZfQwr4roA6xpY
brK3hwtF6aS0kpwbaJ9E71yWBlcWJGCylKlJLaqpwvr4ASGlsU1XX17v2fLa
HD+7rgfoAy1pYTLaHbxvr6R0XX1PzzhY2kxwvrg3oBy1pZTLaxzgYakMJlM8
MHylEged1.YZvrsZFGylNYX1coZTLa1zhY2kxQwrgoBy1pZLLafOcX1VYigY
Ch2fLaZD94Yy3SClsUynX1LwjgY2kpwvrYASKlcWJGCylENUX1VUigYyhlNL
aqrwvrYwuAY1wBzH6voAw1nXT.6oCu1llQsT1SKZsMciZB1SEVsQynP0SGRs
QzX.0m15gnuB9Yo46+nUq8g1wedjnpnobQ2Ez9.K5Q68ikxp5z7j5TES+oiQ
cqCOxAh0mhcHGwNsOcwNwPriYHhCLT6Sw3QkTfKLj.ggZeDbNeKAXrDyEVhg
wRtndn6hLnk3tnDGSZhaZ2NK6z4rCG6DtvRXJw4tnFmigOvEtvRnRSfKrDGi
kbQ2DGSeK2I0dn5lbAds6hLnk.Wz2hIME5.6.X5aAmXoHLVxE8s.FBg1cN2J
O.SeK3h6BpeLlO5jhDNIQwQkobxsm.L04fK.e.FbD3hatCXlFA3BbDPIWtLE
FHqV5mcSElYR3.Ewvf9XQtvRXZnXtn2kgA8wbRzCyjiXt.8wv.YYANnxCUiq
CTj9ab0EpukBXmX9SGTQ4RYo96O43a58lPZuooW.S+5VVL9Vdu4B0aZ1Evzr
W2zv3a58PF8lledlFikeQijYMpR1t8yxxJ6AqMg+ljOUT1+su0eSZt4if9ik
xOm1c7b8HIkKtKsVtntozr3bOX+FV4uoPIv7lTa4rRbWYWDu8VWr7jMlS82Z
VlV7W0I0MU29mx7FyxtoD0pjlr5mGHludUZV1hhLi6MqeOy76VeNeydu1Nb+
w9AOxMrXNkFoBK2.THjEn2RsgPryzLsmCs6j3BdLg0dn7.VHWn2JhABncKxd
mF6IaQnwFKPhiHbyVpgnJas6okju1rBmrv1UkzN51xhsEkcoT0oF2e7M0EqK
SVlJyq64UlTrIba9dgq169qcp+FU.Ms6b1stqOYbeUVpJA92sY2gRD6DVM+W
ZX2W5J5NSewr9v+dVirasXeceIYwBkAdVhFngA7v1PdPe1KtcH5H3fkEeI+j
8vm7Kgp9iGqKOFIO7OdL4zcPBKVv00ugg.vzAyHFkRA26fuuTJ+A7PczCZ+o
cKZPTPqa5Zm6ejYYEe4j8tHlfpCZgQzHpFUHTQTwy598WUn7rz+0x1or1ChP
hbk.z+Gq38EYKOY2usTjnS+hXFUnUBcW7kSbvRCto0CQRaTcN7f.H1VNFFZJ
LUA1X34g14qGaAoNmuc0+ADNPd3J
-----------end_max5_patcher-----------
steadykammer commented 6 years ago

hi there @menderchuck . sorry for delay in reply - i had to take a couple weeks off for a different project. firstly, surely you meant "signal vector size should be set to 64 or less", not "I/O vector size should be set to 64 or less"? i see the source code is funky regards ordering of clock, but for me i cannot reproduce the play staying at '1' bug. also, i am struggling to understand why we need all the scheduled events in your demo patch? they all just carry on fine. am i missing something?

menderchuck commented 6 years ago

hi, thanks a lot for getting back to me. i haven't spent any more time on this, but i hopefully will soon.

On Mar 06, 2018, at 08:36 AM, stykmr notifications@github.com wrote:

hi there menderchuck. sorry for delay in reply - i had to take a couple weeks off for a different project. firstly, surely you meant "signal vector size should be set to 64 or less", not "I/O vector size should be set to 64 or less"?

yeah, i don't have a clear understanding of the difference between those settings. in any case, yes, signal vector size. set it to 32.  

i see the source code is funky regards ordering of clock, but for me i cannot reproduce the play staying at '1' bug. also, i am struggling to understand why we need all the scheduled events in your demo patch? they all just carry on fine. am i missing something?

the purpose of rapidly setting the period for multiple metro objects is to execute more internal clock processing. that is, schedule many random period clock callbacks to make the race condition i described more likely to occur. if you double the metro instances, that should do it.

on my iMac and MacBook pro, Max 7.3.4, it's 100% reproducible.

would you be okay with a change to the current reporting behavior when the transport is stopped? it's pretty obvious that constantly outputting the list when stopped (while setting/resetting the clock on every other callback) isn't the intent in the code. it would be pretty confusing to preserve that in the code while working on this issue.

anyway, i'm going to take a stab at fixing this. there's no workaround for me in my main patch.

thanks!   — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

steadykammer commented 6 years ago

thanks! comments inline...

On 8 Mar 2018, at 08:11, menderchuck notifications@github.com wrote:

hi, thanks a lot for getting back to me. i haven't spent any more time on this, but i hopefully will soon.

On Mar 06, 2018, at 08:36 AM, stykmr notifications@github.com wrote:

hi there menderchuck. sorry for delay in reply - i had to take a couple weeks off for a different project. firstly, surely you meant "signal vector size should be set to 64 or less", not "I/O vector size should be set to 64 or less"?

yeah, i don't have a clear understanding of the difference between those settings. in any case, yes, signal vector size. set it to 32.

i see the source code is funky regards ordering of clock, but for me i cannot reproduce the play staying at '1' bug. also, i am struggling to understand why we need all the scheduled events in your demo patch? they all just carry on fine. am i missing something?

the purpose of rapidly setting the period for multiple metro objects is to execute more internal clock processing. that is, schedule many random period clock callbacks to make the race condition i described more likely to occur. if you double the metro instances, that should do it.

on my iMac and MacBook pro, Max 7.3.4, it's 100% reproducible.

yeah i am an idiot. indeed i understand the race conditions test now. yes i will double it and look again.

however, be warned that addressing these schedular issues in the current code base may be near impossible, and need to wait until i rewrite it all for version 2 before being more robust.

would you be okay with a change to the current reporting behavior when the transport is stopped? it's pretty obvious that constantly outputting the list when stopped (while setting/resetting the clock on every other callback) isn't the intent in the code. it would be pretty confusing to preserve that in the code while working on this issue.

anyway, i'm going to take a stab at fixing this. there's no workaround for me in my main patch.

yes it is a no brainer to fix this. let me know any ideas. in the mean time i will take a look. it is just another condition to test against so it should be pretty simple.

thanks!