search

davidedc / livecodelab

a web based livecoding environment
http://livecodelab.net/
MIT License
328 stars 63 forks source link

Timekeeper cleanup #241

Closed rumblesan closed 9 years ago

rumblesan commented 9 years ago

So these changes should make the time keeper and sound system a bit more solid.

There's still some work to do

There's also a question about how the BPM updating should work. Currently it works fine, but it will update at the next quarter note (i.e. every run of the beatLoop callback). This might not be the desired behaviour, so we should figure out what is

davidedc commented 9 years ago

can we merge the improvements to Pulse and the TimeKeeper without waiting for the full new web audio stack? So we make things more solid right now step by step and we try to minimise the risks of the long(er) shots?

rumblesan commented 9 years ago

I was thinking about this last night, and it's probably not a bad idea. that said, at the moment it shouldn't actually be that difficult to use one of the libraries as the IE fallback.

which one were we using for IE btw? it seemed like buzz was being used for the startup sound, but actually lowLag was doing all the other audio.

On 4 November 2014 04:43, Davide Della Casa notifications@github.com wrote:

can we merge the improvements to Pulse and the TimeKeeper without waiting for the full new web audio stack? So we make things more solid right now step by step and we try to minimise the risks of the long(er) shots?

— Reply to this email directly or view it on GitHub https://github.com/davidedc/livecodelab/pull/241#issuecomment-61592875.

noio commented 9 years ago

To reply to the first post: even if pulse integration is not so neat—or broken—it'll be easy to fix later. The feature probably has 0 users currently ;) .

The beat updating is tricky. If the beat is supplied externally, doing a delayed update is preferable because you want to wait for a next midi clock signal. Someone else (who does know what their doing) probably has a real solution for this.

davidedc commented 9 years ago

@rumblesan ...don't remember what was the policy for the different browsers...

I think there was a little framework in place to switch between players, was that unsuitable for adding webaudio?

rumblesan commented 9 years ago

OK, I've updated this so that if the web audio api isn't available it falls back to a version that uses buzz. seems to work ok, but i've not got an instance of IE to test with.

davidedc commented 9 years ago

I just asked at work for a previous-gen Dell laptop, I'll give this a go.

rumblesan commented 9 years ago

ah awesome, that would help. I've actually realised that there is a windows tower PC in the house, so might see if I can get that set up to test with whatever version of IE is on there. The setup with buzz there is pretty simple, but it should support IE 9, 10 and 11. Every other browser that we want to support should be fine with the web audio api, but I'll try to get more testing done

davidedc commented 9 years ago

audio examples play very oddly in my chrome/OSX, do they play OK for you?

rumblesan commented 9 years ago

ok, so this should be working better in Chrome and Firefox. Previously only the buzz version of the library was being used and that isn't especially optimised. Now the web audio api is being used and it seems pretty solid to me.

The buzz version needs a much improved voice system, but that's pretty much a solved problem.

A bigger problem is that currently the parser doesn't actually support spaces in strings, which means that we can't organise patterns like we normally would. I'm going to concentrate on fixing that, because I think it's going to require a pretty big overhaul of the grammar. Guess that's my weekend sorted heh

davidedc commented 9 years ago

nice, seems solid sound-wise, a couple of comments though:

rumblesan commented 9 years ago

ok, so the issue with rotation working weirdly was because the beat function in timekeeper wasn't giving back correct values. This is now fixed and stuff is working.

Clashes with the main thread aren't really an issue in browsers where the web audio api is being used because audio playback actually happens in another thread. That's one of the nice things about the web audio api. This can still be an issue when using the buzz audio api though, but I'm leaving out the animationLoop frame skipping stuff until I've had a chance to actually test on IE. Maybe you've noticed some issues though @davidedc

As for the frame not being cleared, yep I'm seeing that and need to fix it. Going to get that done next

davidedc commented 9 years ago

