- Fixed: When note_on() is called and another copy of the same note is

already playing on the channel, it should stop it with finish_note(), not
  kill_note(). This can be clearly heard in the final cymbal crashes of
  D_DM2TTL where TiMidity cuts them off because the final cymbals are played
  with a velocity of 1 before the preceding cymbals have finished. (I wonder
  if I should be setting the self_nonexclusive flag for GUS patches to
  disable even this behavior, though, since gf1note.c doesn't turn off
  duplicate notes.)
- Changed envelope handling to hopefully match the GUS player's. The most
  egregious mistake TiMidity makes is to treat bit 6 as an envelope enable
  bit. This is not what it does; every sample has an envelope. Rather, this
  is a "no sampled release" flag. Also, despite fiddling with the
  PATCH_SUSTAIN flag during instrument loading, TiMidity never actually
  used it. Nor did it do anything at all with the PATCH_FAST_REL flag.


SVN r934 (trunk)

docs/rh-log.txt

@@ -1,3 +1,20 @@
+April 23, 2008
+(Note to self: xplasma.mid sounds really bad. Find out why.)
+- Fixed: When note_on() is called and another copy of the same note is
+  already playing on the channel, it should stop it with finish_note(), not
+  kill_note(). This can be clearly heard in the final cymbal crashes of
+  D_DM2TTL where TiMidity cuts them off because the final cymbals are played
+  with a velocity of 1 before the preceding cymbals have finished. (I wonder
+  if I should be setting the self_nonexclusive flag for GUS patches to
+  disable even this behavior, though, since gf1note.c doesn't turn off
+  duplicate notes.)
+- Changed envelope handling to hopefully match the GUS player's. The most
+  egregious mistake TiMidity makes is to treat bit 6 as an envelope enable
+  bit. This is not what it does; every sample has an envelope. Rather, this
+  is a "no sampled release" flag. Also, despite fiddling with the
+  PATCH_SUSTAIN flag during instrument loading, TiMidity never actually
+  used it. Nor did it do anything at all with the PATCH_FAST_REL flag.
+
 April 23, 2008 (Changes by Graf Zahl)
 - Fixed: wbstartstruct's lump name fields were only 8 characters long
   and not properly zero-terminated when all 8 characters were used.

src/sound/music_timidity_mididevice.cpp

@@ -598,38 +598,38 @@ FString TimidityMIDIDevice::GetStats()
 	CritSec.Enter();
 	for (i = used = 0; i < Renderer->voices; ++i)
 	{
-		switch (Renderer->voice[i].status)
+		int status = Renderer->voice[i].status;
+
+		if (!(status & Timidity::VOICE_RUNNING))
 		{
-		case Timidity::VOICE_FREE:
 			dots << TEXTCOLOR_PURPLE".";
-			break;
-
-		case Timidity::VOICE_ON:
-			dots << TEXTCOLOR_GREEN;
-			break;
-
-		case Timidity::VOICE_SUSTAINED:
-			dots << TEXTCOLOR_BLUE;
-			break;
-
-		case Timidity::VOICE_OFF:
-			dots << TEXTCOLOR_ORANGE;
-			break;
-
-		case Timidity::VOICE_DIE:
-			dots << TEXTCOLOR_RED;
-			break;
 		}
-		if (Renderer->voice[i].status != Timidity::VOICE_FREE)
+		else
 		{
 			used++;
-			if (Renderer->voice[i].envelope_increment == 0)
+			if (status & Timidity::VOICE_SUSTAINING)
 			{
-				dots << TEXTCOLOR_BLUE"+";
+				dots << TEXTCOLOR_BLUE;
+			}
+			else if (status & Timidity::VOICE_RELEASING)
+			{
+				dots << TEXTCOLOR_ORANGE;
+			}
+			else if (status & Timidity::VOICE_STOPPING)
+			{
+				dots << TEXTCOLOR_RED;
 			}
 			else
 			{
-				dots << ('1' + Renderer->voice[i].envelope_stage);
+				dots << TEXTCOLOR_GREEN;
+			}
+			if (Renderer->voice[i].envelope_increment == 0)
+			{
+				dots << "+";
+			}
+			else
+			{
+				dots << ('0' + Renderer->voice[i].envelope_stage);
 			}
 		}
 	}

src/timidity/instrum.cpp

@@ -359,6 +359,7 @@ fail:
 			}
 		}
 
+#if 0
 		/* seashore.pat in the Midia patch set has no Sustain. I don't
 		   understand why, and fixing it by adding the Sustain flag to
 		   all looped patches probably breaks something else. We do it
@@ -411,6 +412,7 @@ fail:
 				cmsg(CMSG_INFO, VERB_DEBUG, " - No sustain, removing envelope\n");
 			}
 		}
+#endif
 
 		for (j = 0; j < 6; j++)
 		{

src/timidity/mix.cpp

@@ -38,35 +38,35 @@ int recompute_envelope(Voice *v)
 
 	stage = v->envelope_stage;
 
-	if (stage > RELEASEC)
+	if (stage >= ENVELOPES)
 	{
 		/* Envelope ran out. */
-		v->status = VOICE_FREE;
+		/* play sampled release */
+		v->status &= ~(VOICE_SUSTAINING | VOICE_LPE);
+		v->status |= VOICE_RELEASING | VOICE_STOPPING;
 		return 1;
 	}
 
-	if (v->sample->modes & PATCH_NO_SRELEASE)
+	if (stage == RELEASE && !(v->status & VOICE_RELEASING) && (v->sample->modes & PATCH_SUSTAIN))
 	{
-		if (v->status == VOICE_ON || v->status == VOICE_SUSTAINED)
+		v->status |= VOICE_SUSTAINING;
-		{
+		/* Freeze envelope until note turns off. Trumpets want this. */
-			if (stage > DECAY)
+		v->envelope_increment = 0;
-			{
-				/* Freeze envelope until note turns off. Trumpets want this. */
-				v->envelope_increment = 0;
-				return 0;
-			}
-		}
 	}
-	v->envelope_stage = stage + 1;
+	else
+	{
+		v->envelope_stage = stage + 1;
 
-	if (v->envelope_volume == v->sample->envelope_offset[stage])
+		if (v->envelope_volume == v->sample->envelope_offset[stage])
-	{
+		{
-		return recompute_envelope(v);
+			return recompute_envelope(v);
+		}
+		v->envelope_target = v->sample->envelope_offset[stage];
+		v->envelope_increment = v->sample->envelope_rate[stage];
+		if (v->envelope_target < v->envelope_volume)
+			v->envelope_increment = -v->envelope_increment;
 	}
-	v->envelope_target = v->sample->envelope_offset[stage];
+
-	v->envelope_increment = v->sample->envelope_rate[stage];
-	if (v->envelope_target < v->envelope_volume)
-		v->envelope_increment = -v->envelope_increment;
 	return 0;
 }
 
@@ -78,10 +78,7 @@ void apply_envelope_to_amp(Voice *v)
 	{
 		env_vol *= v->tremolo_volume;
 	}
-	if (v->sample->modes & PATCH_NO_SRELEASE)
+	env_vol *= v->envelope_volume / float(1 << 30);
-	{
-		env_vol *= v->envelope_volume / float(1 << 30);
-	}
 	// Note: The pan offsets are negative.
 	v->left_mix = MAX(0.f, (float)calc_gf1_amp(env_vol + v->left_offset) * final_amp);
 	v->right_mix = MAX(0.f, (float)calc_gf1_amp(env_vol + v->right_offset) * final_amp);
@@ -426,13 +423,13 @@ void mix_voice(Renderer *song, float *buf, Voice *v, int c)
 	{
 		return;
 	}
