Clicks

Report anything that appears to be a bug here.

Moderator: jesse

Post Reply
uebe
Posts: 4
Joined: Tue Dec 20, 2011 8:30 am

Clicks

Post by uebe »

I found two bugs, that cause SL to click..
When I press undo_all and record a new loop, there's no fade in on the recorded loop. Seems to be because after undo_all fLoopFadeAtten is still 1.
I fixed it by insterting pLS->fLoopFadeAtten = 0.0f; in plugin.cc (Function: runSooperLooper) in case STATE_TRIG_START:

Code: Select all

				  // cause input-to-loop fade in
				  pLS->fLoopFadeAtten = 0.0f;
				  pLS->fLoopFadeDelta = 1.0f / xfadeSamples;
				  pLS->fPlayFadeDelta = -1.0f / xfadeSamples;
There's also a click of one sample in multiply. I changed the comparison to >= in plugin.cc (Function: runSooperLooper)

Code: Select all

		 // increment 
		 loop->dCurrPos = loop->dCurrPos + fRate;
	      

		 // ASSUMPTION: our rate is +1 only		 
		 if ((unsigned long)loop->dCurrPos  >= (loop->lLoopLength)) {
Two other small bugs: The state and waiting au parameters are not working. I disabled the else case in SooperLooperAU.cpp (Function: parameter_changed) to make the state parameter working:

Code: Select all

	//else {
	
		if (instance == -1) {
			// all loops
			for (size_t i=0; i < _engine->loop_count(); ++i) {
				paramid = (i+1)*10000 + ctrl_id;
				changedUnit.mParameterID = paramid;
				AUParameterListenerNotify (NULL, NULL, &changedUnit);
			}
			changedUnit.mParameterID = 1000 + ctrl_id;	
			AUParameterListenerNotify (NULL, NULL, &changedUnit);
		}
		else {
			// normal control
			changedUnit.mParameterID = paramid;
			AUParameterListenerNotify (NULL, NULL, &changedUnit);
			if (selected) {
				changedUnit.mParameterID = 1000 + ctrl_id;	
				AUParameterListenerNotify (NULL, NULL, &changedUnit);
			}
		}
		//cerr << "notifying for param change on: " << ctrl_id << "  inst: " << instance << endl;
	//}
And added this line in engine.cpp (Function: mainloop) to make the waiting parameter working:

Code: Select all

			// emit a parameter changed for state and others
			for (unsigned int n=0; n < _instances.size(); ++n) {
				ParamChanged(Event::State, n); // emit
				ParamChanged(Event::Waiting, n);
				ParamChanged(Event::LoopPosition, n);
				ParamChanged(Event::LoopLength, n);
				ParamChanged(Event::CycleLength, n);				
				ParamChanged(Event::FreeTime, n);
			}
Hope this helps...
Thanks for your work!!!
Greetings, Uebe
kasbah
Posts: 117
Joined: Tue Sep 06, 2011 7:01 pm

Re: Clicks

Post by kasbah »

I compiled and tested you changes on my netbook. I still get clicks on it though but I suspect this is a general performance issue as I never got these running it on my laptop. I can't seem to get rid of them on my netbook no matter what I set xfade to. So I thought this would help (I generally use undo-all a lot). Doesn't seem to break anything though :)

Can't really test the AU specific changes unfortunately. Thanks for the improvements. I have merged these into the dev branch on my repo where I am collecting all the stable changes so that Jesse can easily update for the next version.
uebe
Posts: 4
Joined: Tue Dec 20, 2011 8:30 am

Re: Clicks

Post by uebe »

Found some other clicks :-)
There are no real crossfades when using undo, redo, undo all, and redo all. Only fade-ins of the new loop. Depending on what you're playing this causes heavy clicks.. Here's a solution for real crossfades (fade out of the old loop while fading in the new one):
Added some new states for the sampler:

Code: Select all

/* States of the sampler */

#define STATE_OFF        0
#define STATE_TRIG_START 1
#define STATE_RECORD     2
#define STATE_TRIG_STOP  3
#define STATE_PLAY       4
#define STATE_OVERDUB    5
#define STATE_MULTIPLY   6
#define STATE_INSERT     7
#define STATE_REPLACE    8
#define STATE_DELAY      9
#define STATE_MUTE       10
#define STATE_SCRATCH    11
#define STATE_ONESHOT    12
#define STATE_SUBSTITUTE  13
#define STATE_PAUSED     14
#define STATE_UNDO_ALL	15//Uebe new
#define STATE_TRIGGER_PLAY 16
#define STATE_UNDO		17//Uebe new
#define STATE_REDO		18//Uebe new
#define STATE_REDO_ALL	19//Uebe new
All following changes are in the function runSooperLooper in plugin.cc
Some new variables and pointers...

