monome / norns

norns is many sound instruments.
http://monome.org
GNU General Public License v3.0
630 stars 146 forks source link

softcut: broken loop positions #868

Closed tehn closed 5 years ago

tehn commented 5 years ago

EDIT: the problem is actually loop setting doesn't seem to get the length right

somewhere since 2.0 phase polls broke. (i think but can't 100% confirm i recall things working properly in 2.0 but the error reports came in at 2.1)

here's a test script:

position = 0

function update_position(i,pos)
  print(i,pos)
  position = pos-1
  redraw()
end

function init()
    softcut.enable(1,1)
    softcut.buffer(1,1)
    softcut.rate(1,0.5)
    softcut.loop(1,1)
    softcut.loop_start(1,1)
    softcut.loop_end(1,3)
    softcut.position(1,1)
    softcut.play(1,1)
    softcut.fade_time(1,0)
    softcut.phase_quant(1,0.125)
    softcut.phase_offset(1,0)

  softcut.event_phase(update_position)
  softcut.poll_start_phase()
end

function redraw()
  screen.clear()
  screen.move(10,20)
  screen.line_rel(position*32,0)
  screen.stroke()
  screen.update()
end

gives numbers like this:

1   0.875
1   1.0
1   1.125
1   1.25
1   1.375
1   1.5
1   1.625
1   1.75
1   1.875
1   2.0
1   2.125
1   2.25
1   2.375
1   2.5
1   2.625
1   2.75
1   0.875
1   1.0

should be confined from 1.0 to less than 3.0. also the interval of reports is jumpy around the loop cross. note that this is with offset=0 which should make the calc really simple.

the phase offset/quant calc happens here: https://github.com/monome/norns/blob/master/crone/src/softcut/SoftCutVoice.cpp#L243 (sorry about the confusing formatting in line 244)

but that hasn't been touched for some time. will dig in deeper to see if somehow something else went weird... the recent commits are mostly about SVF and reset, so i can't see what's going on yet

tehn commented 5 years ago

here's the compare view: https://github.com/monome/norns/compare/v2.0.0...master

tehn commented 5 years ago

hold on... phase_quant_offset wasn't in 2.0??

https://github.com/monome/norns/compare/v2.0.0...master#diff-addc4bc2f21cbb4bdbc5d8ac3cfc3000R695

tehn commented 5 years ago

setting phase_quant(1,0.01) (offset=0) seems the loop jumps a full 0.1s early

1   0.89999997615814
1   0.9099999666214
1   0.0
1   0.0099999997764826
1   0.019999999552965

fade_time is set to 0.

next test is to do something with audio to see if the loops are actually the correct length

tehn commented 5 years ago

doesn't seem the loop is doing the full range from start-end.

with softcut.phase_quant(1,0.5) and measuring intervals with util.time() i'm seeing

0.49902319908142
0.41796183586121
0.50134515762329
0.41917896270752
0.49909901618958
0.42041492462158

so the 0.1 jump is really 0.08ish. i'll check this interval via audio.

@catfact i'm now in the swamp... i'll get back on this later this evening

tehn commented 5 years ago

git checkout v2.0.1 and now i'm getting

0.50048303604126
0.49936294555664
0.50128197669983
0.49816203117371
0.50071716308594
0.49924397468567
0.5014820098877
0.49843192100525
0.5013439655304
0.49826693534851
0.50146818161011
0.4986720085144
0.50117492675781
0.49830198287964
0.50213503837585
0.49874496459961

looks right. will start to look at crone loop logic...

edit: 2.0.1 seems to be working correctly and i'm pretty stumped by this compare: https://github.com/monome/norns/compare/v2.0.1...master?w=1

catfact commented 5 years ago

in 2.0.0, jitter b/c the old version explicitly fired from the audio thread when quantized value changed, new version simply polls at 1ms

it looks like a seconds/samples scaling thing with new version, or something

i'll fix it, but the real fix is a little work cause we actually have to fire updates from the audio thread as appropriate, but audio thread can't call lo_send (which allocates) so it has to be syncd with worker thread created by Poll and that needs different logic than when the worker thread is just sleeping between callbacks.

catfact commented 5 years ago

sorry i misread this slightly, so you mean the report times look wonky and the report values are merely offset (it's not a unit conversion thing)

the report times wonkiness i can understand, 1ms poll jittering against arbitrary quantization interval would make arbitrary patterns. working on fixing this with dedicated phase updater thread.

the offset thing i don't have an explanation for (yet, will keep looking), only relevant change i see is that offset is set to zero on reset.

tehn commented 5 years ago

apologies, this issue became unclear as i was logging by debug process here.

what i'm actually seeing now is what seems to be the loop length is incorrect.

here's a test script where i attempt to properly set all of the softcut values (after a reset):

t = 0

-- just print a delta time since last poll report
function update_position(i,pos)
  local now = util.time()
  print(now-t)
  t = now
end

function init()
    softcut.enable(1,1)
    softcut.buffer(1,1)
    softcut.rate(1,0.5)
    softcut.loop(1,1)
    softcut.loop_start(1,0)
    softcut.loop_end(1,1)
    softcut.position(1,0)
    softcut.play(1,1)
    softcut.fade_time(1,0)
    softcut.phase_quant(1,0.125)
    softcut.phase_offset(1,0)

  softcut.event_phase(update_position)
  softcut.poll_start_phase()
end

whereupon i get values like this:

0.49902319908142
0.41796183586121
0.50134515762329
0.41917896270752

but on v2.0.1 it's correct. (ie, values right around 0.5 every single line)

i'm going to resume looking at this shortly. i would've assumed that the new reset behavior simply introduced some softcut value i wasn't manually resetting, but i really can't locate it, and can't figure out what startup values would've changed vs. 2.0.1

tehn commented 5 years ago

ok did an audio recording (via TAPE, how convenient!) and measured my 1.0 second loop from the code above (added data to the buffer for an impulse)

loop length is ~0.92 so i'm not sure the polls are the problem, but i definitely still can't spot the issue with loop length. i'm initializing fade_time to 0. will keep looking

Screen Shot 2019-08-16 at 1 24 33 PM

tehn commented 5 years ago

re: sample rates. spotted this yesterday-- changed and tested w/o results but maybe something else is there

https://github.com/monome/norns/compare/v2.0.1...master?w=1#diff-5c85b2c2aef22672dd24907caf964a1cR90

sch.setSampleRate(sampleRate); was changed to `sch.setSampleRate(hz);

catfact commented 5 years ago

think the problem is this stupid assignment in init which should not be there https://github.com/monome/norns/blob/master/crone/src/softcut/SoftCutHead.cpp#L21

previously it didn't matter but now head::init() is being called by the new reset func

tehn commented 5 years ago

should it be 48000 or just removed? I can test this now

catfact commented 5 years ago

Probably removed

catfact commented 5 years ago

Does that address the poll values also?

I do have WIP to improve their timing accuracy and minimize traffic

tehn commented 5 years ago

yes fully fixes polls.

do you want me to hold off on an update?