View Issue Details

IDProjectCategoryView StatusLast Update
0006346The Dark ModSound Systempublic20.04.2024 13:24
Reporterstgatilov Assigned Tostgatilov  
PrioritynormalSeveritynormalReproducibilityN/A
Status assignedResolutionopen 
Product VersionTDM 2.11 
Target VersionTDM 2.13 
Summary0006346: Occasional volume = 0 in sound emitter parameters works unexpectedly
DescriptionSound parameters (most importantly volume) are set in sound shader and in sound emitter.
Sound shader settings are set in .sndshd file and cannot be modified.
Sound emitter settings are usually set in entity spawnargs and are dynamic: they can be changed during gameplay.
The function idSoundEmitterLocal::OverrideParms combines both of these parameters into effective parameters set.
Note: perhaps there is some third parameter set too, I'm not sure here...

Unfortunately, OverrideParms behavior is weird: if emitter parameter is nonzero, then it is used, otherwise the parameter from shader is used.
Given that emitter parameter is often computed in nontrivial way (e.g. for footstep sounds we have to add various modifiers for running, crouching, creeping), we occasionally get emitter volume exactly zero, which drastically changes the efficient volume.

The proper fix would be to record which spawnargs are specified, and only override them --- regardless of whether the overriding parameter is zero or not.
Same applies to dynamic parameters: you can either set parameter value to be overridden (it can even be zero), or you can disable parameter override.
Steps To ReproduceDaft Mugi was caught by this when tweaking footstep volumes:
  https://forums.thedarkmod.com/index.php?/topic/22259-212-footstep-sound-shader-volumes/&do=findComment&comment=489985
In his case, tdm_footstep_tile_run with "volume -12" was efficiently louder than with "volume -8", because the latter case has zero emitter volume.

Also, it seems that guards walking over metal have efficient volume -10 instead of 0 now because of this issue.
Tagssound

Activities

stgatilov

stgatilov

02.12.2023 16:46

administrator   ~0016200

Attached my working patch in order to not lose it.
I suppose I'd wait with it until 2.13.

Also I committed a minor fix in svn rev 16862.
6346_sound_override.patch (14,367 bytes)   
Index: game/ai/AI.cpp
===================================================================
--- game/ai/AI.cpp	(revision 10520)
+++ game/ai/AI.cpp	(working copy)
@@ -6703,7 +6703,7 @@
 		sndShader = declManager->FindSound( sound.c_str() );
 		SetSoundVolume( sndShader->GetParms()->volume + GetMovementVolMod() );
 		StartSoundShader( sndShader, SND_CHANNEL_BODY, 0, false, NULL );
-		SetSoundVolume( 0.0f );
+		SetSoundVolume();
 
 		// propagate the suspicious sound to other AI
 		PropSoundDirect( localSound, true, false, 0.0f, 0 ); // grayman #3355
Index: game/Entity.cpp
===================================================================
--- game/Entity.cpp	(revision 10520)
+++ game/Entity.cpp	(working copy)
@@ -917,11 +917,22 @@
 
 	memset( refSound, 0, sizeof( *refSound ) );
 
-	refSound->parms.minDistance = args->GetFloat( "s_mindistance" );
-	refSound->parms.maxDistance = args->GetFloat( "s_maxdistance" );
-	refSound->parms.volume = args->GetFloat( "s_volume" );
-	refSound->parms.shakes = args->GetFloat( "s_shakes" );
+	auto ReadFloatParam = [&]( const char *spawnargName, int ssomFlag ) -> float {
+		// stgatilov #6346: mark parameter as overriding only when spawnarg is present and nonempty
+		// otherwise, parameter should not override anything (i.e. disabled)
+		const char *text;
+		if ( args->GetString( spawnargName, "", &text ) && text[0] ) {
+			refSound->parms.overrideMode |= ssomFlag;
+			return atof( text );
+		}
+		return 0.0f;
+	};
 
+	refSound->parms.minDistance = ReadFloatParam( "s_mindistance", SSOM_MIN_DISTANCE_OVERRIDE );
+	refSound->parms.maxDistance = ReadFloatParam( "s_maxdistance", SSOM_MAX_DISTANCE_OVERRIDE );
+	refSound->parms.volume = ReadFloatParam( "s_volume", SSOM_VOLUME_OVERRIDE );
+	refSound->parms.shakes = ReadFloatParam( "s_shakes", SSOM_SHAKES_OVERRIDE );
+
 	args->GetVector( "origin", "0 0 0", refSound->origin );
 
 	refSound->referenceSound  = NULL;