-	if (v->status == VOICE_DIE)
+	if (v->status & VOICE_STOPPING)
 	{
 		if (count >= MAX_DIE_TIME)
 			count = MAX_DIE_TIME;
 		sp = resample_voice(song, v, &count);
 		ramp_out(sp, buf, v, count);
-		v->status = VOICE_FREE;
+		v->status = 0;
 	}
 	else
 	{

src/timidity/playmidi.cpp

@@ -37,7 +37,7 @@ void Renderer::reset_voices()
 {
 	for (int i = 0; i < voices; i++)
 	{
-		voice[i].status = VOICE_FREE;
+		voice[i].status = 0;
 	}
 }
 
@@ -254,7 +254,7 @@ void Renderer::kill_key_group(int i)
 	}
 	while (j--)
 	{
-		if (voice[j].status != VOICE_ON && voice[j].status != VOICE_SUSTAINED) continue;
+		if ((voice[j].status & VOICE_RUNNING) && !(voice[j].status & (VOICE_RELEASING | VOICE_STOPPING))) continue;
 		if (i == j) continue;
 		if (voice[i].channel != voice[j].channel) continue;
 		if (voice[j].sample->key_group != voice[i].sample->key_group) continue;
@@ -273,6 +273,7 @@ void Renderer::start_note(int chan, int note, int vel, int i)
 	Instrument *ip;
 	int bank = channel[chan].bank;
 	int prog = channel[chan].program;
+	Voice *v = &voice[i];
 
 	if (ISDRUMCHANNEL(chan))
 	{
@@ -301,66 +302,71 @@ void Renderer::start_note(int chan, int note, int vel, int i)
 	}
 	if (ip->sample->scale_factor != 1024)
 	{
-		voice[i].orig_frequency = calculate_scaled_frequency(ip->sample, note & 0x7F);
+		v->orig_frequency = calculate_scaled_frequency(ip->sample, note & 0x7F);
 	}
 	else
 	{
-		voice[i].orig_frequency = note_to_freq(note & 0x7F);
+		v->orig_frequency = note_to_freq(note & 0x7F);
 	}
 	select_sample(i, ip, vel);
 
-	voice[i].status = VOICE_ON;
+	v->status = VOICE_RUNNING;
-	voice[i].channel = chan;
+	v->channel = chan;
-	voice[i].note = note;
+	v->note = note;
-	voice[i].velocity = vel;
+	v->velocity = vel;
-	voice[i].sample_offset = 0;
+	v->sample_offset = 0;
-	voice[i].sample_increment = 0; /* make sure it isn't negative */
+	v->sample_increment = 0; /* make sure it isn't negative */
 
-	voice[i].tremolo_phase = 0;
+	v->tremolo_phase = 0;
-	voice[i].tremolo_phase_increment = voice[i].sample->tremolo_phase_increment;
+	v->tremolo_phase_increment = voice[i].sample->tremolo_phase_increment;
-	voice[i].tremolo_sweep = voice[i].sample->tremolo_sweep_increment;
+	v->tremolo_sweep = voice[i].sample->tremolo_sweep_increment;
-	voice[i].tremolo_sweep_position = 0;
+	v->tremolo_sweep_position = 0;
 
-	voice[i].vibrato_sweep = voice[i].sample->vibrato_sweep_increment;
+	v->vibrato_sweep = voice[i].sample->vibrato_sweep_increment;
-	voice[i].vibrato_sweep_position = 0;
+	v->vibrato_sweep_position = 0;
-	voice[i].vibrato_control_ratio = voice[i].sample->vibrato_control_ratio;
+	v->vibrato_control_ratio = voice[i].sample->vibrato_control_ratio;
-	voice[i].vibrato_control_counter = voice[i].vibrato_phase = 0;
+	v->vibrato_control_counter = voice[i].vibrato_phase = 0;
 
 	kill_key_group(i);
 
-	memset(voice[i].vibrato_sample_increment, 0, sizeof(voice[i].vibrato_sample_increment));
+	memset(v->vibrato_sample_increment, 0, sizeof(v->vibrato_sample_increment));
 
 	if (channel[chan].panning != NO_PANNING)
 	{
-		voice[i].left_offset = channel[chan].left_offset;
+		v->left_offset = channel[chan].left_offset;
-		voice[i].right_offset = channel[chan].right_offset;
+		v->right_offset = channel[chan].right_offset;
 	}
 	else
 	{
-		voice[i].left_offset = voice[i].sample->left_offset;
+		v->left_offset = v->sample->left_offset;
-		voice[i].right_offset = voice[i].sample->right_offset;
+		v->right_offset = v->sample->right_offset;
 	}
 
 	recompute_freq(i);
-	recompute_amp(&voice[i]);
+	recompute_amp(v);
-	if (voice[i].sample->modes & PATCH_NO_SRELEASE)
+
+	/* Ramp up from 0 */
+	v->envelope_stage = ATTACK;
+	v->envelope_volume = 0;
+	v->control_counter = 0;
+	recompute_envelope(v);
+	apply_envelope_to_amp(v);
+
+	if (v->sample->modes & PATCH_LOOPEN)
 	{
-		/* Ramp up from 0 */
+		v->status |= VOICE_LPE;
-		voice[i].envelope_stage = ATTACK;
-		voice[i].envelope_volume = 0;
-		voice[i].control_counter = 0;
-		recompute_envelope(&voice[i]);
 	}
-	else
-	{
-		voice[i].envelope_increment = 0;
-	}
-	apply_envelope_to_amp(&voice[i]);
 }
 
 void Renderer::kill_note(int i)
 {
-	voice[i].status = VOICE_DIE;
+	Voice *v = &voice[i];
+
+	if (v->status & VOICE_RUNNING)
+	{
+		v->status &= ~VOICE_SUSTAINING;
+		v->status |= VOICE_RELEASING | VOICE_STOPPING;
+	}
 }
 
 /* Only one instance of a note can be playing on a single channel. */
