search

monome / norns

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

tape: return false immediately on open() if stream is running #1770

Closed catfact closed 8 months ago

catfact commented 8 months ago

this is a quick bandaid to begin addressing https://github.com/monome/norns/issues/1758

previous to the change, calling open() on a running stream would bork the state machine between disk loop, audio and main threads.

with the change, calling open() on a running stream does nothing. not ideal but at least it keeps working.

next, i'll try and update such that the same action starts a fadeout and blocks the caller until fadeout is complete. i'd recommend merging this change first and i'll build on it (can't promise a timeline)

i think with this change, debouncing in lib/fileselect.lua becomes unnecessary, but haven't verified that by actually rewriting the module.

catfact commented 8 months ago

... oh, maybe it would make sense to return boolean result of open() back to lua also.

tehn commented 8 months ago

will create a rollback for https://github.com/monome/norns/pull/1759 and test

catfact commented 8 months ago

...I forgot. Of course this is happening over OSC. So I can't return an error code, and I can't block the main thread. (Needs norns-converged.)

I can block the OSC server thread. That might be OK. But it will introduce some timing hitch around the action.

tehn commented 8 months ago

the fact that it returns and doesn't try to play i believe makes it so the previous bug isn't happening (i tested on hardware, though perhaps i'm clueless as to how the original bug was manifesting)

currently the UI is sometimes not-reflective of the play state (ie, says it's playing but it's not) but that's not what i perceived the original bug to be

catfact commented 8 months ago

Yes that's what I think too. Can't say I've ever used fileselect or the preview feature.

But without the change it's very easy to break the state of the tape module. All you need to do is call open without first calling stop and waiting for the fadeout to finish and file handle to close.

Even with the change, I can imagine it being frustrating to design an interface when you can't know the state of the player directly and - don't know if a given call to open succeeds. For example if you trigger playback immediately on selecting a scrolling list entry. If you land on the entry you want and it doesn't play (still busy) then you try and scroll away and it plays an adjacent entry and still doesn't let you select the one you're after.

But lua-side timeouts just implement the same behavior with much less precision, so at least I don't see this being worse.

(Unless: does the present timeout actually implement a queue? Or just discard input during the timeout period. Sorry I didn't look)