@@ -948,6 +959,7 @@
 		refSound->parms.soundShaderFlags |= SSF_UNCLAMPED;
 	}
 	refSound->parms.soundClass = args->GetInt( "s_soundClass" );
+	refSound->parms.overrideMode |= SSOM_FLAGS_OR;
 
 	temp = args->GetString( "s_shader" );
 	if ( temp[0] != '\0' ) {
@@ -3997,7 +4009,12 @@
 */
 void idEntity::SetSoundVolume( float volume ) {
 	refSound.parms.volume = volume;
+	refSound.parms.overrideMode |= SSOM_VOLUME_OVERRIDE;
 }
+void idEntity::SetSoundVolume( void ) {
+	refSound.parms.volume = 0.0f;
+	refSound.parms.overrideMode &= ~SSOM_VOLUME_OVERRIDE;
+}
 
 /*
 ================
Index: game/Entity.h
===================================================================
--- game/Entity.h	(revision 10520)
+++ game/Entity.h	(working copy)
@@ -586,7 +586,8 @@
 	bool					StartSound( const char *soundName, const s_channelType channel, int soundShaderFlags, bool broadcast, int *length, float propVolMod = 0, int msgTag = 0); // grayman #3355
 	bool					StartSoundShader( const idSoundShader *shader, const s_channelType channel, int soundShaderFlags, bool broadcast, int *length);
 	void					StopSound( const s_channelType channel, bool broadcast );	// pass SND_CHANNEL_ANY to stop all sounds
-	void					SetSoundVolume( float volume );
+	void					SetSoundVolume( float volume );	// stgatilov #6346: set and enable override
+	void					SetSoundVolume( void );			// stgatilov #6346: disable override
 	void					UpdateSound( void ); // grayman #4337
 	int						GetListenerId( void ) const;
 	idSoundEmitter *		GetSoundEmitter( void ) const;
Index: game/gamesys/SaveGame.cpp
===================================================================
--- game/gamesys/SaveGame.cpp	(revision 10520)
+++ game/gamesys/SaveGame.cpp	(working copy)
@@ -433,6 +433,7 @@
 	WriteFloat( refSound.parms.shakes );
 	WriteInt( refSound.parms.soundShaderFlags );
 	WriteInt( refSound.parms.soundClass );
+	WriteInt( refSound.parms.overrideMode );
 }
 
 void idSaveGame::WriteRenderView( const renderView_t &view ) {
@@ -1035,6 +1036,7 @@
 	ReadFloat( refSound.parms.shakes );
 	ReadInt( refSound.parms.soundShaderFlags );
 	ReadInt( refSound.parms.soundClass );
+	ReadInt( refSound.parms.overrideMode );
 }
 
 void idRestoreGame::ReadRenderView( renderView_t &view ) {
Index: game/Moveable.cpp
===================================================================
--- game/Moveable.cpp	(revision 10520)
+++ game/Moveable.cpp	(working copy)
@@ -446,7 +446,7 @@
 					PropSoundS( NULL, sndPropName, f, 0 ); // grayman #3355
 				}
 			
-				SetSoundVolume(0.0f);
+				SetSoundVolume();
 
 				nextSoundTime = gameLocal.time + 500;
 			}
Index: game/Player.cpp
===================================================================
--- game/Player.cpp	(revision 10520)
+++ game/Player.cpp	(working copy)
@@ -5381,7 +5381,7 @@
 	const idSoundShader	*sndShader = declManager->FindSound( sound );
 	SetSoundVolume( sndShader->GetParms()->volume + volAdjust);
 	StartSoundShader( sndShader, SND_CHANNEL_ANY, SSF_GLOBAL, false, NULL );
-	SetSoundVolume( 0.0f );
+	SetSoundVolume();
 
 	// propagate the suspicious sound to AI
 	PropSoundDirect( soundName, true, false, volAdjust, 0 );
@@ -11438,7 +11438,7 @@
 		const idSoundShader* sndShader = declManager->FindSound( sound );
 		SetSoundVolume( sndShader->GetParms()->volume + GetMovementVolMod() + crouchVolAdjust); // grayman #3485
 		StartSoundShader( sndShader, SND_CHANNEL_BODY, 0, false, NULL );
-		SetSoundVolume( 0.0f );
+		SetSoundVolume();
 
 		// propagate the suspicious sound to other AI
 		PropSoundDirect( localSound, true, false, crouchVolAdjust, 0 ); // grayman #3355, grayman #3485
Index: sound/snd_emitter.cpp
===================================================================
--- sound/snd_emitter.cpp	(revision 10520)
+++ sound/snd_emitter.cpp	(working copy)
@@ -415,43 +415,95 @@
 	memset( &parms, 0, sizeof( parms ) );
 }
 
+idCVar s_override_parms_mode(
+	"s_override_parms_mode", "0", CVAR_SOUND | CVAR_INTEGER,
+	"Implementation of sound params override:\n"
+	"  0 --- original mode where nonzero value means override\n"
+	"  1 --- new mode where override flag is stored separately\n"
+	"  2 --- compare original and new modes, warn when there is difference",
+	0, 2
+);
+
 /*
 ==================
 idSoundEmitterLocal::OverrideParms
 ==================
 */
-void idSoundEmitterLocal::OverrideParms( const soundShaderParms_t *base, 
-									  const soundShaderParms_t *over, soundShaderParms_t *out ) {
+void idSoundEmitterLocal::OverrideParms( const soundShaderParms_t *base, const soundShaderParms_t *over, soundShaderParms_t *out, const char *comment ) {
 	if ( !over ) {
+		// NULL override: nothing to do
 		*out = *base;
 		return;
 	}
-	if ( over->minDistance ) {
-		out->minDistance = over->minDistance;
-	} else {
-		out->minDistance = base->minDistance;
+
+	soundShaderParms_t resultOld, resultNew;
+
+	{
+		// original Doom 3 mode: override if overriding parameter is nonzero
+		resultOld = *base;
+		if ( over->minDistance ) {
+			resultOld.minDistance = over->minDistance;
+		}
+		if ( over->maxDistance ) {
+			resultOld.maxDistance = over->maxDistance;
+		}
+		if ( over->shakes ) {
+			resultOld.shakes = over->shakes;
+		}
+		if ( over->volume ) {
+			resultOld.volume = over->volume;
+		}
+		if ( over->soundClass ) {
+			resultOld.soundClass = over->soundClass;
+		}
+		// always OR flags
+		resultOld.soundShaderFlags |= over->soundShaderFlags;
 	}
-	if ( over->maxDistance ) {
-		out->maxDistance = over->maxDistance;
-	} else {
-		out->maxDistance = base->maxDistance;
+	{
+		// new mode: override depending on proper flags
+		resultNew = *base;
+		if ( over->overrideMode & SSOM_MIN_DISTANCE_OVERRIDE ) {
+			resultNew.minDistance = over->minDistance;
+		}
+		if ( over->overrideMode & SSOM_MAX_DISTANCE_OVERRIDE ) {
+			resultNew.maxDistance = over->maxDistance;
+		}
+		if ( over->overrideMode & SSOM_SHAKES_OVERRIDE ) {
+			resultNew.shakes = over->shakes;
+		}
+		if ( over->overrideMode & SSOM_VOLUME_OVERRIDE ) {
+			resultNew.volume = over->volume;
+		}
+		if ( over->overrideMode & SSOM_SOUND_CLASS_OVERRIDE ) {
+			resultNew.soundClass = over->soundClass;
+		}
+
+		if ( over->overrideMode & SSOM_FLAGS_OR ) {
+			resultNew.soundShaderFlags |= over->soundShaderFlags;
+		} else if ( over->overrideMode & SSOM_FLAGS_OVERRIDE ) {
+			resultNew.soundShaderFlags = over->soundShaderFlags;
+		}
 	}
-	if ( over->shakes ) {
-		out->shakes = over->shakes;
-	} else {
-		out->shakes = base->shakes;
+
+	if ( s_override_parms_mode.GetInteger() == 2 ) {
+		if ( memcmp( &resultOld, &resultNew, sizeof(resultOld) ) != 0 ) {
+			common->Warning( "OverrideParms: different result for '%s'", comment );
+			#define PRINTDIFF( member ) \
+				if ( resultOld.member != resultNew.member ) \
+					common->Printf("  %s: %s -> %s\n", #member, std::to_string(resultOld.member).c_str(), std::to_string(resultNew.member).c_str() );
+			PRINTDIFF( volume );
+			PRINTDIFF( minDistance );
+			PRINTDIFF( maxDistance );
+			PRINTDIFF( shakes );
+			PRINTDIFF( soundShaderFlags );
+			PRINTDIFF( soundClass );
+		}
 	}
-	if ( over->volume ) {
-		out->volume = over->volume;
+	if ( s_override_parms_mode.GetInteger() == 0 ) {
+		*out = resultOld;
 	} else {
-		out->volume = base->volume;
+		*out = resultNew;
 	}
-	if ( over->soundClass ) {
-		out->soundClass = over->soundClass;
-	} else {
-		out->soundClass = base->soundClass;
-	}
-	out->soundShaderFlags = base->soundShaderFlags | over->soundShaderFlags;
 }
 
 /*
@@ -745,6 +797,7 @@
 		soundWorld->writeDemo->WriteFloat( parms->shakes );
 		soundWorld->writeDemo->WriteInt( parms->soundShaderFlags );
 		soundWorld->writeDemo->WriteInt( parms->soundClass );
+		soundWorld->writeDemo->WriteInt( parms->overrideMode );
 	}
 
 	this->origin = origin;
@@ -849,7 +902,7 @@
 	soundShaderParms_t	chanParms;
 
 	chanParms = shader->parms;
-	OverrideParms( &chanParms, &this->parms, &chanParms );
+	OverrideParms( &chanParms, &this->parms, &chanParms, shader->GetName() );
 	chanParms.soundShaderFlags |= soundShaderFlags;
 
 	if ( chanParms.shakes > 0.0f ) {
@@ -1130,6 +1183,7 @@
 		soundWorld->writeDemo->WriteFloat( parms->shakes );
 		soundWorld->writeDemo->WriteInt( parms->soundShaderFlags );
 		soundWorld->writeDemo->WriteInt( parms->soundClass );
+		soundWorld->writeDemo->WriteInt( parms->overrideMode );
 	}
 
 	for ( int i = 0; i < SOUND_MAX_CHANNELS; i++ ) {
@@ -1142,7 +1196,7 @@
 			continue;
 		}
 
-		OverrideParms( &chan->parms, parms, &chan->parms );
+		OverrideParms( &chan->parms, parms, &chan->parms, (chan->soundShader ? chan->soundShader->GetName() : "???") );
 
 		if ( chan->parms.shakes > 0.0f && chan->soundShader != NULL ) {
 			chan->soundShader->CheckShakesAndOgg();
Index: sound/snd_local.h
===================================================================
--- sound/snd_local.h	(revision 10520)
+++ sound/snd_local.h	(working copy)
@@ -450,7 +450,7 @@
 
 	void				Clear( void );
 
-	void				OverrideParms( const soundShaderParms_t *base, const soundShaderParms_t *over, soundShaderParms_t *out );
+	void				OverrideParms( const soundShaderParms_t *base, const soundShaderParms_t *over, soundShaderParms_t *out, const char *comment );
 	void				CheckForCompletion( int current44kHzTime );
 	void				Spatialize( bool primary, idVec3 listenerPos, int listenerArea, idRenderWorld *rw ); // grayman #4882
 
Index: sound/snd_shader.cpp
===================================================================
--- sound/snd_shader.cpp	(revision 10520)
+++ sound/snd_shader.cpp	(working copy)
@@ -158,6 +158,7 @@
 	//this will results in missing sounds later, when soundCache is finally available
 	assert( soundSystemLocal.soundCache || soundSystemLocal.s_noSound.GetBool() );
 
+	memset( &parms, 0, sizeof(parms) );
 	parms.minDistance = 1;
 	parms.maxDistance = 10;
 	parms.volume = 1;
Index: sound/snd_world.cpp
===================================================================
--- sound/snd_world.cpp	(revision 10520)
+++ sound/snd_world.cpp	(working copy)
@@ -374,6 +374,7 @@
 			readDemo->ReadFloat( parms.shakes );
 			readDemo->ReadInt( parms.soundShaderFlags );
 			readDemo->ReadInt( parms.soundClass );
+			readDemo->ReadInt( parms.overrideMode );
 			EmitterForIndex( index )->UpdateEmitter( origin, listenerId, &parms );
 		}
 		break;
@@ -405,6 +406,7 @@
 			readDemo->ReadFloat( parms.shakes );
 			readDemo->ReadInt( parms.soundShaderFlags );
 			readDemo->ReadInt( parms.soundClass );
+			readDemo->ReadInt( parms.overrideMode );
 			EmitterForIndex( index )->ModifySound( (s_channelType)channel, &parms );
 		}
 		break;
@@ -1511,6 +1513,7 @@
 	saveGame->WriteFloat(params->shakes);
 	saveGame->WriteInt(params->soundShaderFlags);
 	saveGame->WriteInt(params->soundClass);
+	saveGame->WriteInt(params->overrideMode);
 }
 
 /*
@@ -1681,6 +1684,7 @@
 	saveGame->ReadFloat(params->shakes);
 	saveGame->ReadInt(params->soundShaderFlags);
 	saveGame->ReadInt(params->soundClass);
+	saveGame->ReadInt(params->overrideMode);
 }
 
 /*
Index: sound/sound.h
===================================================================
--- sound/sound.h	(revision 10520)
+++ sound/sound.h	(working copy)
@@ -45,6 +45,15 @@
 static const int	SSF_NO_DUPS =			BIT(9);	// try not to play the same sound twice in a row
 static const int	SSF_NO_EFX =			BIT(10);// do not apply EFX effect to the sound
 
+// stgatilov #6346: override mode for emitter/channel params
+static const int	SSOM_MIN_DISTANCE_OVERRIDE =	BIT(0);	// override corresponding parameter?
+static const int	SSOM_MAX_DISTANCE_OVERRIDE =	BIT(1);	// (if flag is missing, then new parameter is ignored)
+static const int	SSOM_VOLUME_OVERRIDE =			BIT(2);	// ...
+static const int	SSOM_SHAKES_OVERRIDE =			BIT(3);	// ...
+static const int	SSOM_SOUND_CLASS_OVERRIDE =		BIT(4);	// ...
+static const int	SSOM_FLAGS_OVERRIDE =			BIT(5);	// override soundShaderFlags with new ones?
+static const int	SSOM_FLAGS_OR =					BIT(6); // OR base and new parameters together?
+
 //stgatilov #2454: verbosity level of subtitles
 enum SubtitleLevel {
 	SUBL_IGNORE		= 0,		// auxilliary value for tdm_subtitles: don't show any subtitles regardless of level
@@ -62,6 +71,7 @@
 	float					shakes;
 	int						soundShaderFlags;		// SSF_* bit flags
 	int						soundClass;				// for global fading of sounds
+	int						overrideMode;			// stgatilov #6346: SSOM_* flags define how to override with these params
 } soundShaderParms_t;
 
 
6346_sound_override.patch (14,367 bytes)   
stgatilov

stgatilov

30.03.2024 19:37

administrator   ~0016598

Here is the announcement:
  https://forums.thedarkmod.com/index.php?/topic/22412-213-sound-parameter-0-overrides/

In order to save compatibility, I'm going to replace spawnargs like this:
  "s_mindistance" "0.0"
with this:
  "s_mindistance" ""
Zero value will have different meaning after this engine change, while empry value means that same and its meaning will not change.

I'll have to apply this script to all the missions and hope for the best:
  r17015 Added Python script to replace sound spawnargs with zero value with empty value.

Another small fix (obtained by running the script on "def" directory):
  r17013 Deleted meaningless "s_volume" "0" from atdm:secret_messages.
stgatilov

stgatilov

11.04.2024 21:05

administrator   ~0016636

Committed the change:
  r10703 Added a cvar under which zero value of sound parameter is not ignored as a special case.
  r10705 Renamed cvar s_overrideParmsMode.

It is not yet enabled.
It can be enabled by setting cvar s_overrideParmsMode.

Value 0 (current default) means using old TDM & Doom 3 behavior.
Value 1 (future) means using new mode where it is possible to override parameter with zero and zero is not a special value.
Value 2 (debugging) compares both modes with each other and prints warning when they don't match.
stgatilov

stgatilov

20.04.2024 13:24

administrator   ~0016657

Enabled new behavior by default:
  r10718 Set s_overrideParmsMode to 1 (new behavior).

Issue History

Date Modified Username Field Change
02.12.2023 14:43 stgatilov New Issue
02.12.2023 14:43 stgatilov Status new => assigned
02.12.2023 14:43 stgatilov Assigned To => stgatilov
02.12.2023 16:40 stgatilov Target Version TDM 2.12 => TDM 2.13
02.12.2023 16:46 stgatilov Note Added: 0016200
02.12.2023 16:46 stgatilov File Added: 6346_sound_override.patch
30.03.2024 19:37 stgatilov Note Added: 0016598
01.04.2024 22:30 Fiver Tag Attached: sound
11.04.2024 21:05 stgatilov Note Added: 0016636
20.04.2024 13:24 stgatilov Note Added: 0016657