New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for enhanced sound. #151

Merged
merged 3 commits into from Jun 2, 2015

Conversation

Projects
None yet
5 participants
@jg18
Contributor

jg18 commented May 30, 2015

Add support for enhanced in-mission sound, with 128 audio channels and a
new system for specifying per-sound priorities and concurrent playback
limits using modular extensions of soudns.tbl.

Add support for enhanced sound.
Add support for enhanced in-mission sound, with 128 audio channels and a
new system for specifying per-sound priorities and concurrent playback
limits using modular extensions of soudns.tbl.
return first_free_channel;
}
/**

This comment has been minimized.

@Echelon9

Echelon9 May 30, 2015

Contributor

What function does this comment block relate to? Can it be deleted?

@Echelon9

Echelon9 May 30, 2015

Contributor

What function does this comment block relate to? Can it be deleted?

This comment has been minimized.

@jg18

jg18 Jun 1, 2015

Contributor

I'm not sure what you're referring to. The fragment looks to be the end of get_free_channel_enhanced(), which attempts to trash the "least important" sound, generates the OpenAL source if needed, and returns the channel found (-1 on failure).

@jg18

jg18 Jun 1, 2015

Contributor

I'm not sure what you're referring to. The fragment looks to be the end of get_free_channel_enhanced(), which attempts to trash the "least important" sound, generates the OpenAL source if needed, and returns the channel found (-1 on failure).

This comment has been minimized.

@Echelon9

Echelon9 Jun 1, 2015

Contributor

Apologies, I should have commented on the end, not start, of the comment block so these inline boxes were clearer.
I'm referring to the comment block that starts with "Generic function for getting a free channel".

It appears a pre-Doxygen layout, yet there is no function below it before the next Doxygen comment block which relates to ds_get_free_channel(). If Doxygen-ified, can the older function description be removed as redundant?

@Echelon9

Echelon9 Jun 1, 2015

Contributor

Apologies, I should have commented on the end, not start, of the comment block so these inline boxes were clearer.
I'm referring to the comment block that starts with "Generic function for getting a free channel".

It appears a pre-Doxygen layout, yet there is no function below it before the next Doxygen comment block which relates to ds_get_free_channel(). If Doxygen-ified, can the older function description be removed as redundant?

Show outdated Hide outdated code/cmdline/cmdline.cpp
@@ -167,6 +167,7 @@ Flag exe_params[] =
{ "-snd_preload", "Preload mission game sounds", true, EASY_MEM_ALL_ON, EASY_DEFAULT_MEM, "Audio", "http://www.hard-light.net/wiki/index.php/Command-Line_Reference#-snd_preload", },
{ "-nosound", "Disable all sound", false, 0, EASY_DEFAULT, "Audio", "http://www.hard-light.net/wiki/index.php/Command-Line_Reference#-nosound", },
{ "-nomusic", "Disable music", false, 0, EASY_DEFAULT, "Audio", "http://www.hard-light.net/wiki/index.php/Command-Line_Reference#-nomusic", },
{ "-enhanced_sound", "Enable enhanced sound", false, 0, EASY_DEFAULT, "Audio", "http://www.hard-light.net/wiki/index.php/Command-Line_Reference#-enhanced_sound", },

This comment has been minimized.

@Echelon9

Echelon9 May 30, 2015

Contributor

Whitespace should be amended, so that the text aligns with the lines above.

@Echelon9

Echelon9 May 30, 2015

Contributor

Whitespace should be amended, so that the text aligns with the lines above.

This comment has been minimized.

@jg18

jg18 Jun 1, 2015

Contributor

Odd, the text looks aligned on this page but not in VS2013. As such, I have no idea how to align it in VS2013. Maybe the number of spaces per tab is wrong? Or there's some other setting in VS2013 that's wrong that's preventing me from seeing the text aligned?

@jg18

jg18 Jun 1, 2015

Contributor

Odd, the text looks aligned on this page but not in VS2013. As such, I have no idea how to align it in VS2013. Maybe the number of spaces per tab is wrong? Or there's some other setting in VS2013 that's wrong that's preventing me from seeing the text aligned?

This comment has been minimized.

@chief1983

chief1983 Jun 1, 2015

Member

Instead of just going by looks, might need to verify whether the other lines are using spaces vs tabs.

@chief1983

chief1983 Jun 1, 2015

Member

Instead of just going by looks, might need to verify whether the other lines are using spaces vs tabs.

This comment has been minimized.

@jg18

jg18 Jun 1, 2015

Contributor

Problem tracked down. I was using a non-monospace font (Arial Black) so the text wasn't aligned in VS2013. Switched to Consolas and now I'm able to fix it.

@jg18

jg18 Jun 1, 2015

Contributor

Problem tracked down. I was using a non-monospace font (Arial Black) so the text wasn't aligned in VS2013. Switched to Consolas and now I'm able to fix it.

This comment has been minimized.

@jg18

jg18 Jun 1, 2015

Contributor

Fixed in commit faa0c5e.

@jg18

jg18 Jun 1, 2015

Contributor

Fixed in commit faa0c5e.

Show outdated Hide outdated code/gamesnd/gamesnd.cpp
EnhancedSoundData( SND_ENHANCED_PRIORITY_MUST_PLAY, 1), // SND_ENERGY_ADJUST = 9, //!< energy level change success
EnhancedSoundData( SND_ENHANCED_PRIORITY_MUST_PLAY, 1), // SND_ENERGY_ADJUST_FAIL = 10, //!< energy level change fail
EnhancedSoundData( SND_ENHANCED_PRIORITY_MUST_PLAY, 1), // SND_ENERGY_TRANS = 11, //!< energy transfer success
EnhancedSoundData( SND_ENHANCED_PRIORITY_MUST_PLAY, 1), // SND_ENERGY_TRANS_FAIL = 12, //!< energy transfer fail

This comment has been minimized.

@Echelon9

Echelon9 May 30, 2015

Contributor

Whitespace should be amended, such that // SND_ENERGY_TRANS_FAIL aligns with the start of comments on the lines above.

@Echelon9

Echelon9 May 30, 2015

Contributor

Whitespace should be amended, such that // SND_ENERGY_TRANS_FAIL aligns with the start of comments on the lines above.

This comment has been minimized.

@jg18

jg18 Jun 1, 2015

Contributor

Fixed in commit faa0c5e.

@jg18

jg18 Jun 1, 2015

Contributor

Fixed in commit faa0c5e.

Show outdated Hide outdated code/sound/ds.cpp
bool ds_check_for_openal_soft()
{
const ALchar * renderer = alGetString(AL_RENDERER);
Assertion(renderer!= NULL, "ds_check_for_openal_soft: renderer is null!");

This comment has been minimized.

@asarium

asarium May 30, 2015

Member

Assertions should be used for programming errors but if the OpenAL implementation returns something invalid here it's a recoverable data error so I think it's better to just print a message to the log.

@asarium

asarium May 30, 2015

Member

Assertions should be used for programming errors but if the OpenAL implementation returns something invalid here it's a recoverable data error so I think it's better to just print a message to the log.

This comment has been minimized.

@jg18

jg18 Jun 1, 2015

Contributor

Fixed in commit faa0c5e.

@jg18

jg18 Jun 1, 2015

Contributor

Fixed in commit faa0c5e.

@asarium

This comment has been minimized.

Show comment
Hide comment
@asarium

asarium May 30, 2015

Member

Currently the new code is only used if a commandline flag is set but that's a bit counterintuitive. I would propose changing it so the enhanced code is always enabled and if OpenALSoft is not detected it would be disabled and a message is written to the debug log. A -no-enhanced-sound option can be added to always disable the new code.

Member

asarium commented May 30, 2015

Currently the new code is only used if a commandline flag is set but that's a bit counterintuitive. I would propose changing it so the enhanced code is always enabled and if OpenALSoft is not detected it would be disabled and a message is written to the debug log. A -no-enhanced-sound option can be added to always disable the new code.

@The-E

This comment has been minimized.

Show comment
Hide comment
@The-E

The-E Jun 1, 2015

Member

That would indeed be preferable, as it follows the precedent set by the various graphics flags.

Member

The-E commented Jun 1, 2015

That would indeed be preferable, as it follows the precedent set by the various graphics flags.

@chief1983

This comment has been minimized.

Show comment
Hide comment
@chief1983

chief1983 Jun 1, 2015

Member

Actually the graphics flags are currently a grab bag of some disable, some enable, whatever. I'm not sure I'm really a fan of that because it makes just reading the checklist difficult, but I do agree that since that's the case, for this flag, enabling it when possible by default would make the most sense.

Member

chief1983 commented Jun 1, 2015

Actually the graphics flags are currently a grab bag of some disable, some enable, whatever. I'm not sure I'm really a fan of that because it makes just reading the checklist difficult, but I do agree that since that's the case, for this flag, enabling it when possible by default would make the most sense.

@The-E

This comment has been minimized.

Show comment
Hide comment
@The-E

The-E Jun 2, 2015

Member

This is good to go in then.

Member

The-E commented Jun 2, 2015

This is good to go in then.

The-E added a commit that referenced this pull request Jun 2, 2015

Merge pull request #151 from jg18/enhanced_sound
Add support for enhanced sound.

@The-E The-E merged commit d7f54dd into scp-fs2open:master Jun 2, 2015

2 checks passed

continuous-integration/appveyor AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment