Having trouble setting an AudioChannel volume

Started by bx83, Tue 16/06/2020 13:52:09

Previous topic - Next topic

bx83

I recently made a function which is just like AudioChannel.SetRoomLocation(), but allow you to choose the area in which the sound exists (as opposed to the entire screen, which would only see the volumes go to 0 if you're at the edge of the screen).

My code:

Code: ags
function soundPlayLocation (int x, int y, float r, AudioClip* soundSample, int residual)
{
	float d=0.0;
	int 	p=0;
	int 	sp=0;
	
	if (soundPlayer1==null) {
		sp=1;
		soundPlayer1=soundSample.Play(eAudioPriorityNormal, eRepeat);
	} else {
		if (soundPlayer2==null) {
			sp=2;
			soundPlayer2=soundSample.Play(eAudioPriorityNormal, eRepeat);
		} else {
			if (soundPlayer3==null) {
				sp=3;
				soundPlayer3=soundSample.Play(eAudioPriorityNormal, eRepeat);
			} else {
				Display("more than 3 sound samples.");
				return -1;
			}
		}
	}
	
	float dx = IntToFloat(abs(player.x - x));
	float dy = IntToFloat(abs(player.y - y));
		
	d=Maths.Sqrt((dx * dx) + (dy * dy));
	
	if (d>r) {
		switch (sp) {
			case 1: soundPlayer1.Volume=residual;
			case 2: soundPlayer2.Volume=residual;
			case 3: soundPlayer3.Volume=residual;
		}
	} else {
		p=FloatToInt( 100.0-((d/r)*100.0) );
		if (p<residual) p=residual;
		switch(sp) {
			case 1: soundPlayer1.Volume = p;
			case 2: soundPlayer2.Volume = p;
			case 3: soundPlayer3.Volume = p;
		}
	}	
}


Typical calling code:
Code: ags
soundPlayLocation(415, 610, 50.0, aFlys2, 10);


This focuses on a circle, who's center point is x,y, with a radius of r, and a residual sound so that it will always play no lower than 'residual's volume, even when you're outside the circle.
It works fine; but when it comes to playing more than 1 sample, it doesn't work.

I thought I would make a version with 3 global AudioChannel* variables, and find the newest one and assign soundSample to it, allowing for 3 positions in the room the play from.

However as soon as I load the game, I get this error:
Code: ags
		switch (sp) {
			case 1: soundPlayer1.Volume=residual;  <--THIS LINE - 'null pointer referenced'
			case 2: soundPlayer2.Volume=residual;
			case 3: soundPlayer3.Volume=residual;
		}

...despite the fact that it first loads soundSample into soundPlayer1 at beginning:
Code: ags
if (soundPlayer1==null) {
		sp=1;
		//if (soundPlayer1.PlayingClip!=soundSample)
		soundPlayer1=soundSample.Play(eAudioPriorityNormal, eRepeat);
...


What have I done wrong?

Snarky

#1
I assume that soundPlayer1/2/3 are defined in global scope so that you can adjust them in repexec? But you don't store the x,y coordinates, so how is that going to work?

There are several weird things here, but maybe most critically, soundSample.Play() may return null, if it doesn't succeed in playing the sample.

I would also eliminate the sp variable and switch/case logic, and simply keep a pointer to the actual AudioChannel you're interested in. (And since this volume control logic will be the same for the repexec update as for the initial control, factor that out into its own function.)

Edit: One more thing. To post AGS code, you can and should use the [code=ags][/code] tag.

bx83

SoundPlayer1/2/3 are defined in global scope.
Why would I store the x,y coordinates?
Hopefully soundSample.Play() will not throw null because I know all the sound samples exist. Or did you mean something else?
So keep a global int pointer to soundPlayer1/2/3? Might be simpler to just have 3 separate functions?..
“Factor that out into its own function” - not sure what you mean by this.

Crimson Wizard

#3
First obvious mistake is that you do not have "break;" after each "case:". So in case of "1" it will try to set volume for all the channel pointers.

Quote from: bx83 on Tue 16/06/2020 14:50:32
Hopefully soundSample.Play() will not throw null because I know all the sound samples exist. Or did you mean something else?

Play can throw null if there's no free channel for the sound to play on, or if user disabled sound in setup.

Snarky

#4
Quote from: bx83 on Tue 16/06/2020 14:50:32
Why would I store the x,y coordinates?

Isn't the whole point of this that the volume will change as the player walks around, depending on how far away they are from each sound source?

That's something you would have to update in repeatedly_execute(), by comparing the player's current position to the position of each sound source (just like you do in this function â€" hence: factor it out). So you would need to store the position of each sound source. Right?

Quote from: bx83 on Tue 16/06/2020 14:50:32
Hopefully soundSample.Play() will not throw null because I know all the sound samples exist. Or did you mean something else?

The main reasons it might return null is that you've run out of AudioChannels for this sound type (and you're setting a lower priority than any of the currently playing tracks), or if sound is disabled altogether.

QuoteSo keep a global int pointer to soundPlayer1/2/3? Might be simpler to just have 3 separate functions?..

I would do this rather differently. I'd start by defining a data type for this kind of audio source, and the logic to update the volume:

Code: ags
// Header

struct SoundSource
{
  AudioChannel* channel;
  int x;
  int y;
  int radius;
  int minVolume;

  import bool Init(AudioClip* clip, AudioPriority pri, int x, int y, int radius, int minVolume);
  import void UpdateVolume(int x, int y);
  import void Stop();
};

// Script
void SoundSource::UpdateVolume(int x, int y)
{
  if(this.channel != null)
  {
    int dx = this.x - x;
    int dy = this.y - y;
    int d_squared = dx*dx + dy*dy;

    if(d_squared >= radius*radius)
      this.channel.Volume = minVolume;
    else
      this.channel.Volume = FloatToInt(100.0 - (Maths.Sqrt(IntToFloat(d_squared))/IntToFloat(radius)) * (100.0 - IntToFloat(minVolume)));
  }
}

bool SoundSource::Init(AudioClip* clip, AudioPriority pri, int x, int y, int radius, int minVolume)
{
  if(this.channel != null)
    this.channel.Stop();

  this.x = x;
  thix.y = y;
  this.radius = radius;
  this.minVolume = minVolume;

  this.channel = clip.Play(pri, eRepeat);
  return this.channel != null;
}

void SoundSource::Stop()
{
  if(this.channel != null)
   this.channel.Stop();

  this.channel = null;
}


Then for rooms with sources like this, I would create an array of however many sources you want, and just update them continuously:

Code: ags
#define SOUND_SOURCE_COUNT 3

SoundSource soundSources[SOUND_SOURCE_COUNT];

function romLoadOrWhatever()
{
  soundSources[0].Init(aFlys2, eAudioPriorityNormal, 415, 610, 50, 10); // Centered at (415,610), radius 50, minVolume 10
  soundSources[1].Init(aSomethingElse, eAudioPriorityNormal, 230, 410, 100, 10); // Centered at (230,410), radius 100, minVolume 10
  soundSources[2].Init(aNotherSound, eAudioPriorityNormal, 600, 150, 25, 10); // Centered at (600,150), radius 25, minVolume 10
}

function late_repeatedly_execute_always()
{
  for(int i=0; i<SOUND_SOURCE_COUNT; i++)
    soundSources[i].UpdateVolume(player.x, player.y);
}


Here we don't bother setting the volume on Init(), we rely on the UpdateVolume() call in late_repeatedly_execute_always(). In case of problems, just add an UpdateVolume() call right after each Init().

You also need to make sure to kill the audio when the player leaves the room. You can call this function:

Code: ags
void StopSoundSources()
{
  for(int i=0; i<SOUND_SOURCE_COUNT; i++)
    soundSources[i].Stop();
}

bx83

For late_repeatedly_execute_always(), is this in global script, or it can also be used for room?
RomLoadOrWhatever() represents roomLoad() or repex() or some init function in the room?
Man that blows the shit out of my tiny 4 hours of coding :p

Snarky

Quote from: bx83 on Tue 16/06/2020 16:32:46
For late_repeatedly_execute_always(), is this in global script, or it can also be used for room?

You can add it in the room script as well (and I would).

Quote from: bx83 on Tue 16/06/2020 16:32:46
RomLoadOrWhatever() represents roomLoad() or repex() or some init function in the room?

Not RepExec(), but roomLoad() or some init function, yes. You probably want to trigger it in room load, but you can also trigger it on any other event (e.g. if the character turns on a faucet).

Quote from: bx83 on Tue 16/06/2020 16:32:46
Man that blows the shit out of my tiny 4 hours of coding :p

Glad I could help.


bx83

Okay, think I've followed your instructions, but there's an error.

In GlobalScript.asc - error on line 9:
Code: ags
void SoundSource::UpdateVolume(int x, int y)
{
	if(this.channel != null)
	{
		int dx = this.x - x;
		int dy = this.y - y;
		int d_squared = dx*dx + dy*dy;
 
		if(d_squared >= radius*radius)                  <===== Undefined symbol 'radius'
			this.channel.Volume = minVolume;
		else
			this.channel.Volume = FloatToInt(100.0 - (Maths.Sqrt(IntToFloat(d_squared))/IntToFloat(radius)) * (100.0 - IntToFloat(minVolume)));
	}
}
 
bool SoundSource::Init(AudioClip* clip, AudioPriority pri, int x, int y, int radius, int minVolume)
{
	if(this.channel != null)
		this.channel.Stop();
 
	this.x = x;
	thix.y = y;
	this.radius = radius;
	this.minVolume = minVolume;
 
	this.channel = clip.Play(pri, eRepeat);
	return this.channel != null;
}
 
void SoundSource::Stop()
{
	if(this.channel != null)
	 this.channel.Stop();
 
	this.channel = null;
}
		
void StopSoundSources()
{
	for(int i=0; i<SOUND_SOURCE_COUNT; i++)
		soundSources[i].Stop();
}


In GlobalScript.ash:
Code: ags
struct SoundSource
{
	AudioChannel* channel;
	int x;
	int y;
	int radius;
	int minVolume;
 
	import bool Init(AudioClip* clip, AudioPriority pri, int x, int y, int radius, int minVolume);
	import void UpdateVolume(int x, int y);
	import void Stop();
};

#define SOUND_SOURCE_COUNT 3
SoundSource soundSources[SOUND_SOURCE_COUNT];

import function StopSoundSources();


In room code:

Code: ags
function room_Load()
{
  soundSources[0].Init(aFlys2, eAudioPriorityNormal, 415, 610, 50, 2);
  soundSources[1].Init(aMeatMarket_tinnyradio, eAudioPriorityNormal, 1150, 365, 80, 40);
}

function late_repeatedly_execute_always()
{
  for(int i=0; i<SOUND_SOURCE_COUNT; i++)
    soundSources[i].UpdateVolume(player.x, player.y);
}


What to do?

Crimson Wizard

radius should be this.radius, because it's struct's member.

bx83

globalscript.ash:
import bool Init(AudioClip* clip, AudioPriority pri, int x, int y, int radius, int minVolume=0);

globalscript.ash:
void SoundSource::UpdateVolume(int x, int y)
{
   if(this.channel != null)
   {
      int dx = this.x - x;
      int dy = this.y - y;
      int d_squared = dx*dx + dy*dy;

      if(d_squared >= (this.radius * this.radius))
         this.channel.Volume = this.minVolume;
      else
         this.channel.Volume = FloatToInt(100.0 - (Maths.Sqrt(IntToFloat(d_squared))/IntToFloat(this.radius)) * (100.0 - IntToFloat(this.minVolume)));
   }
}

bool SoundSource::Init(AudioClip* clip, AudioPriority pri, int x, int y, int radius, int minVolume)
{
   if(this.channel != null)
      this.channel.Stop();

   this.x = x;
   this.y = y;
   this.radius = radius;
   this.minVolume = minVolume;

   this.channel = clip.Play(pri, eRepeat);
   return this.channel != null;
}

Some errors, now fixed and working beautifully :)

Snarky

Yeah, I forgot to put the this. before some of the variable references.

I would think very carefully about this bit in GlobalScript.ash:

Code: ags
#define SOUND_SOURCE_COUNT 3
SoundSource soundSources[SOUND_SOURCE_COUNT];


Declaring a variable (in this case, an array) in a header is usually considered an error.

Putting this in a header means it gets copied to every room script (as well as the global script). So now every room has an array like this, with the same name, but they are different arrays.  This can lead to unexpected behavior.

Now, in this case that unexpected behavior might actually be what you want, so I won't definitely say it's wrong. But it does mean that, for example, if you leave a room without first stopping the audio, and then try to call StopSoundSources() from the next room, nothing will happen and the sounds from the former room will continue to play (no longer updating volume) until you run out of AudioChannels.

If this kind of effect is only used in a few rooms, I would instead suggest defining the arrays (and duplicating StopSoundSources()) separately in each room. That's how I wrote it. But if you use it a lot, I would suggest moving it into a separate module (in general, I don't think utility code like this should go in GlobalScript) and have the array in that script, along with some additional logic to automatically wipe the array each time you switch rooms.

bx83

So put #define SOUND_SOURCE_COUNT 3 in a rooms room_Load()?

I usually run StopSoundSources() before I leave the room, but you're right -- this makes no difference to the sounds, so I think I'll put it in the room code.

How... exactly do I do this? :P I've put both lines in room code, then I get an 'undefined token' (line 4) in this code (also in the room):
Code: ags
function late_repeatedly_execute_always()
{
  for(int i=0; i<SOUND_SOURCE_COUNT; i++)
    soundSources[i].UpdateVolume(player.x, player.y);
}


Snarky

#13
You'd put the #define SOUND_SOURCE_COUNT 3 at the top of the room script.

You also need to declare the soundSources[] array in the room script; that's presumably what the "undefined token" refers to. This is meant to be what your room script might look like:

Code: ags
#define SOUND_SOURCE_COUNT 3
SoundSource soundSources[SOUND_SOURCE_COUNT];

// Convenience function to make sure we turn off sounds before leaving the room (replaces StopSoundSources())
void ChangePlayerRoom(int room, int x, int y)
{
  for(int i=0; i<SOUND_SOURCE_COUNT; i++)
    soundSources[i].Stop();
  player.ChangeRoom(room, x, y);
}

function room_Load()
{
  soundSources[0].Init(aFlys2, eAudioPriorityNormal, 415, 610, 50, 10); // Centered at (415,610), radius 50, minVolume 10
  soundSources[1].Init(aSomethingElse, eAudioPriorityNormal, 230, 410, 100, 10); // Centered at (230,410), radius 100, minVolume 10
  soundSources[2].Init(aNotherSound, eAudioPriorityNormal, 600, 150, 25, 10); // Centered at (600,150), radius 25, minVolume 10
}

