yoyz / picoloop

96 stars 9 forks source link

[update] New observation about MDA Drum on rpi 3 #27

Open lysergik opened 2 years ago

lysergik commented 2 years ago

Hi,

Since I've seen some new commit on MDA drum code I wondered if you made some consequent change to it and if it would help it running on the pi 3. Also the recent 64-bit release of a raspberry pi OS gave me hope.

So I downloaded the last sources of picoloop and compiled it on my pi 3 and launched it, I tried to get mda drum a first time and it crashed as it did before, but after tweaking a bit other synth, including their waveshape, I retried using mdadrum and then it worked. I tried to switch to other MDA presets and it worked until for an unkown reason a preset made picoloop crash. I tried again this approach and saw that MDA drum can render preset on the pi but not all. I didn't had time to see which one worked and which don't, and if their runnability on the pi 3 is something constant or not however either you already patch something up or I did not noticed some of the preset working before. Did you noticed that some of them could run ?

If it is the case this topic is useless, if not we should look at this strange behaviour to see where the bug come from since I don't see why some of them could be rendered and not other. Is there some borderline values in some .dsi patch leading picoloop to have a huge load on the cpu, are there some .dsi patch that uses some parameters that other don't uses at all ? If you have any clue on what is going on I would be glad to hear it.

EDIT: Since the folder didicated to MDA Drum in the picoloop hasn't been modified but it's only the one outside of the one on synthengine folder) I guess that your commits doesn't had any effect on this behaviour.

EDIT 2: I think I found what is causing this bug, or at least it's a something that will reveal why it is bugging this way. I took a bit of my to try to see which presets goes nuts on which don't. I found that Hat_c.dsi and both clap preset of the TR-909 are crashing picoloop, the simple kick of the 808, the h_open and syntom_1 of the Electro set and pretty much all the insstrument of CR78 sets are crashing picoloop. So far I've noticed that all those share a similar construction they lacks parameters in the section [general] they also lacks parameters in the section [overtones] and also lacks the entire field [noiseband2] and [distorition]. Because the load_patch version withing drumsynth_oop5.cpp is looking for string that are not there in the .ds file I guess it complicates lthe life of the program. If so far it seem to me to be the better explanation I don't get why a regular computer can handle that wihtout any sign of struggle and why it crashes on the pi. However if I am somewhat right it would be a good news, since it could be fix easily, either by modifying the .ds patch that fit the same description, either by modifying the load-patch function.

I will try to fix it myself since it doesn't seem that big of a deal in the end, I will start by writing some bash script to identify all the files that fit the structuration of buggy patches. And then correct them by adding the missing parameters. If none of that fixes the bug I will then go more on the GDB/Valgrind side of things to ssee whats going on and then fixing the load-patch function.

EDIT 3: Definitevly the bug is coming bad formating of .ds file. With the help of bash scripts I could isolate a list of bad .ds file (telling from their lenght in lines) here it is:

Acoustic/Brush1.ds 35 Acoustic/Brush2.ds 35 Acoustic/JazzKick.ds 35 Acoustic/Kick.ds 35 Acoustic/K_Muffle.ds 35 cr78/Bongo_h.ds 36 cr78/Bongo_l.ds 35 cr78/Clave.ds 35 cr78/Conga.ds 35 cr78/Hihat.ds 35 cr78/Kick.ds 35 cr78/Maracas.ds 35 cr78/Rim.ds 35 cr78/Snare.ds 35 cr78/Tamb.ds 35 Effects/Bubble.ds 35 Effects/Glass.ds 34 Effects/Reverse.ds 35 Electro/H_open.ds 35 Electro/Syntom_1.ds 35 tr808/Kick.ds 34 tr909/Clap.ds 34 tr909/Hat_c.ds 35 tr909/TR909_Clap.ds 34 I managed to fix most of those file by copying part of another file and it doesn't seem to change the sound it produces and they doesn't make picoloop crash. From the list only one is still messing with the code, tr909/clap.ds. I will look at that and try to fix it but since there's already another 909 clap if it is too complicated I might just modify the instrument list to get rid of it. I tested most of the rest of the preset and everything seem fine now. I'll also try this new collection of .ds patch on older build of picoloop to make sure the bug was comming from there. Anyway I'm happy to be done with the bug I was strugling with it since 2018 !!!

When the final tweaking will be done I will submit a pull request.

yoyz commented 1 year ago

You know what ! It has been a real stuggle for me to understand why it crash. And you have done a fantastic analysis here :) It's been a while since I have not dig into this source code, but i'm really really happy on the work you have achieved here, and I guess your analysis is perfectly fine. I still don't understand the root cause, but will look at this in the future :)

MDADrum is really a fantastic really old plugin synthesis, and being able to make it run smoothly is really something I hope one day I will take time to solve it. I don't want to modify the preset file, because they are what they are, and it's not the "right way to solve thing" But if there is a good way to solve that in the source code itself then I'm fully happy.

Thanks a lot :)

lysergik commented 1 year ago

You're welcome ! As for the preset I wondering if they all been made under the same version of the software, those who tend to crash seems to have made under an older version of the MDADrum from my point of view it's more an issue of file formatting than something that is due to the code, since functionality must have been added along the way and so breaking the preset file compatibility between version of the software. However, modifying the original code in order to see if the preset file is either old or recent format, and applying the right data extraction method for them could do the trick. As for the root causes of the crash happening on the Pi and not on a regular computer is probably due to differences in CPU architectures. I'm clearly not an experts on CPU architecture but I won't be surprised that a x86 binary would be "more clever" in order to avoid hitting certain segmentation fault, thanks to some extra assembler logic for managing memory. On the other side I won't be surprised if ARM architecture embed less memory management logic and so is unable to avoid the segfault. I think it's the best solution so far, and the fact that "patched" malfunctioning dsi then works on the pi means the seg fault is coming from the function loading preset.

By the way if you have time one day to patch that into picoloop code I'll be glad to see your solution to the problem. Also, the synth is great sounding, however I tend to like having hands on control on the parameter of the synth I use. So if it would possible to had some parameter tweaking for MDADrum inside picoloop it would be great. I tried to implement it myself however my it was quite buggy so I gave up on that.

Since it been quite some time we've exchanged about picoloop I would also like to point things I noticed lateley. Last I've made some progress on my project forking picoloop, with I gain knowledge in C++ and programming audio, which helped me grasp how picoloop works and how the code is structured. This helped me to try to tweak a bit some synth or adding a live keyboard feature, recording notes played into the sequencer, allowing to load more samples on LGPT, panning tracks, adding 4 way polyphony for each tracks, muting tracks, and I began to work on implementing new FX in order to offer a wider palette of FX to choose from the only delay currently available. That being said I noticed two things that would be worth some tweaks for picoloop main branch.

First, when looking at Master.h I noticed it is VERY easy to upgrade the number of tracks picoloop is playing by modify a single value, I found that so helpful to offer more possibility to the user (even thought it messes up with the display of the load and save menu). This simple number of track upgrade doesn't come with any issue, all tracks works as intended as long as you have enough ram ta carry all the buffers loaded, but in fact I eventually found one bug. I could tweak the number of track all the way to 8 different without experiencing any issue, howether after that when picooop is loading 9 tracks you can then see the program froze if you try to use LGPT. Why ? I wonder for a long time but in fact there's a macro defining the number of track for LGPT, from what I understand, since LGPT is a tracker based things it's meant like picoloop to work with multiple channel in parallel, so one instance is initialize a from the entire program and then you can call it's function to generate sounds. However LGPT has a limited number tracks, 8 tracks total, even though your not using more than 8 tracks of LGPT across the picoloop tracks, it loading one instance of LGPT on one track of at least an altered version of picoloop running at least 9 tracks is somehow making crash because it clashes with hard coded limit of LGPT 8 tracks. For Picoloop tracks updaters enthusiast know that going to LGPT source code directory you'll find an header named "lgpt.h" where you can modify the value of the macro to set it to whatever size you'd like. The macro is named "#define SONG_CHANNEL_COUNT" and by default is set to 8. For you Yoyz, it's not so much a bug, as the code on github won't ever crash because of that, however I wonder linking Master.h to lgpt.h in order to link the value of "#define SONG_CHANNEL_COUNT" and "#define TRACK_MAX" would be a good thing or not for the main branch. It's been a long time since I discover that and i don't recall if modify the value of SONG_CHANNEL_COUNT has an effect on ram use or not.

The other things I noticed is also not a bug, but this times it is not something I found out changing the original functionality of the program. If you look closely at Csynth and Tsynth code you'll find out that both embed in their synth engine a support for polyphony. They both support like a dozen or more voice of polyphony(I think it goes to something like thirty for Tsynth). At first I was thinking this was not a issue, since you trigger only one note at a time it shouldn't that resources hungry ? Well, not at all, in fact even though your using only one voice of polyphony from the engine, both engine are initializing buffers for each polyphony voice, and filling them with zeros... and so taking a lot of RAM for basically no purpose. Since picoloop initializes 4 instance of Csynth and 4 of Tsynth, it's literately hundreds of buffers that are initializes (even if no csynth or tsynth is used at all by the user) taking up RAM even though they can't be possibly used. It's obviously a huge performance issue, even more as picoloop as been designed for running low resources platform, and for those builds those synth weren't included because too resources hungry, but it's not so much because they are doing fancy stuff than because they are embedding polyphony. It's been over a year I patch that for my fork, so I don't recall exactly in what file I found out the macros that needs to be modified in order to restrict polyphony of the engine to only one voice but it can be done, also I don't recall exactly what gain I had in term of RAM use but it was VERY sensible. My tweak version ran at something like 600 or 800 MB of RAM use, and after patching this useless polyphony feature on both engine I drop to 200 MB or so. My numbers I probably flawed due to my memory of the event, however I recall that even after implementing polyphony (four voice for each track) inside picoloop for each machine I still was running the program with way less RAM use as before stripping of polyphony of both synth engine. So If my previous observations isn't that useful for the main branch (even though it's cool to know about it), the one should be patch in newer version of picoloop since the resources gains are astonishing.

All that being said, I hope I would get some time to back working on my fork on one day being able to make it a standalone groovebox as I wanted so many years ago, and if I found out interesting things in terms of resources use optimization or bug fixing along I'll let you know. Also if I finally manage to put out my fork with the functionality I want I will be glad to share the code with you.