OK let's see how it comes out, perhaps not worth speculating too much, but I'd have thought that using the web audio api would allow us to schedule the next, say, 100ms sounds, at the time when the frame is painted. That would allow us to have only one set of intervals (the one for painting the frame) governing the sound with higher precision. If I understand correctly how this branch works, we still have two sets of intervals that clash with one another - the frame painting and the sound scheduling. I understand that the web audio api plays sounds in its own thread (it's the same as we do it now) - but what the web audio api would allow us to do is to "schedule sounds at a later time" without having a "thread" to actually start the sound? You know what I mean? Basically the web audio API would allow us to do this: http://www.html5rocks.com/en/tutorials/audio/scheduling/ . I'm speculating here but as long as we have two sets of intervals getting in the way of each other (cause the frame painting can be rather unpredictable in its length), we might have hiccups (because, as now, it's not like a thread has to "wait on a sound" do be played, that's not how I understand it works now, the hiccups come from the frame painting interfering with the scheduling of sounds (rather than the playing), which it would seem to me we'd still have now). Better to try than to speculate, but food for thought anyways.

davidedc commented 9 years ago

Perhaps the best way to describe the difference of the current system/this branch vs. what web audio api could potentially do: suppose we have a tranceKick every 1 ms (absurd, but, let's suppose).

rumblesan commented 9 years ago

So I'm going to start having a look at this again as I'd really like to get it merged. I'm doing some work to move over to using browserify to build LCL, as it's much easier and simpler than using require I've found, but I'm having issues with some of the sound libraries using globals, so simplifying this might make the task easier. Were there any issues with this branch that still needed fixing?

rumblesan commented 9 years ago

just rebased this onto master and it seems to be working pretty solidly

rumblesan commented 9 years ago

Also just tried it in firefox and it's fine as well. This is on OSX though, not tried windows.

davidedc commented 9 years ago

So here were my issues with this pull back then:

That, and the fact that sound support has been traditionally very fiddly to implement and test.

So that's why this pull has been hanging...

If this came in as an addendum (rather than a replacement) to the already existing library solutions, and it did make use of the webaudio "start playing the sample while the main thread is doing other things", then this would be just a win-win to merge.

It's the idea of regressing on IE support and having to test the fiddly part and yet to still have the contention problem over the main thread (between graphics and sample playback)... it made this pull request "easy to postpone" for me...

rumblesan commented 9 years ago

so IE support should be fine now. I added buzz back in and this will fall back to that if the web audio API isn't available. I'll give it more of a poke tomorrow when I can more properly assess the ins and outs. I actually have access to a windows machine I realise, so I can test on that haha. will let you know, but if it all seems fine then I'd like to get it merged. the sound system will be a whole lot simpler

rumblesan commented 9 years ago

ok, this has been rebased onto master again after the cleanup PR merge.

I've now fixed the issue with reset not actually clearing the screen. Also made sure it works when the previous program was using paintOver animation mode

rumblesan commented 9 years ago

turns out the old screen clearing code I removed is actually pretty important. removed that commit

rumblesan commented 9 years ago

tested this on chrome, firefox and safari on OSX, and also on chrome and firefox on windows. All working fine.

IE11 sound is still broken, but I think that's going to be a bigger investigation and I'm tempted to say that we should merge this and then start investigating that. thoughts?

davidedc commented 9 years ago

let me give it some hell. Is there a way we can test the midi input?

davidedc commented 9 years ago

works for me too.

One thing though, we have a "grunt releasebuild --language=vX" build option in order to only include one language or the other. The build system would then automatically eliminate the "dead code" caused by the flag, hence including the minimum code needed for only one language or the other.

I liked that option as I could build and push publicly the build for v1 only (for the moment)... can it be added back again?

davidedc commented 9 years ago

(I didn't test the midi input by the way)

rumblesan commented 9 years ago

So the language build option I don't actually think is so much use. I'd rather just always have both languages available until we can retire v1 Guy On 21 Jun 2015 18:49, "Davide Della Casa" notifications@github.com wrote:

(I didn't test the midi input by the way)

— Reply to this email directly or view it on GitHub https://github.com/davidedc/livecodelab/pull/241#issuecomment-113935333.

davidedc commented 9 years ago

I don't think we should expose users to the selection of two similar languages though... that means that if I merge this we can't publish master until we retire v1? I don't think we can know for sure when that is?

davidedc commented 9 years ago

ah realised buildTimeOptions are already removed in master, gonna merge this then. Will worry about pre-baking only one language when we get close to a release.

rumblesan commented 9 years ago

so I have some plans around improving the build system again. specifically, moving away from grunt and requirejs to browserify. It will be much simpler once I'm done, and we can have a much improved buildTimeOptions style system.