Code: Select all

  unsigned int xCurrPos = 0;
  unsigned int lpCurrPos = 0;  
  LADSPA_Data *pLoopSample, *spLoopSample, *rLoopSample, *rpLoopSample, *xLoopSample;
  LoopChunk *lastloop, *prevloop, *nextloop;
More ore less rewriten part in switch(lMultiCtrl)

Code: Select all

	case MULTI_UNDO:
	case MULTI_UNDO_TWICE:
	{


	   switch(pLS->state) {
	      case STATE_PLAY:
	      case STATE_RECORD:
	      case STATE_OVERDUB:
	      case STATE_MULTIPLY:
	      case STATE_INSERT:
	      case STATE_TRIG_START:
	      case STATE_TRIG_STOP:		 
	      case STATE_REPLACE:
	      case STATE_SUBSTITUTE:
	      case STATE_DELAY:
	      case STATE_MUTE:
	      case STATE_PAUSED:
			   
			   // cancel whatever mode, back to play mode (or mute)
			   pLS->state = pLS->wasMuted ? STATE_MUTE : STATE_PLAY;
		      
		      if (pLS->waitingForSync) {
			      // don't undo loop
			      // just return to play (or mute)
			      pLS->waitingForSync = 0;
			      pLS->nextState = -1;
		      }
		      else if (pLS->fNextCurrRate != 0.0f) {
			      // undo pending reverse
			      pLS->fNextCurrRate = 0.0f;
		      }

			   if (pLS->state == STATE_MUTE) {
				   // undo ONE)
				   undoLoop(pLS, false);;
			   } else {
				   if (loop->prev) {
					   pLS->state = STATE_UNDO;
					   pLS->nextState = STATE_PLAY;
				   } else {
					   pLS->state = STATE_UNDO_ALL;
				   }

			   }
			   
			   pLS->fLoopFadeDelta = -1.0f / xfadeSamples;
			   pLS->fFeedFadeDelta = 1.0f / xfadeSamples;
			   
			   pLS->fPlayFadeAtten = 0.0f;
			   pLS->fPlayFadeDelta = 1.0f / xfadeSamples;
			   
			   if (pLS->state == STATE_MUTE) {
				   pLS->fPlayFadeDelta = 0.0f;
			   } else if (pLS->state == STATE_UNDO_ALL) {
				   pLS->fPlayFadeAtten = 1.0f;
				   pLS->fPlayFadeDelta = -1.0f / xfadeSamples;
			   }

		      DBG(fprintf(stderr,"%u:%u  Undoing and reentering PLAY state from UNDO\n", pLS->lLoopIndex, pLS->lChannelIndex));
		      break;
	   }
	   
	} break;

	case MULTI_UNDO_ALL:
	{
		if (pLS->state != STATE_OFF) {
			DBG(fprintf(stderr,"%u:%u  UNDO all loops\n", pLS->lLoopIndex, pLS->lChannelIndex));
			pLS->fPlayFadeDelta = -1.0f / xfadeSamples;
			
			pLS->fLoopFadeDelta = -1.0f / xfadeSamples;
			pLS->fFeedFadeDelta = 1.0f / xfadeSamples;
			
			pLS->state = pLS->wasMuted ? STATE_MUTE : STATE_PLAY;
			
			if (loop && loop->lLoopLength) {
				pLS->state = STATE_UNDO_ALL;
				
				pLS->fLoopFadeDelta = -1.0f / (xfadeSamples);
				pLS->fPlayFadeDelta = -1.0f / xfadeSamples;// fade out for undo all
				pLS->wasMuted = true;
			}			
		}
	} break;

	case MULTI_REDO_ALL:
	{
		if (pLS->state == STATE_OFF) {
			pLS->state = STATE_PLAY;
			pLS->wasMuted = false;
		} else {
			pLS->state = pLS->wasMuted ? STATE_MUTE : STATE_PLAY;
		}
		
		if (!loop || pLS->state == STATE_MUTE) {
			// redo ALL)
			lastloop = pLS->headLoopChunk;
			redoLoop(pLS);
			 
			while (pLS->headLoopChunk != lastloop) {
			lastloop = pLS->headLoopChunk;
			redoLoop(pLS);
			}
			if (!pLS->headLoopChunk) {
				pLS->state = STATE_OFF;
			}
		} else {
			if (loop->next) {
				pLS->state = STATE_REDO_ALL;
				pLS->nextState = STATE_PLAY;
			}
		}
		
		pLS->fLoopFadeDelta = -1.0f / xfadeSamples;
		pLS->fFeedFadeDelta = 1.0f / xfadeSamples;
		
		pLS->fPlayFadeAtten = 0.0f;
		pLS->fPlayFadeDelta = 1.0f / xfadeSamples;
		
		if (pLS->state == STATE_MUTE) {
			pLS->fPlayFadeDelta = 0.0f;
		}

		DBG(fprintf(stderr,"REDO all loops\n"));

	} break;
	
	case MULTI_REDO:
	{
	   switch (pLS->state) {
		   case STATE_PLAY:
		   case STATE_RECORD:
		   case STATE_TRIG_START:
		   case STATE_TRIG_STOP:		 
		   case STATE_OVERDUB:
		   case STATE_MULTIPLY:
		   case STATE_INSERT:
		   case STATE_REPLACE:
		   case STATE_SUBSTITUTE:
		   case STATE_MUTE:
		   case STATE_PAUSED:
		   case STATE_OFF:
			   
			   if (pLS->state == STATE_OFF) {
				   pLS->state = STATE_PLAY;
				   pLS->wasMuted = false;
			   } else {
				   pLS->state = pLS->wasMuted ? STATE_MUTE : STATE_PLAY;
			   }
			   
			   if (!loop || pLS->state == STATE_MUTE) {
				   // we don't need a fadeout
				   redoLoop(pLS);
				   if (!pLS->headLoopChunk) {
					   pLS->state = STATE_OFF;
				   }
			   } else {
				   // we need a x-fade
				   if (loop->next) {
					   pLS->state = STATE_REDO;
					   pLS->nextState = STATE_PLAY;
					   pLS->fPlayFadeAtten = 0.0f;
				   }
			   }
			   
			   pLS->fLoopFadeDelta = -1.0f / xfadeSamples;
			   pLS->fFeedFadeDelta = 1.0f / xfadeSamples;
			   			   
			   if (pLS->state == STATE_MUTE) {
				   // we don't need a fade in
				   pLS->fPlayFadeDelta = 0.0f;
			   } else {
				   pLS->fPlayFadeDelta = 1.0f / xfadeSamples;
			   }

			break;
	   }

	} break;