function room_LeaveLeft()
{
  ChangePlayerRoom(3, 460, 540);
}

function late_repeatedly_execute_always()
{
  for(int i=0; i<SOUND_SOURCE_COUNT; i++)
    soundSources[i].UpdateVolume(player.x, player.y);
}

bx83

#14
Okay, works now :)

I've changed:

Code: ags
#define SOUND_SOURCE_COUNT 3
SoundSource soundSources[SOUND_SOURCE_COUNT];
void StopSoundSources()
{
	for(int i=0; i<SOUND_SOURCE_COUNT; i++)
		soundSources[i].Stop();
}


...to be at the top of the room script.

This definition code is commented out in globalscript.ash:
Code: ags
import void StopSoundSources();


I didn't bother implementing the ChangePlayerRoom() because this will only be in like 5 rooms.

bx83

#15
Another problem; soundSource audio plays, for a second, at 100% volume, when re-entering a room.

video:
https://bluekeystudios.com/filez/soundsourceprob.mov

This also happens in the second room I made.

Snarky

Yeah, I wondered whether that might happen. In that case you need to call UpdateVolume() right after SoundSource.Init().

Maybe it's most convenient to add it as an optional parameter to Init(), like this:

Code: ags
// Header, inside SoundSource struct:
  import bool Init(AudioClip* clip, AudioPriority pri, int x, int y, int radius, int minVolume=0, Character* trackingChar=0)

