Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Mar 11, 2024

Conversation

catfact
Copy link
Collaborator

@catfact catfact commented Mar 10, 2024

this is a quick bandaid to begin addressing #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
Copy link
Collaborator Author

catfact commented Mar 10, 2024

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

@tehn
Copy link
Member

tehn commented Mar 11, 2024

will create a rollback for #1759 and test

@tehn tehn merged commit f5c3f3c into main Mar 11, 2024
@catfact
Copy link
Collaborator Author

catfact commented Mar 11, 2024

...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
Copy link
Member

tehn commented Mar 11, 2024

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
Copy link
Collaborator Author

catfact commented Mar 11, 2024

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants