tiagolr / time1

GNU General Public License v3.0
47 stars 1 forks source link

Plugin Can Lose Project Sync #3

Open Mrugalla opened 5 months ago

Mrugalla commented 5 months ago

as shown here as well^^: https://youtu.be/kNccM_mzQCA?si=SFopOhUrSR8L-jfv&t=167

tiagolr commented 5 months ago

I added a fix for this, not sure if its working I may have to get my hands on Bitwig to test if its fixed and also GATE1 potential similar issues.

tiagolr commented 5 months ago

Not able to reproduce the project sync issues, I try loading a wave file, move it to second beat and both TIME1 and GATE1 start on second beat aswell, any tips to replay this issue?

Mrugalla commented 5 months ago

It happens only when the plugin was just activated. this can be the case when you had it on a track and just opened the project file of your DAW or when you had deactivated the plugin during your session (in bitwig that means hitting ALT+A on the plugin, but idk how it works in FL).

Deactivating is different from bypassing, because the entire plugin stops processing and has to be reloaded entirely if you activate it again, while a bypassed plugin can still process audio if the processBlockBypassed method (that's what it is called in JUCE at least) is implemented.

Since it only happens when the plugin was just instantiated I suppose it does something weird in the method where it prepares itself for audio processing. Often it is possible to sync up the processes in processBlock as well to make sure that these things resolve themselves quickly

Mrugalla commented 5 months ago

Update: it can not be reproduced in Cubase Artist 9.5.3 so it might be a Bitwig-specific bug. Maybe it would be good to ask them for an NFR. You gotta be really careful with project sync features in that DAW. some plugin features are simply not implemented, like the bool that tells you if it currently loops, and some features are buggily implemented. for example Bitwig only sends the correct project position to the plugins if you have moved the start playhead and all loop points somewhere else at least once after you have opened a project. I did make sure to do that when testing your plugin to rule out if it's because of that though and it doesn't seem to be.

Mrugalla commented 4 months ago

additional thought: could it be that Time1 uses the very first processBlock call to find out where it is in the project and then doesn't continue to sync it up anymore for the next calls until the temposync rate parameter has changed? if yes: a possible fix could be that the plugin doesn't only update this state on parameter changes, but also on bpm and project position changes. you can identify if the project position is in an unexpected position by adding the number of samples per block to the current project position in samples every processBlock. I once made a video about stuff like that. it's in the context of a JUCE project but I tried to visualize the concept in many different ways. maybe you can find something useful there, too: https://youtu.be/rZTO2KB3DW4?si=b239ONGykWR_m0Ch

tiagolr commented 4 months ago

Thanks, I haven´t looked into this issue so far as I'm fixing HighDPI support first.

It is true that the SyncQuarterNotes calculation is only done when param temposync changes, I even added an extra call to try to fix this issue, seems it didn't work.

I would ask if you can give me the exact steps to reproduce this issue as I could not replay it last time, I have Bitwig studio installed, should I deactivate the plugin, move the project position and reactivate the plugin? Are those the steps? I'll try again very soon.

Mrugalla commented 4 months ago

It happens everytime you load the plugin. so either when you load a project that already has instances of it or when you had deactivated the plugin and now reactivated it. deactivating plugins is done by hitting ALT+A on the module of the plugin

tiagolr commented 4 months ago

still not able to reproduce this issue :(, its possible in your clip that when you start the playback is not in sync because the delay buffer does not start at the beginning of the loop, I cannot find an issue in the code with project sync as you suggest or with the play position being in sync with the project, I also pushed an attempt fix for this in a later version maybe it worked.

Mrugalla commented 4 months ago

Ok, I took another look at the problem and I noticed that it's probably not (or not just) a problem with the project position, but of the bpm. check out this video in which I switch between different bpms: https://www.youtube.com/watch?v=_A2Ie8jUXw8

at 0:03 you hear the plugin correctly doing the modulation. that's what i expected, because it's at 120bpm, which is often considered to be the default bpm. the graphics of the oscilloscope are not quite right, but the sound is there.

at 0:06 i go to a higher bpm and without reactivating the plugin it now has the wrong timing. so apparently bpm changes do not reset the phase of the modulation atm.

at 0:18 i do the same with bpms lower than 120 and the opposite problem happens, where it now chokes a bunch of transients. but basically the same problem.

at 0:26 i reactivate the plugin to find out that it sounds exactly the same as before, so an instance that was just activated also seems to use the default value of 120bpm instead of using the one given by the project.

at 0:36 i do the only thing that resets it successfully atm, which is changing the sync parameter's value.


I took a look at the code. Here's the line where you get the current tempo and use it to change the speed of the modulator. https://github.com/tiagolr/time1/blob/master/src/TIME1.cpp#L313

It's being used here to move the phase, the beatPos. https://github.com/tiagolr/time1/blob/master/src/TIME1.cpp#L383

I personally made the experience that things tend to run out of sync easily when using systems where you just add a value to the phase of something, instead of synthesizing the phase for each frame from the ppq and bpm at all times. but that might not be the problem here. after all the moment getTempo() is called the new tempo should be there so it should at least modulate at that speed then.

since that left me confused I decided to instead look at the case that works to see if any of that needs to be in processBlock: https://github.com/tiagolr/time1/blob/master/src/TIME1.cpp#L249

so apparently there are multiple things here:

  1. controls are dirty
  2. syncQN is being defined
  3. delay is being resized

given that with the current structure you have there the delay needs to be resized on bpm changes and not just on parameter changes