borfo Posted January 10, 2015 Author Report Share Posted January 10, 2015 (edited) TK - in order to implement NoFX, +Echo and +Duplicate, I need to pass variables into core.c (and probably live.c as well, but I haven't implemented anything there yet). Currently, SEQ_ROBOTIZE_Event() returns a binary coded set of flags - that works for NoFX, but the variable needs scope outside of SEQ_CORE_Tick in order to implement +Echo and +Duplicate (those functions would activate the Echo and Duplicate functions for the step even if FX Echo is set to off). How would you recommend passing variables from SEQ_ROBOTIZE_Event() into functions in seq_core.c and seq_live.c from SEQ_ROBOTIZE_Event() (eg: functions like SEQ_CORE_Echo())? I thought about putting the flags in the track CCs, but there's probably a better way. Also, any hints on how to implement a +sustain robotizer? I got it to cause a note to sustain, but it never turned off, and no more notes were played. Sustain, +Echo and +duplicate are fully implemented on the Robotizer side, that code is commented out of the UI though. SEQ_ROBOTIZE_Event() returns the flags, and the flags are parsed in SEQ_CORE_Tick() into u8 robotizer_sustain, u8 robotizer_nofx, u8 robotizer_echo, and u8 robotizer_duplicate. nofx is the only flag that core.c currently does anything with though, because of the scope thing. ###### Also, nerdy copyright lawyer note re: the copyright notice - Thanks for putting my name in the code, that's pretty cool actually... But why don't you just list me as a contributor in the header of the robotizer files, and list yourself as the copyright owner. I will (and hereby do) assign the copyright in my contributions to this code to you, that way you retain clean ownership of the whole MIDIbox source if this code makes it into the official release. Edited January 10, 2015 by borfo Quote Link to comment Share on other sites More sharing options...
TK. Posted January 11, 2015 Report Share Posted January 11, 2015 Cool, thanks for the changes! :smile: First of all: I gave you access to the (hidden) Programmers Lounge. Here are the informations about SVN write access to the repository: It makes sense that you upload your changes directly to the repository - I can always switch back to an earlier version if I'm not happy with the changes. ;-) I also added a new type in seq_robotize.h which simplifies the access to robotizer flags: typedef union { u8 ALL; struct { u8 SUSTAIN:1; u8 NOFX:1; u8 ECHO:1; u8 DUPLICATE:1; }; } seq_robotize_flags_t; Sustain: search for tcc->mode.SUSTAIN, just OR this flag with the robotizer flag: (tcc->mode.SUSTAIN || robotizer_flags.SUSTAIN) Don't put temporary flags into the CC array, it's better to keep them in the local stack like it's currently done. From time to time it makes sense to check the performance impact of your code. Go to the INFO page (Main menu, GP#16), the Stopwatch will display the consumed time for each SEQ_CORE_Tick() The maximum value will be captured and can be cleared by pressing GP9 button in this page. Currently the function consumes +30 uS on a STM32F1 if a single track is playing. It could be improved. E.g. I noticed that you reverted an important change that I made: you are writing for example: if( ( (s16)SEQ_RANDOM_Gen_Range(0, 31 * 31) <= tcc->robotize_oct_probability * roboprob ) && robooct ) { // Octave Event but it's better to write: if( robooct && ( (s16)SEQ_RANDOM_Gen_Range(0, 31 * 31) <= tcc->robotize_oct_probability * roboprob ) ) { // Octave Event so that SEQ_RANDOM_Gen_Range() won't be called if robooct isn't activated Same for robonote, robovel, robolen The copy of tcc-> variables into local variables isn't really necessary: roboprob = tcc->robotize_probability; robovel = tcc->robotize_vel; robolen = tcc->robotize_len; robonote = tcc->robotize_note; robooct = tcc->robotize_oct; roboskip = tcc->robotize_skip_probability; robosustain = tcc->robotize_sustain_probability; robonofx = tcc->robotize_nofx_probability; roboecho = tcc->robotize_echo_probability; roboduplicate = tcc->robotize_duplicate_probability; because the impact of indirect addressing isn't so high compared to other things (such as function calls) I would remove this - it makes the code more difficult to maintain. Best Regards, Thorsten. Quote Link to comment Share on other sites More sharing options...
borfo Posted January 11, 2015 Author Report Share Posted January 11, 2015 (edited) Cool. Thanks. Is there a good reference anywhere that you know of on how to structure code to minimize consumption of RAM and CPU cycles? I've never programmed for anything where I really had to be concerned about that sort of thing before. Your comment about not calling the random function until absolutely necessary makes sense, as does not copying existing variables into local variables. I'll fix that stuff up. Should I be concerned about straightforward math? ...like, should I try not to multiply/divide/add/concatenate/etc. things unless absolutely necessary, or is the impact of straightforward evaluation like that relatively minimal? How about structures like if or switch statements? Any other stuff I should try to avoid? Edited January 11, 2015 by borfo Quote Link to comment Share on other sites More sharing options...
TK. Posted January 11, 2015 Report Share Posted January 11, 2015 I don't know a good reference for such topics, I just observe and decide, and if I'm unsure, I measure ;-) After a successful build you will find an assembler listing under project_build/project.lss which gives some useful information, e.g. how many assembly instructions are generated by a C construct. Makes sense to check this file from time to time... Should I be concerned about straightforward math? ...like, should I try not to multiply/divide/add/concatenate/etc. things unless absolutely necessary, or is the impact of straightforward evaluation like that relatively minimal? Cortex-M has special instructions for these math operations, therefore no need to be worried about this. How about structures like if or switch statements? Depends on the implementation and available alternative methods. E.g. sooner or later I will optimize the large switch statements in seq_cc.c by a lookup table. I did something similar in MBCV V2 (-> http://svnmios.midibox.org/filedetails.php?repname=svn.mios32&path=%2Ftrunk%2Fapps%2Fprocessing%2Fmidibox_cv_v2%2Fsrc%2Fcomponents%2FMbCv.cpp ) with great success. But it's not worth the effort for switch statements below 30..40 items, and I don't think that you've to consider this in your code domain... If I see something "which hurts", I will optimize it sooner or later and show you the improved version. Best Regards, Thorsten. Quote Link to comment Share on other sites More sharing options...
borfo Posted January 12, 2015 Author Report Share Posted January 12, 2015 (edited) OK, committed the efficiency changes in robotize.c -- thanks again for the svn access, hope I didn't destroy anything with my first commit. It looks like the function adds about +20 uS now on an LPC core, if I'm reading the stopwatch correctly. Is that reasonable processor time for a function like this? Would it be more efficient if I pulled a long (say 64 bit) random number only once, and then used bit sequences within that long number as my random numbers later in the robotizer function? I'm thinking I could pull quite a few pseudorandom numbers out of 64 bits with some simple bitmasking, which would allow me to do a lot more complex robotizing with only one call to the random function. Would that be more efficient than the current implementation, or would pulling a 64 bit number once and manipulating it be more of a drain? Edited January 12, 2015 by borfo Quote Link to comment Share on other sites More sharing options...
lukas412 Posted January 12, 2015 Report Share Posted January 12, 2015 Wow this is very cool guys. Thanks for your work! Quote Link to comment Share on other sites More sharing options...
jojjelito Posted January 12, 2015 Report Share Posted January 12, 2015 Definitely cool! Gotta try this out :) Thanks for coding. Quote Link to comment Share on other sites More sharing options...
TK. Posted January 13, 2015 Report Share Posted January 13, 2015 Is that reasonable processor time for a function like this? There is always room for improvements ;-) E.g. you are writing: //exit if there's no chance of robotizing anything if( ! tcc->robotize_active || tcc->robotize_probability == 0 || ( !tcc->robotize_vel && !tcc->robotize_len && !tcc->robotize_note && !tcc->robotize_oct && !tcc->robotize_skip_probability && !tcc->robotize_sustain_probability && !tcc->robotize_nofx_probability && !tcc->robotize_echo_probability && !tcc->robotize_duplicate_probability ) ) return returnflags; // nothing to do This kind of check isn't required, because with the tcc->robotize_active switch you've already ensured that the function doesn't consume time if it isn't activated. It will only be enabled if the user intends to use the function, and then it's unlikely that he doesn't set one of the parameters. Please reduce this check to "if( !tcc->robotize_active || !tcc->robotize_probability )", this should be sufficient. Would it be more efficient if I pulled a long (say 64 bit) random number only once, and then used bit sequences within that long number as my random numbers later in the robotizer function? Yes, it makes sense to reduce the number of SEQ_RANDOM_Gen() calls It will return a 32bit number Since you need a range from 0..1023 (=10 bit), you could extract 3 random numbers out of the 32bit value It doesn't make sense to work with 64bit numbers here... Best Regards, Thorsten. Quote Link to comment Share on other sites More sharing options...
borfo Posted January 13, 2015 Author Report Share Posted January 13, 2015 (edited) What sort of stopwatch +uS number should I be aiming for on the LPC core? ...also, with the long random number thing: I'm thinking that if I take bits 1-10, 2-11, 3-12, etc., I could take a lot more than 3 pseudorandom sequences out of a 32 bit number... I think they'd all be more or less random enough, especially if I used them in different parts of the robotizer. I'll do some experimenting and see how that affects the stopwatch value. What should I pass as the seed to SEQ_RANDOM_Gen? Edited January 13, 2015 by borfo Quote Link to comment Share on other sites More sharing options...
TK. Posted January 13, 2015 Report Share Posted January 13, 2015 What sort of stopwatch +uS number should I be aiming for on the LPC core? Than lower than better. ;-) But don't spent too much time on this optimization step, it can be counter-productive. ...also, with the long random number thing: I'm thinking that if I take bits 1-10, 2-11, 3-12, etc., I could take a lot more than 3 pseudorandom sequences out of a 32 bit number... I think they'd all be more or less random enough, especially if I used them in different parts of the robotizer. No good idea, take separate parts to avoid any dependency. What should I pass as the seed to SEQ_RANDOM_Gen? 0 (so that the previous seed won't be overwritten) Best Regards, Thorsten. Quote Link to comment Share on other sites More sharing options...
borfo Posted January 16, 2015 Author Report Share Posted January 16, 2015 (edited) I committed a few changes to seq_robotize.c - eliminated the unnecessary checks you pointed out, fewer random generator calls now, and the probability checking is now in its own function which bitmasks out 10 bit parts of 32 bit random numbers, and only gets new random numbers if and when required. It's a bit more efficient, seems to have shaved off about 5 - 7 uS. Thanks for all your help with this, by the way. A few more questions: Is there a preferred method for determining the number of the track that's currently being worked with from within, for example, SEQ_CORE_ScheduleEvent()? Can the current track number be determined from a seq_core_trk_t or seq_cc_trk_t object? ...or should I just pass the track number to SEQ_CORE_ScheduleEvent() as an argument if it's needed? Are there Doxygen documents specifically for the SEQ like there are for the NG? (http://ucapps.de/midibox_ng/doxygen/index.html) Is there a search interface for those NG docs? ....and a comment/question re: reusing bits of the same 32 bit number: I said "take bits 1-10, 2-11, 3-12, etc.,", which was a bit dumb... Wasn't thinking when I wrote that. Moving over by only one bit would obviously create a significant dependency... But since my randoms are 10 bit numbers, wouldn't shifting over further, say by 5 bits (taking bits 1-10, 6-15, 11-20, etc.) create enough randomness for this purpose, without too much dependency? I could get five 10bit pseudorandom numbers out of one 32 bit number that way... EDIT: Added the sustain robotizer. It adds 7-10 uS, but that seems to all be from the existing SUSTAIN implementation, not from the fact that it's being robotized. Edit Edit: Added the +Echo and the +Duplicate robotizers... These robotizers use the settings on the Echo and Duplicate FX pages, ignoring the enabled/disabled state of those FX. In other words: adjust the +Echo setting on the FX robotizer page to occasionally apply echo to notes, using the echo settings currently selected on the FX echo page. Change the settings on the Echo page to change the robotized echo. With all the robotizers enabled, the CPU usage goes up significantly, but I think most of that is coming from the other FX implementations, not too much from the robotizer. The robotizer seems to add +18 uS or less compared to just having Echo, sustain, and duplicate on. Optimizing suggestions are more than welcome, of course. LPC-project.hexSTM-project.hex Edited January 17, 2015 by borfo Quote Link to comment Share on other sites More sharing options...
TK. Posted January 17, 2015 Report Share Posted January 17, 2015 Is there a preferred method for determining the number of the track that's currently being worked with from within, for example, SEQ_CORE_ScheduleEvent()? Can the current track number be determined from a seq_core_trk_t or seq_cc_trk_t object? ...or should I just pass the track number to SEQ_CORE_ScheduleEvent() as an argument if it's needed? The right way would be to pass the track number to this function as an argument (API has to be changed) Are there Doxygen documents specifically for the SEQ like there are for the NG? You could create one with Doxygen Here the config file for MBNG: http://svnmios.midibox.org/listing.php?repname=svn.mios32&path=%2Ftrunk%2Fapps%2Fcontrollers%2Fmidibox_ng_v1%2Fdoc%2F The robotizer seems to add +18 uS or less compared to just having Echo, sustain, and duplicate on. well done! :) Best Regards, Thorsten. Quote Link to comment Share on other sites More sharing options...
electrodancer Posted January 25, 2015 Report Share Posted January 25, 2015 this is great...thank you very much! Quote Link to comment Share on other sites More sharing options...
jerd Posted September 5, 2015 Report Share Posted September 5, 2015 Thanks for this fx, very nice. I have one question, should the robotizer values be saved when the session is saved? I cant seem to save the parameters with the session. Quote Link to comment Share on other sites More sharing options...
TK. Posted September 5, 2015 Report Share Posted September 5, 2015 (edited) Unfortunately we can't save the settings due to memory limitations. With MBSEQ V4+ (running on a STM32F4) this should be possible, but it will still take some months until the extended firmware version will be available (and it won't run on a STM32F1 or LPC17 based core...)Best Regards, Thorsten. Edited September 5, 2015 by TK. Quote Link to comment Share on other sites More sharing options...
k-rAd MB6401 Posted October 31, 2015 Report Share Posted October 31, 2015 Can't wait to implement this robotizer function! How do i get my hands on this? Very cool guys. Quote Link to comment Share on other sites More sharing options...
TK. Posted November 1, 2015 Report Share Posted November 1, 2015 Enter MENU->FX->Robo (GP button #3 in FX page)Best Regards, Thorsten. Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.