// Script. Note new "trackingChar" argument
bool SoundSource::Init(AudioClip* clip, AudioPriority pri, int x, int y, int radius, int minVolume, Character* trackingChar)
{
  if(this.channel != null)
    this.channel.Stop();

  this.x = x;
  this.y = y;
  this.radius = radius;
  this.minVolume = minVolume;

  this.channel = clip.Play(pri, eRepeat);
  if(trackingChar != null)                            // <--- Note this line and next!
    this.UpdateVolume(trackingChar.x, trackingChar.y);

  return this.channel != null;
}


Now you can call:

Code: ags
  soundSources[0].Init(aFlys2, eAudioPriorityNormal, 415, 610, 50, 0, player); // Centered at (415,610), radius 50, minVolume 0, set initial volume based on player position


And it should ensure that the volume is set correctly right from the start.

bx83


bx83

I have a fresh hell. I get clicks and pops when I play a sound, as it's ramping up in volume as I approach the epicenter of the sound source. What's going on? Code:

Code: ags
#define SOUND_SOURCE_COUNT 3
SoundSource soundSources[SOUND_SOURCE_COUNT];
void StopSoundSources()
{
	for(int i=0; i<SOUND_SOURCE_COUNT; i++)
		soundSources[i].Stop();
}

function room_Load()
{
	soundSources[0].Init(aWubwubwub, eAudioPriorityNormal, 430, 420, 150, 3, player);
}

function late_repeatedly_execute_always()
{
  for(int i=0; i<SOUND_SOURCE_COUNT; i++)
    soundSources[i].UpdateVolume(player.x, player.y);
}

Snarky

Hmm, if you're getting clicks and pops when changing the volume of an AudioChannel, that sounds like an engine problem. I'd maybe try with a different sound file to see if it's tied to something in the audio encoding, but otherwise I don't know what you could do.

SMF spam blocked by CleanTalk