@@ -377,13 +383,20 @@ void Renderer::note_on(int chan, int note, int vel)
 
 	while (i--)
 	{
-		if (voice[i].status == VOICE_FREE)
+		if (!(voice[i].status & VOICE_RUNNING))
 		{
 			lowest = i; /* Can't get a lower volume than silence */
 		}
 		else if (voice[i].channel == chan && ((voice[i].note == note && !voice[i].sample->self_nonexclusive) || channel[chan].mono))
 		{
-			kill_note(i);
+			if (channel[chan].mono)
+			{
+				kill_note(i);
+			}
+			else
+			{
+				finish_note(i);
+			}
 		}
 	}
 
@@ -400,7 +413,7 @@ void Renderer::note_on(int chan, int note, int vel)
 		i = voices;
 		while (i--)
 		{
-			if (voice[i].status != VOICE_ON && voice[i].status != VOICE_DIE)
+			if ((voice[i].status & VOICE_RELEASING) && !(voice[i].status & VOICE_STOPPING))
 			{
 				v = voice[i].attenuation;
 				if (v < lv)
@@ -420,7 +433,7 @@ void Renderer::note_on(int chan, int note, int vel)
 		   we could use a reserve of voices to play dying notes only. */
 
 		cut_notes++;
-		voice[lowest].status = VOICE_FREE;
+		voice[lowest].status = 0;
 		start_note(chan, note, vel, lowest);
 	}
 	else
@@ -431,44 +444,54 @@ void Renderer::note_on(int chan, int note, int vel)
 
 void Renderer::finish_note(int i)
 {
-	if (voice[i].sample->modes & PATCH_NO_SRELEASE)
+	Voice *v = &voice[i];
+
+	if ((v->status & (VOICE_RUNNING | VOICE_RELEASING)) == VOICE_RUNNING)
 	{
-		/* We need to get the envelope out of Sustain stage */
+		v->status &= ~VOICE_SUSTAINING;
-		voice[i].envelope_stage = RELEASE;
+		v->status |= VOICE_RELEASING;
-		voice[i].status = VOICE_OFF;
+
-		recompute_envelope(&voice[i]);
+		if (!(v->sample->modes & PATCH_NO_SRELEASE))
-		apply_envelope_to_amp(&voice[i]);
+		{
-	}
+			v->status &= ~VOICE_LPE;	/* sampled release */
-	else
+		}
-	{
+		if (!(v->sample->modes & PATCH_NO_SRELEASE) || (v->sample->modes & PATCH_FAST_REL))
-		/* Set status to OFF so resample_voice() will let this voice out
+		{
-		   of its loop, if any. In any case, this voice dies when it
+			/* ramp out to minimum volume with rate from final release stage */
-		   hits the end of its data (ofs >= data_length). */
+			v->envelope_stage = RELEASEC;
-		voice[i].status = VOICE_OFF;
+			recompute_envelope(v);
+			// Get rate from the final release ramp, but force the target to 0.
+			v->envelope_target = 0;
+			v->envelope_increment = -v->sample->envelope_rate[RELEASEC];
+		}
+		else if (v->sample->modes & PATCH_SUSTAIN)
+		{
+			if (v->envelope_stage < RELEASE)
+			{
+				v->envelope_stage = RELEASE;
+			}
+			recompute_envelope(v);
+		}
 	}
 }
 
 void Renderer::note_off(int chan, int note, int vel)
 {
-	int i = voices;
+	int i;
-	while (i--)
+	
+	for (i = voices; i-- > 0; )
 	{
-		if (voice[i].status == VOICE_ON &&
+		if ((voice[i].status & VOICE_RUNNING) && !(voice[i].status & (VOICE_RELEASING | VOICE_STOPPING))
-			voice[i].channel == chan &&
+			&& voice[i].channel == chan && voice[i].note == note)
-			voice[i].note == note)
 		{
 			if (channel[chan].sustain)
 			{
-				voice[i].status = VOICE_SUSTAINED;
+				voice[i].status |= NOTE_SUSTAIN;
 			}
 			else
 			{
 				finish_note(i);
 			}
-			if (!voice[i].sample->self_nonexclusive)
-			{
-				return;
-			}
 		}
 	}
 }
