From: Stefan F. <st...@sf...> - 2010-06-06 11:25:15
|
Hi out there... I think that I have found parts of the source of click-sounds in the new FxMixer... void FxChannel::doProcessing( sampleFrame * _buf ) { ... ... ... if( ! m_muteModel.value() ) { // do mixer sends. loop through the channels that send to this one for( int i = 0; i < m_receives.size(); ++i) { fx_ch_t senderIndex = m_receives[i]; FxChannel * sender = fxm->effectChannel(senderIndex); // mix it with this one float amt = fxm->channelSendModel(senderIndex, m_channelIndex)->value(); // CPU::bufMixCoeff( _buf, sender->m_buffer, // sender->m_volumeModel.value() * amt, fpp ); CPU::bufMixCoeff( _buf, sender->m_buffer, 0.75, fpp ); // verify (on the left-channel) that the Clicks do originate from wrong values coming out of // amt and m_volumeModel.value()... if(0) for( int n=0; n<fpp; n++ ) { if( i== 0 ) _buf[n][0] = amt*0.125;//sender->m_volumeModel.value()*0.125; else _buf[n][0] += amt*0.125;//sender->m_volumeModel.value()*0.125; } } ... ... } There is still a lot I do not understand in the inner workings of that mixer (this mainly is due to the fact, that it makes heavy re-usage of m_buffer for all things here and there... Especially I do not understand why every volume-change and even automation-Stuff is still working with amt and with m_volumeModel.value() being removed at that stage of the mixer. And why are there two values for just one parameter being set? ... So, could anyone please enlighten me, how this Mixer is supposed to work internally? I bet I can puzzel this out by myself, but this might take a lot of time,... so this is just the kind request that someone gives me a hint or so, what it is doing when (at least what should it do when...)... cu Stefan PS: weird remark thingy: this seems to remove the clicks from the file-render, but not from the realtime-rendering... hmm,... strange... PPS: I have verified that the clicks (at least for the file-renderer is coming from grossly wrong volume-values, so one frame here and there gets completely too much amplification --> "*Click*"). |
From: Achim S. <lmm...@ma...> - 2010-06-06 13:25:37
|
Hi Stefan, > I think that I have found parts of the source of click-sounds in the new > FxMixer... Great to hear that. Can't wait to have a version of LMMS from git master that's actually producing music, not noise. bye, Settel |
From: Stefan F. <st...@sf...> - 2010-06-06 14:52:30
|
Am 06.06.2010 13:24, schrieb Stefan Fendt: Slight correction/update: it now seems to me, that not the amt-values/volume-values are wrong, but simply doProcessing() is running twice ore more times on the same fragment, but only sometimes... So the send-buffer of a source sometimes just get's added multiple times (which behaves like having a wrong amplitude-value)... hmm,... cu Stefan |
From: Stefan F. <st...@sf...> - 2010-06-07 14:48:49
|
Hi again, after reading the source all and all over again (and still without understanding every single aspect of it, so my wild guesses may be all completely wrong/dump), I start to think that this might be a logic bug. >From what I can tell, now, the mixer should recursively iterate through all the channels, from parents to childs to the leaves until it finds channels which only do depend on leaves (audio-tracks). These can be rendered to the buffer as all dependencies are known for these channels. Then it should go back to the parents of these channels and render them if all dependencies are met, too. ... so far so good. But this (from my current analysis -- as stated above it may be all wrong at all), is not what is happening with the current code. The current code does not go up to the leaves but only to the input channels of the current channel (which is not the same) and renders these. By doing so, it is -- from my point of view -- impossible to say that all source-channels of the current FX-Channel are really ready to be processed... void FxChannel::doProcessing( sampleFrame * _buf ) { FxMixer * fxm = engine::fxMixer(); const fpp_t fpp = engine::getMixer()->framesPerPeriod(); // <tobydox> ignore the passed _buf // <tobydox> always use m_buffer // <tobydox> this is just an auxilliary buffer if doProcessing() // needs one for processing while running // <tobydox> particularly important for playHandles, so Instruments // can operate on this buffer the whole time // <tobydox> this improves cache hit rate _buf = m_buffer; if( ! m_muteModel.value() ) { // do mixer sends. loop through the channels that send to this one for( int i = 0; i < m_receives.size(); ++i) { fx_ch_t senderIndex = m_receives[i]; FxChannel * sender = fxm->effectChannel(senderIndex); // mix it with this one float amt = fxm->channelSendModel(senderIndex, m_channelIndex)->value(); CPU::bufMixCoeff( _buf, sender->m_buffer, sender->m_volumeModel.value() * amt, fpp ); } Ok, we have gone through all the other FX-channels which send to us... but how do we know that really all of the input-channels are ready at that point? As far as I can tell, we can't... In most of the cases using simple mixer-setups it can resolve all of the inputs /but/ due to the general out-of-order nature of the underlying threads, it is (from my point of view) uncertain that this is allways the case. ... And this would correspond to the fact that these "clicks" do not happen always. I suppose (wildly guessing) they just happen if this is executed out of order... const float v = m_volumeModel.value(); m_fxChain.startRunning(); m_stillRunning = m_fxChain.processAudioBuffer( _buf, fpp ); m_peakLeft = engine::getMixer()->peakValueLeft( _buf, fpp ) * v; m_peakRight = engine::getMixer()->peakValueRight( _buf, fpp ) * v; } else { m_peakLeft = m_peakRight = 0.0f; } m_state = ThreadableJob::Done; // check if any of its parents are now able to be processed for(int i=0; i<m_sends.size(); ++i) { // if parent.unstarted and every parent.leaf.done: FxChannel * parent = fxm->effectChannel(m_sends[i]); if( parent->state() == ThreadableJob::Unstarted ) { bool everyLeafDone = true; for( int j=0; j<parent->m_receives.size(); ++j ) { if( fxm->effectChannel( parent->m_receives[j] )->state() != ThreadableJob::Done ) { everyLeafDone = false; break; } } This little change here does stabelize things a lot here on my machine... I am however not completely sure, why... ;-) My guess: it prevents a parent from being run too early... However the clicks are not completely gone (and can't by this modification as it does not change anything on the dependecies-tracking inside the channel-tree...) but a lot less often. if( everyLeafDone && i==(m_sends.size()-1) ) { MixerWorkerThread::addJob(parent); } } } } >From my point of view -- as stated above to be taken with some salt and grain -- the mixer follows an intermixed push-pull model (maybe accidently), currently while it clearly should follow a straigth pull-model... cu Stefan |
From: Stefan F. <st...@sf...> - 2010-06-07 15:51:34
|
Some clarification (hopefully) on my previous mail: The parent should trigger the child and not vice versa (as it seems to be in current state). Why? The parent is the only one knowing what input-sources it needs and when it needs them, so in some slightly oversimplified pseudo-code this would be something like this: doProcess() { for( int n=0; n<senders.size(); ++n ) { if( !sender[n].isReady() ) { // recursively go through all childs, calculating their result sender[n].doProcess(); } // OK, this child is done, so mix it into our own output mix( _buf, sender[n].buf, amt ); } // If the track-output is a sender, too, this is all needed to be done. // If the track-output is not handled as a source we need to handle // all track-outputs directly sending to us separately here... // //for( int n=0; n<sources.size(); ++n ) //{ // mix( _buf, source[n].buf, amt ); //} } Any comments? cu Stefan |
From: Tobias D. <tob...@gm...> - 2010-06-07 22:47:47
|
Hi, Am Montag, 7. Juni 2010, um 17:50:48 schrieb Stefan Fendt: > The parent should trigger the child and not vice versa (as it seems to > be in current state). Why? The parent is the only one knowing what > input-sources it needs and when it needs them, so in some slightly > oversimplified pseudo-code this would be something like this: This code indeed looks more clear and logical to me. I'd like a solution based on this code. Especially this could be advanced for avoiding processing unused FX sends (i.e. such ones that never get routed to master). Andrew (who developed the FX send support), any comments on this? Toby |
From: Andrew R K. <and...@gm...> - 2010-06-09 14:52:22
|
Sorry, I don't have time to look at this right now. I know it's evil to introduce a bug and not fix it, I will try to get to it when I can. On Mon, Jun 7, 2010 at 3:47 PM, Tobias Doerffel <tob...@gm...>wrote: > Hi, > > Am Montag, 7. Juni 2010, um 17:50:48 schrieb Stefan Fendt: > > The parent should trigger the child and not vice versa (as it seems to > > be in current state). Why? The parent is the only one knowing what > > input-sources it needs and when it needs them, so in some slightly > > oversimplified pseudo-code this would be something like this: > This code indeed looks more clear and logical to me. I'd like a solution > based > on this code. Especially this could be advanced for avoiding processing > unused > FX sends (i.e. such ones that never get routed to master). > > Andrew (who developed the FX send support), any comments on this? > > Toby > > > ------------------------------------------------------------------------------ > ThinkGeek and WIRED's GeekDad team up for the Ultimate > GeekDad Father's Day Giveaway. ONE MASSIVE PRIZE to the > lucky parental unit. See the prize list and enter to win: > http://p.sf.net/sfu/thinkgeek-promo > _______________________________________________ > LMMS-devel mailing list > LMM...@li... > https://lists.sourceforge.net/lists/listinfo/lmms-devel > > -- Andy Kelley http://superjoesoftware.com/ |
From: Stefan F. <st...@sf...> - 2010-06-20 11:56:38
|
Hi, At least "partial" success... I have a modified mixer-code, now which works without clicks against the current git master... it still need some polishing(*)... but if nothing stops me (har... har... har... family? and/or the strange segfault... see below... ;-)) I will post the changes to the list this evening (which would allow me to work on the filters again)... cu Stefan [*] "partially" is meant like this: the sends _do_ work, the clicks _are_ gone and the code doesn't spawn a gazillion of unneeded threads (wasting CPU for just spawning and waiting for tiny threads) inside the mixer any more... but it seems I can't get the send-amount-value without producing a segfault... (so it's currently set to 1.0 ... *erm* ...) ... Can just be a few more day's until I find out why ... :D PS: due to the odd mixture of push and pull models inside the core I can not guarantee that I have not broken any other things... Maybe it would be a clever idea of a) testing the patch as hard as possible and b) (at some time) clean rewriting the core... (Albeit I must admit, that I don't have the time to do that...) |
From: Stefan F. <st...@sf...> - 2010-06-20 16:17:52
Attachments:
lmms-changed-mixer-20100620-SMF.diff
|
Ok, here we go... This is a (not cleaned up, yet!) patch, so you can change the mixer like I did... I hope it doesn't break anything (so please carefully test it before applying it to the git master...). I esp. did not wipe the old code but only #if-ed it out... cu (and have fun!) Stefan PS: If it crystalizes out that the patch will be accepted, I will be very happy to submit a cleaner version... (or maybe getting write-access to the repo? ... Well, due to strong restrictions in my spare-time you should not expect to see too much noise from me anyways... ;-))... |
From: Achim S. <lmm...@ma...> - 2010-07-05 22:49:02
|
Hi Stefan, Your patch is a huge step forward, thanks for posting it :-) I gave it a shot just a few minutes ago, here's my experience: * patch applied without problems * no clicks experienced during 10 minutes of testing * however, every like 30 bars, sound output is paused for ~1/4 second * when the above mentioned pause happens, the osci in the top and the VU bars in the mixer are paused, too. * CPU usage is normal (2 cores both at 40%) during playback and has a very very tiny peak on one CPU during the above mentioned pause. As a summary: With the patch, I can at least do some work again but the mixer code still needs work :-( My recommendation is to roll back to git master's state back in september 2009, before the FX mixer rewrite was applied :-( bye, Settel System info: * Ubuntu Linux on 32 bit x86 (Dual Core) * LMMS from latest git master with Stefans patch applied * Output driver ALSA with 1024 frames buffer |
From: Stefan F. <st...@sf...> - 2010-06-20 18:01:13
|
OK, ... the segfault with the send-value is still there... it sometimes works and sometimes crashes badly... :( Stefan |
From: Stefan F. <st...@sf...> - 2010-06-20 18:55:47
Attachments:
lmms-changed-mixer-20100620-SMF-2nd-try.diff
|
Am 20.06.2010 20:00, schrieb Stefan Fendt: > OK, ... the segfault with the send-value is still there... it sometimes > works and sometimes crashes badly... :( > OK, this one works without the "mysterious" segfault... (see comments or make a diff of the diff). I hope it works not only for me... ;-), so have fun with it... Stefan |
From: Tobias D. <tob...@gm...> - 2010-08-16 21:35:02
Attachments:
lmms-mixer-fixes-20100816-toby.diff
|
Hi, Am Sonntag, 20. Juni 2010, um 20:55:03 schrieb Stefan Fendt: > OK, this one works without the "mysterious" segfault... (see comments or > make a diff of the diff). > > I hope it works not only for me... ;-), so have fun with it... Thanks for this patch! I reviewed it and I liked the simplicity. However there's one problem: we can't afford giving up multithreading, especially not for FX processing as this is taking most of the CPU time. I therefore merged the two attempts, the original one and yours and made a new algorithm. It is fully multithreading-capable (building a job list via post-order-processing of the dependency graph) and has a "pull"-architecture (and additionally uses the accellerated CPU-functions for mixing buffers (however this is arch-indep.)). Please have a look at the attached patch. Still might be buggy but basically works and fixes the horrible clicks. BTW: you're right that it is silly to have a push-architecture on the one side (instrument/sample track processing etc.) and a pull-like arch. on the other side (FX mixer) - the problem of a long evolutionary development. You expressed some concerns that the audio tracks push their data into fx channels before those are processed. That's not a problem as rendering currently happens in different stages (+worker thread synchronisation after each stage) It's definitely worth thinking about how to make audio tracks follow the pull- architecture as well so that in the end, FX channels pull their data from them. This allows to merge two rendering stages, i.e. remove a worker thread synchronisation point and thus improving performance (by processing just one large dependency graph with all kinds of jobs inside). However, we first should get the FX mixer working again properly. Best regards Toby |
From: Stefan F. <st...@sf...> - 2010-08-30 13:03:02
|
Am 16.08.2010 23:32, schrieb Tobias Doerffel: I like the archive of the list... ;-) ... Still, I don't know why this didn't make it through to me... ;-) > Thanks for this patch! I reviewed it and I liked the simplicity. However ... > Please have a look at the attached patch. Still might be buggy but basically > works and fixes the horrible clicks. > Seems to work fine, here... all the best, Stefan |
From: Paul G. <dr...@gm...> - 2010-06-29 17:06:34
|
Haven't looked at this yet, but looking forward to it. I'll let you know what happens. |
From: Stefan F. <st...@sf...> - 2010-08-28 10:04:46
|
Hi out there... I have running my mixer-patch now for a rather long while without problems... it makes me think, that it works rather properly... Was there a specific reason it was not applied so far — so I maybe can fix the issue which lead to it being not applied? Stefan |
From: Tobias D. <tob...@gm...> - 2010-08-29 19:55:17
|
Hi Stefan, Am Samstag, 28. August 2010, um 11:37:33 schrieb Stefan Fendt: > I have running my mixer-patch now for a rather long while without > problems... it makes me think, that it works rather properly... > > Was there a specific reason it was not applied so far — so I maybe can > fix the issue which lead to it being not applied? Yes, the reason I pointed out in my mail from 2010-08-16: it does not utilize multithreading. Did you try my patch? It's based on your work (thanks!) but has further improvements regarding MT. I'd like to apply it soon - maybe I should simply do so and wait for reactions :-) Toby |
From: Stefan F. <st...@sf...> - 2010-08-30 10:54:26
|
Hi Tobias, > Yes, the reason I pointed out in my mail from 2010-08-16: it does not utilize > multithreading. Did you try my patch? It's based on your work (thanks!) but > has further improvements regarding MT. I'd like to apply it soon - maybe I > should simply do so and wait for reactions :-) > hmm,... weird... doesn't seem like I ever got that mail... :( OK then, what about simply apply that patch and wasit for reactions? ;-) The only reason I didn't do this mthreaded was that it didn't show to be slower without... So, from my point of view: just apply it (including your mt-patch) and we'll see... ;-) all the best, Stefan |
From: Tobias D. <tob...@gm...> - 2010-08-30 15:41:10
|
Hi, Am Montag, 30. August 2010, um 12:53:40 schrieb Stefan Fendt: > OK then, what about simply apply that patch and wasit for reactions? ;-) Done. > The only reason I didn't do this mthreaded was that it didn't show to be > slower without... Did you try under full load, i.e. multiple channels loaded with various CPU intensive LADSPA plugins? Toby |
From: Stefan F. <st...@sf...> - 2010-08-30 17:17:25
|
Am 30.08.2010 17:40, schrieb Tobias Doerffel: Hi Toby, > Done. ;-) fine... so, let's pull again,... ;-) >> The only reason I didn't do this mthreaded was that it didn't show to be >> slower without... >> > Did you try under full load, i.e. multiple channels loaded with various CPU > intensive LADSPA plugins? > Yes,... the first file I encountered this behaviour with was using ~80% load overall on my quadcore-machine, so in the first place I thought of an ladspa-bug or so (quite complicated setup with lot's of FX but only 8 Channels in use -> e-guitar-simulation using vibes). With the non-mt-mixer it still remained being there cpu-load-wise (in fact I had the feeling — but didn't explicitly test for it — it might even be a little faster than before)... Stefan |