Added the new states to the STATE_PLAY in switch(pLS->state)

Code: Select all

	case STATE_UNDO_ALL:
	case STATE_REDO_ALL:
	case STATE_UNDO:
	case STATE_REDO:
	case STATE_PLAY:
	case STATE_ONESHOT:
	case STATE_SCRATCH:
	case STATE_MUTE:
	case STATE_PAUSED:
	{
	   //fprintf(stderr,"in play begin\n");	   
	   // play  the input out mixed with the recorded loop.
Changes in the for loop (;lSampleIndex < SampleCount;
lSampleIndex++)

Code: Select all

		 lCurrPos =(unsigned int) fmod(loop->dCurrPos, loop->lLoopLength);
		 //fprintf(stderr, "curr = %u\n", lCurrPos);
		 pLoopSample = & pLS->pSampleBuf[(loop->lLoopStart + lCurrPos) & pLS->lBufferSizeMask];
			  
			  if (pLS->state == STATE_UNDO){
				  prevloop = pLS->headLoopChunk->prev;
				  xCurrPos = (unsigned int) fmod(loop->dCurrPos, prevloop->lLoopLength);
				  xLoopSample = & pLS->pSampleBuf[(prevloop->lLoopStart + xCurrPos) & pLS->lBufferSizeMask];
			  }
			  if (pLS->state == STATE_REDO) {
				  nextloop = pLS->headLoopChunk->next;
				  xCurrPos = (unsigned int) fmod(loop->dCurrPos, nextloop->lLoopLength);
				  xLoopSample = & pLS->pSampleBuf[(nextloop->lLoopStart + xCurrPos) & pLS->lBufferSizeMask];
			  }
			  if (pLS->state == STATE_REDO_ALL) {
				  nextloop = pLS->headLoopChunk;
				  while (nextloop->next) {
					  nextloop = nextloop->next;
				  }
				  xCurrPos = (unsigned int) fmod(loop->dCurrPos, nextloop->lLoopLength);
				  xLoopSample = & pLS->pSampleBuf[(nextloop->lLoopStart + xCurrPos) & pLS->lBufferSizeMask];
			  }

		 rCurrPos = fmod (loop->dCurrPos - (fRate * (lOutputLatency + lInputLatency)), loop->lLoopLength);
		 if (rCurrPos < 0) {
			 rCurrPos += loop->lLoopLength;
		 }

Code: Select all

		 fOutputSample =   tmpWet *  (*pLoopSample)
		    + fDry * fInputSample;
		 if (pLS->state == STATE_UNDO || pLS->state == STATE_REDO || pLS->state == STATE_REDO_ALL) {
			 //fprintf(stderr, "fading.. :%g\n", tmpWet);
			 fOutputSample =   tmpWet *  (*xLoopSample)
			 + fDry * fInputSample + (fWet-tmpWet) * (*pLoopSample);
		 }

		 // jlc play
and after the for loop

Code: Select all

		   if (pLS->state == STATE_UNDO && pLS->fPlayFadeAtten == 1.0f) {
			   // play some of the old loop first and switch later
			   undoLoop(pLS, false);
			   fprintf(stderr, "finished UNDO...\n");
			   pLS->state = pLS->nextState;
		   }
		   if (pLS->state == STATE_REDO && pLS->fPlayFadeAtten == 1.0f) {
			   // play some of the old loop first and switch later
			   redoLoop(pLS);
			   fprintf(stderr, "finished REDO...\n");
			   pLS->state = pLS->nextState;
		   }
		   if (pLS->state == STATE_UNDO_ALL && pLS->fPlayFadeAtten == 0.0f) {
			   // fade out the old loop and goto sate_off
			   clearLoopChunks(pLS);
			   fprintf(stderr, "finished UNDO ALL...\n");
			   pLS->state = STATE_OFF;
		   }
		   if (pLS->state == STATE_REDO_ALL && pLS->fPlayFadeAtten == 1.0f) {
			   // play some of the old loop first and switch later
			   lastloop = pLS->headLoopChunk;
			   redoLoop(pLS);
			   while (pLS->headLoopChunk != lastloop) {
				   lastloop = pLS->headLoopChunk;
				   redoLoop(pLS);
			   }
			   fprintf(stderr, "finished REDO ALL...\n");
			   pLS->state = pLS->nextState;
		   }
		   
		   
	      // recenter around the mod
	      if (loop) {
		      lCurrPos = (unsigned int) fabs(fmod(loop->dCurrPos, loop->lLoopLength));
		      
		      if (recenter) {
			      loop->dCurrPos = lCurrPos + modf(loop->dCurrPos, &dDummy);
		      }
	      }
Hope you can check this and merge it into the dev repo.
Greetings, Uebe
kasbah
Posts: 117
Joined: Tue Sep 06, 2011 7:01 pm

Re: Clicks

Post by kasbah »

Thanks Uebe! It is a bit cumbersome to redo-all your changes though. Could you make a patch? Or fork the github repo and make a pull request? I am actually collecting all the changes in master branch now.
uebe
Posts: 4
Joined: Tue Dec 20, 2011 8:30 am

Re: Clicks

Post by uebe »

Ok, I understand..
Here's the patch file, you can apply on plugin.cc
Attachments
plugin.cc.xfade.patch.zip
patch for plugin.cc
(3.67 KiB) Downloaded 1485 times
kasbah
Posts: 117
Joined: Tue Sep 06, 2011 7:01 pm

Re: Clicks

Post by kasbah »

OK, great! I have put these on the "fix_xfade" branch: https://github.com/kasbah/sooperlooper/tree/fix_xfade

Will give them a spin before merging into master.
kasbah
Posts: 117
Joined: Tue Sep 06, 2011 7:01 pm

Re: Clicks

Post by kasbah »

I noticed some things while playing around with this.

The default undo behaviour of SL has been that if you record, then do something else or record again and then hit undo twice it will only undo up to the first record. It won't go back to the "Off" state. For example, take these actions and the states that follow when they complete:

Code: Select all

Record  - State:Playing
Undo    - State:Off
Record  - State:Playing
Record  - State:Playing
Undo    - State:Playing
Undo    - State:Playing
To me it would make more sense if it went back to the "Off" state but maybe Jesse did this for a reason. I can see that in live use it may be useful as you can just mash undo but never worry about accidentally going back to the "Off" state.

Now with your patch it will go back to "Off" _if_ it isn't muted. When muted it defaults to the old behaviour. Whichever behaviour we go for, it should do the same whether muted or not. I was also thinking about trying to add a MUTED_PLAYING and MUTED_OFF state so that you can un-mute when muted and off.

Another thing, what about clicks when redoing, do they need to be addressed in a similar way?
Post Reply