Volume SFX Control

[yeshua]

So I'm helping improve the JGE engine that we either broke or didn't work to begin with. Either way I'm trying to add volume control to SFX.

if I set volume straight on here without calling anything or using vars it works...
But we cant do that...

Code: Select all

 
JMP3::init();
  pspAudioInit();
  JMP3 * sfx = JMP3::mInstance;
	if (sfx) sfx->pspSetSfxVol();
	pspAudioSetChannelCallback(0, audioOutCallback_0, NULL);
	pspAudioSetChannelCallback(1, audioOutCallback_1, NULL);
	pspAudioSetChannelCallback(2, audioOutCallback_2, NULL);
}

Also pspSetSfxVol() does this

Code: Select all

void JMP3::pspSetSfxVol() {
  int i = 0;
  for&#40;i = 0;i<3;i++&#41;
    pspAudioSetVolume&#40;i, m_sfxvolume, m_sfxvolume&#41;;
&#125;

and m_sfxvolume is set by Jmp3 when you call

Code: Select all

void JSoundSystem::SetSfxVolume(int volume)
{
  JMP3 * sfx = JMP3::mInstance;
	if (sfx) sfx->setSfxVolume(volume);
}

and all setSfxVolume(volume) does is:

Code: Select all

int JMP3::setSfxVolume(int volume) {
   return (m_sfxvolume = volume);
}

According to the sample in the sdk

Code: Select all

pspAudioInit();
	pspAudioSetVolume(0, 0x4000, 0x4000);
	pspAudioSetVolume(1, 0x4000, 0x4000);
	pspAudioSetChannelCallback(0, audioOutCallback0, NULL);
	pspAudioSetChannelCallback(1, audioOutCallback1, NULL);

This should work...

[yeshua]

I tried removing it from the class and using an external variable didnt work.

[Raphael]

So I'm helping improve the JGE engine that we either broke or didn't work to begin with. Either way I'm trying to add volume control to SFX.

if I set volume straight on here without calling anything or using vars it works...
But we cant do that...

Code: Select all

 
JMP3::init();
  pspAudioInit();
  JMP3 * sfx = JMP3::mInstance;
	if (sfx) sfx->pspSetSfxVol();
	pspAudioSetChannelCallback(0, audioOutCallback_0, NULL);
	pspAudioSetChannelCallback(1, audioOutCallback_1, NULL);
	pspAudioSetChannelCallback(2, audioOutCallback_2, NULL);
}

Also pspSetSfxVol() does this

Code: Select all

void JMP3::pspSetSfxVol() {
  int i = 0;
  for&#40;i = 0;i<3;i++&#41;
    pspAudioSetVolume&#40;i, m_sfxvolume, m_sfxvolume&#41;;
&#125;

and m_sfxvolume is set by Jmp3 when you call

Code: Select all

void JSoundSystem::SetSfxVolume(int volume)
{
  JMP3 * sfx = JMP3::mInstance;
	if (sfx) sfx->setSfxVolume(volume);
}

and all setSfxVolume(volume) does is:

Code: Select all

int JMP3::setSfxVolume(int volume) {
   return (m_sfxvolume = volume);
}

According to the sample in the sdk

Code: Select all

pspAudioInit();
	pspAudioSetVolume(0, 0x4000, 0x4000);
	pspAudioSetVolume(1, 0x4000, 0x4000);
	pspAudioSetChannelCallback(0, audioOutCallback0, NULL);
	pspAudioSetChannelCallback(1, audioOutCallback1, NULL);

This should work...

Well... the problem is... you (or maybe the creator of jge) seem to misunderstand what pspAudioSetVolume does/how it handles the paramters given or possibly even: How variables really work.
Calling pspAudioSetVolume(i, m_sfxvolume, m_sfxvolume); once on startup and then changing the value of m_sfxvolume during runtime won't change the volume then (since you give the variables by-value rather then by-reference). What you should do, to change the sound volume at runtime via JSoundSystem::SetSfxVolume, is call JMP3::setSfxVolume and afterwards call JMP3::pspSetSfxVol().

IMO this is a pretty weak design for multiple reasons, one being that you have two different functions with similar names that *should* do only one thing, yet have both functions have to be called in turn to achieve the result.

[yeshua]

Thanks it works

[willow :--)]

Hey Yeshua, here's a quick review seeing the chunks of code here. Your design doesn't look good to me and I believe there has been a misunderstanding when I told you what I think should be done.

Goal: we want independent control of the volume of SFX and Music.

The Jmp3 class in JGE is made to handle music, not sfx.
For sfx we use JAudio.cpp.

Therefore you shouldn't have a function "setSfxVolume" in JMp3, it makes no sense.

you should have one method SetVolume in Jmp3 (it already exists) and a method with the same name in JAudio,

JSoundSystem should be the one having one "SetMusicVolume" and one "SetSfXVolume" methods.

It should look like:

Code: Select all

JSoundSystem::setMusicVolume(int volume){
    myJmp3->setvolume(volume);
}

JSoundSystem::setSfxVolume(int volume){
    myJAudio->setvolume(volume);
}

Ok, I know JGE is not always clean, and unfortunately, the name of the JSoundSystem file is... JSfx.cpp, which doesn't help...but that's where the functions setMusicVolume and setSfxVolume should be, not anywhere else...
Feel free to rename JSfx.cpp to JSoundSystem.cpp.


Also the method names in JMp3 should not start with "psp", since JMp3 is used only by the PSP anyways.