@@ -479,11 +502,11 @@ void Renderer::all_notes_off(int chan)
 	int i = voices;
 	while (i--)
 	{
-		if (voice[i].status == VOICE_ON && voice[i].channel == chan)
+		if ((voice[i].status & VOICE_RUNNING) && voice[i].channel == chan)
 		{
 			if (channel[chan].sustain) 
 			{
-				voice[i].status = VOICE_SUSTAINED;
+				voice[i].status |= NOTE_SUSTAIN;
 			}
 			else
 			{
@@ -500,8 +523,8 @@ void Renderer::all_sounds_off(int chan)
 	while (i--)
 	{
 		if (voice[i].channel == chan && 
-			voice[i].status != VOICE_FREE &&
+			(voice[i].status & VOICE_RUNNING) &&
-			voice[i].status != VOICE_DIE)
+			!(voice[i].status & VOICE_STOPPING))
 		{
 			kill_note(i);
 		}
@@ -513,7 +536,7 @@ void Renderer::adjust_pressure(int chan, int note, int amount)
 	int i = voices;
 	while (i--)
 	{
-		if (voice[i].status == VOICE_ON &&
+		if ((voice[i].status & VOICE_RUNNING) &&
 			voice[i].channel == chan &&
 			voice[i].note == note)
 		{
@@ -535,8 +558,7 @@ void Renderer::adjust_panning(int chan)
 	int i = voices;
 	while (i--)
 	{
-		if ((voice[i].channel == chan) &&
+		if ((voice[i].channel == chan) && (voice[i].status & VOICE_RUNNING))
-			(voice[i].status == VOICE_ON || voice[i].status == VOICE_SUSTAINED))
 		{
 			voice[i].left_offset = chanp->left_offset;
 			voice[i].right_offset = chanp->right_offset;
@@ -550,7 +572,7 @@ void Renderer::drop_sustain(int chan)
 	int i = voices;
 	while (i--)
 	{
-		if (voice[i].status == VOICE_SUSTAINED && voice[i].channel == chan)
+		if (voice[i].channel == chan && (voice[i].status & NOTE_SUSTAIN))
 		{
 			finish_note(i);
 		}
@@ -562,7 +584,7 @@ void Renderer::adjust_pitchbend(int chan)
 	int i = voices;
 	while (i--)
 	{
-		if (voice[i].status != VOICE_FREE && voice[i].channel == chan)
+		if ((voice[i].status & VOICE_RUNNING) && voice[i].channel == chan)
 		{
 			recompute_freq(i);
 		}
@@ -574,8 +596,7 @@ void Renderer::adjust_volume(int chan)
 	int i = voices;
 	while (i--)
 	{
-		if (voice[i].channel == chan &&
+		if (voice[i].channel == chan && (voice[i].status & VOICE_RUNNING))
-			(voice[i].status == VOICE_ON || voice[i].status == VOICE_SUSTAINED))
 		{
 			recompute_amp(&voice[i]);
 			apply_envelope_to_amp(&voice[i]);

src/timidity/resample.cpp

@@ -81,7 +81,7 @@ static sample_t *rs_plain(sample_t *resample_buffer, Voice *v, int *countptr)
 	if (ofs >= le) 
 	{
 		FINALINTERP;
-		v->status = VOICE_FREE;
+		v->status = 0;
 		*countptr -= count + 1;
 	}
 
@@ -309,7 +309,7 @@ static sample_t *rs_vib_plain(sample_t *resample_buffer, float rate, Voice *vp,
 		if (ofs >= le)
 		{
 			FINALINTERP;
-			vp->status = VOICE_FREE;
+			vp->status = 0;
 			*countptr -= count+1;
 			break;
 		}
@@ -494,7 +494,7 @@ sample_t *resample_voice(Renderer *song, Voice *vp, int *countptr)
 		if (*countptr >= (vp->sample->data_length >> FRACTION_BITS) - ofs)
 		{
 			/* Note finished. Free the voice. */
-			vp->status = VOICE_FREE;
+			vp->status = 0;
 
 			/* Let the caller know how much data we had left */
 			*countptr = (vp->sample->data_length >> FRACTION_BITS) - ofs;
@@ -511,9 +511,7 @@ sample_t *resample_voice(Renderer *song, Voice *vp, int *countptr)
 
 	if (vp->vibrato_control_ratio)
 	{
-		if ((modes & PATCH_LOOPEN) &&
+		if (vp->status & VOICE_LPE)
-			((modes & PATCH_NO_SRELEASE) ||
-			 (vp->status == VOICE_ON || vp->status == VOICE_SUSTAINED)))
 		{
 			if (modes & PATCH_BIDIR)
 				return rs_vib_bidir(song->resample_buffer, song->rate, vp, *countptr);
@@ -527,9 +525,7 @@ sample_t *resample_voice(Renderer *song, Voice *vp, int *countptr)
 	}
 	else
 	{
-		if ((modes & PATCH_LOOPEN) &&
+		if (vp->status & VOICE_LPE)
-			((modes & PATCH_NO_SRELEASE) ||
-			(vp->status == VOICE_ON || vp->status == VOICE_SUSTAINED)))
 		{
 			if (modes & PATCH_BIDIR)
 				return rs_bidir(song->resample_buffer, vp, *countptr);

src/timidity/timidity.cpp

@@ -500,7 +500,7 @@ void Renderer::ComputeOutput(float *buffer, int count)
 	}
 	for (int i = 0; i < voices; i++, v++)
 	{
-		if (v->status != VOICE_FREE)
+		if (v->status & VOICE_RUNNING)
 		{
 			mix_voice(this, buffer, v, count);
 		}

src/timidity/timidity.h

@@ -315,8 +315,8 @@ struct GF1PatchData
 	int   RootFrequency;
 	SWORD Tune;
 	BYTE  Balance;
-	BYTE  EnvelopeRate[6];
+	BYTE  EnvelopeRate[ENVELOPES];
-	BYTE  EnvelopeOffset[6];
+	BYTE  EnvelopeOffset[ENVELOPES];
 	BYTE  TremoloSweep;
 	BYTE  TremoloRate;
 	BYTE  TremoloDepth;
@@ -444,7 +444,7 @@ struct Channel
 {
 	int
 		bank, program, sustain, pitchbend, 
-		mono, /* one note only on this channel -- not implemented yet */
+		mono, /* one note only on this channel */
 		pitchsens;
 	WORD
 		volume, expression;
@@ -462,8 +462,6 @@ struct Channel
 
 /* Causes the instrument's default panning to be used. */
 #define NO_PANNING				-1
-/* envelope points */
-#define MAXPOINT				6
 
 struct Voice
 {
@@ -496,11 +494,13 @@ struct Voice
 /* Voice status options: */
 enum
 {
-	VOICE_FREE,
+	VOICE_RUNNING			= (1<<0),
-	VOICE_ON,
+	VOICE_SUSTAINING		= (1<<1),
-	VOICE_SUSTAINED,
+	VOICE_RELEASING			= (1<<2),
-	VOICE_OFF,
+	VOICE_STOPPING			= (1<<3),
-	VOICE_DIE
+
+	VOICE_LPE				= (1<<4),
+	NOTE_SUSTAIN			= (1<<5),
 };
 
 /* Envelope stages: */