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

Load samples asynchronously in a separate thread #21

Merged
merged 1 commit into from Feb 16, 2015

Conversation

munshkr
Copy link
Member

@munshkr munshkr commented Feb 16, 2015

Whenever Dirt needs to load up a new sample from disk, it now creates a thread for reading and loading sound data into cache. Meanwhile the main thread continues playing, solving the problem with large sample files blocking the main execution thread.

To keep it simple, I made it so that there can only be one thread for file reading (that is, there are no queues), and to enforce that, there is a boolean variable (and its corresponding mutex) that determines if there is actually one running. If there is, skip file loading.

This changeset has a (nice) side effect that makes Dirt play a sample at a specific time only if it's already loaded in the samples cache, meaning that the first time you try to play an unloaded sample, it will not actually play until the next "tick" after it's been loaded in the cache. I think this is actually better than the old behavior, because now it will always play a sample in the right moment, even if it's delayed a bit more than before (because it used to play just after loading it).

@munshkr
Copy link
Member Author

munshkr commented Feb 16, 2015

Hmm, I just came across a sample that causes Dirt to segfault, so let me check and test a bit more, just in case.

Whenever Dirt needs to load up a new sample from disk, it now creates a
thread for reading and loading sound data into cache.  Meanwhile the
main thread continues playing, solving the problem with large sample
files blocking the main execution thread.

To keep it simple, there can only be one thread for file reading
(no queue), and to enforce that, there is a boolean variable (and its
corresponding mutex) that determines if there is actually one running.
@munshkr
Copy link
Member Author

munshkr commented Feb 16, 2015

OK, there was a small bug, I think it's fixed :)

yaxu added a commit that referenced this pull request Feb 16, 2015
Load samples asynchronously in a separate thread
@yaxu yaxu merged commit 0771b64 into tidalcycles:master Feb 16, 2015
@yaxu
Copy link
Member

yaxu commented Feb 16, 2015

Thanks for this
I merged it but I don't like the behaviour as it stands.

E.g. if I run this:

d1 $ sound "bd sn:2*2"

then this:

d1 $ sound "bd:2 sn:2*2"

the first time round the bass drum isn't played. It'd be much better if it was late by a few samples in that case.

I guess part of the problem is when it starts loading the sample. It should do that as soon as the OSC message comes in.. I'll have a look into it.

@yaxu
Copy link
Member

yaxu commented Feb 16, 2015

Sorry I reverted it again, this change of behaviour won't work out in many situations.

I'm surprised this doesn't work out better, there's 0.04 seconds of latency in which dirt can load its samples, and as far as I can tell dirt is using it.

I think what we need though is a way that sample triggers are still queued even if samples aren't loaded yet, and only get dequeued when they are.
This could be a runtime option - whether to drop late triggers or not.

@munshkr
Copy link
Member Author

munshkr commented Feb 16, 2015

I see what you mean... For small samples, it should take little time and could play them as soon as it's loaded. Do you think preloading as soon as Dirt receives the OSC message would solve this problem in most cases?

I think what we need though is a way that sample triggers are still queued even if samples aren't loaded yet, and only get dequeued when they are.
This could be a runtime option - whether to drop late triggers or not.

If I understood correctly, you're saying that when file_get fails to get a cached sample and needs to read from disk, it should queue that audio_play call, and when sample is finally loaded, dequeue and retrigger it?

@yaxu
Copy link
Member

yaxu commented Feb 16, 2015

Do you think preloading as soon as Dirt receives the OSC message would solve this problem in most cases?
Yes although from my brief look, it seems to be doing that already. I could be wrong!

If I understood correctly, you're saying that when file_get fails to get a cached sample and needs to read from disk, it should queue that audio_play call, and when sample is finally loaded, dequeue and retrigger it?

Yes, not sure whether it needs a special queue for this, I guess that would be the ideal situation..

@munshkr
Copy link
Member Author

munshkr commented Feb 16, 2015

👍 I'll try to solve this and let you know. Thanks!

@munshkr munshkr deleted the nonblocking-read branch February 17, 2015 02:59
@munshkr
Copy link
Member Author

munshkr commented Feb 17, 2015

@yaxu OK, I've been working on this and I think I managed to solve the issue you mentioned...

Basically Dirt now calls audio_play again, with the same arguments, as soon as the sample is loaded. There was no need for a queue because a thread only handles one sample so I just needed to pass a copy of the arguments to the thread start routine as a callback function.

I also added a command-line option to enable or disable late triggering, just in case someone doesn't want late triggers. I left it enabled as default so it behaves as always. Because, actually, most of the time Dirt plays samples really fast :) Especially if you keep them normalized to skip samplerate conversion, for instance.

I'll keep testing it a bit more before submitting a new PR, but you can have a look here if you want.

lennart pushed a commit to lennart/Dirt that referenced this pull request Jul 11, 2015
Load samples asynchronously in a separate thread
yaxu added a commit that referenced this pull request Nov 1, 2015
Load samples asynchronously in a separate thread
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