-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Fix: Force stereo when opening audio device #51754
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
actual-nh
added
<Bugfix>
This is a fix for a bug (or closes open issue)
[C++]
Changes (can be) made in C++. Previously named `Code`
SDL: Tiles / Sound
Tiles visual interface and sounds.
labels
Sep 21, 2021
I put your summary into the format if you need to change any of the words inside the " " it's fine. |
Just noticed you need to astyle your code. Once you do so, please ping me and I'll kick your tests to start running again. It looks like they'll pass fine, but the repo settings require that all tests listed as required have passed on the final commit to merge. |
I-am-Erk
added a commit
to I-am-Erk/Cataclysm-DDA
that referenced
this pull request
Sep 23, 2021
* reorganized to support expansion * reorganized to support expansion * removed sentences that shouldn't be there * fixed log stable appearing as kitchen construction * Reworked to push expansions back to the top level + doc * testing disallows base name recipe changes * removal of characters in middle of token managed to offend secret json format mandate * Adjusted to CleverRaven#46278 * incorporated some old reverse conflict changes * reorganized the stuff added while this PR was suspended * reorganized the stuff added while this PR was suspended * Fix bike shop variant roof Fix the bike shop variant 1 having an empty roof * Change stairs to ladder Changed stairs down to ladder down * Update the table of contents * Remove duplicate shortcut keys from interaction menu Fixes CleverRaven#51772 Co-authored-by: David Seguin <davidseguin@live.ca> * Jsonified dead scientists map extra * Use string_id for update_mapgen and nested_mapgen These ids were previously just using strings; better to use a dedicated string_id type. This required adding an actual type for the string_id to point to in each case, wrapping the collection of mapgen_function_json objects. * Update cooking_components.json * Force recipes to provide (sub)category As suggested in CleverRaven#50866, require all recipe definitions to provide a category and subcategory when loading from JSON. * Add subcategory to Magiclysm recipe * Clarify description of mon_zombie_dog (CleverRaven#51801) See <https://discord.com/channels/598523535169945603/598614717799596055/890213920957161472> on the dev discord. * Test disabling expensive LGTM checks (CleverRaven#51806) Disable one LGTM checks to see if there's hope of reducing the runtime thereby. * Mythos Mod Resubmission (CleverRaven#51796) * Mythos Mod Resubmission * Remove Occulti as maintainer * Fix typo in "Storehouse survey" blueprint name * Add dialog activity functions, and flag to prevent activity being interrupted (CleverRaven#51809) * Add dialog stuff * docs * Update player_activities.json * use snippets (CleverRaven#51663) Co-authored-by: Saicchi <Saicchi@users.noreply.github.com> Co-authored-by: Kevin Granade <kevin.granade@gmail.com> * Blacksmithing tongs and cooking tongs are two diffrent things. (CleverRaven#51672) * Add blacksmithing tongs. The tongs in game wouldn't be able to carry around a four kilo zweihander. They are designed for cooking and flipping food, so they are very lightweight and thing, they would most likely bend under the heat of the hot metal. * Added the need to use blacksmithing (flatjaw) tongs for the Zweihander Not using tongs for smithing is almost impossible. * Change the blacksmithing toolset to include the tongs. I also removed the tongs from the tools on the zweihander. Co-authored-by: Anton Burmistrov <Night_Pryanik@mail.ru> Co-authored-by: Kevin Granade <kevin.granade@gmail.com> * More granular json-style and astyle (CleverRaven#51731) * More granular json-style and astyle * Melee practice and training dummies (CleverRaven#51598) * Melee practice Added some basic melee practice recipes, including 2 for training dodge. * Training dummy for practice recipes * Dummy recipe and resolve problems * Intermediate recipes and heavy training dummy * Changed the armored dummy recipe The recipe is inspired by the scrap suit recipe. * Added tanto as a possible weapon for stabbing training * Punching bag Co-authored-by: Marloss <78324429+MarlossCDDA@users.noreply.github.com> * Write documentation for neighbor based chunk spawning (CleverRaven#51818) * Resubmit Fix Tire Rims (CleverRaven#51798) * Update goblin.json (CleverRaven#51799) * Fix: Force stereo when opening audio device (CleverRaven#51754) Co-authored-by: Kevin Granade <kevin.granade@gmail.com> * Add support for weakpoint damage and crit multipliers (CleverRaven#51770) * Weakpoints (Part 4.5): Add support for JSON inheritance and default weakpoints (CleverRaven#51822) * Let contributors specify the default weakpoint, by providing an empty ID * Add support for weakpoint inheritance. * Reduce debug.log file size by folding consecutive identical errors (CleverRaven#51791) * debug_fold_repetition: Fold identical errors to reduce log spam * debug_fold_repetition: added timestamp and timeout timestamp for repeated error folding group is that of the last folded error timeout is currently hardcoded to 100ms. If another repeated error is registered that would be folded but is >100ms after the previous error it starts a new folding group * debug_fold_repetitions: bookended fold for readability, made output DRY It's a little easier to read the log as `[First]<folded>[Last]` than `[First]<folded with last timestamp>` Need to test folding output during deinit and DebugLog, made a function to remove repetition. * debug_fold_repetitions: excess repetition error to forced prompt As logged error the excessive repetition error causes breaks in folds which make debug.log less clear while reading, and does not add information Forced prompting once excess threshold is reached will prompt once per excessive repetition set. * Allow using any tool with the drilling quality to make a draw_plate (CleverRaven#51794) Fixes CleverRaven#51688 * The (impotent) Sound and the Fury (CleverRaven#51803) * update m231 Updated price(https://www.invaluable.com/auction-lot/incredibly-rare-colt-m231-port-firing-u-s-propert-2086-c-3574a02a1e), adjusted desc, set sight disp to that of other no-sight guns, adjusted mod locations. * Rapidly changing light levels do not grant max vision to turrets (CleverRaven#51815) * Update fake.json (CleverRaven#51819) * Fix process effects crash (CleverRaven#51827) * Add Complete missions dialog function (CleverRaven#51820) * Complete missions * Update npctalk.cpp * Update npctalk.cpp Co-authored-by: PatrikLundell <j.patrik.r.lundell@gmail.com> Co-authored-by: SegaSaturnity <75910217+SegaSaturnity@users.noreply.github.com> Co-authored-by: kevingranade <kevingranade@users.noreply.github.com> Co-authored-by: UmbralReaper <67179462+UmbralReaper@users.noreply.github.com> Co-authored-by: David Seguin <davidseguin@live.ca> Co-authored-by: Valiant <Night_Pryanik@mail.ru> Co-authored-by: John Bytheway <52664+jbytheway@users.noreply.github.com> Co-authored-by: mythosmod <91185016+mythosmod@users.noreply.github.com> Co-authored-by: Kevin Granade <kevin.granade@gmail.com> Co-authored-by: actual-nh <74678550+actual-nh@users.noreply.github.com> Co-authored-by: Angela Graves <rivet.the.zombie@gmail.com> Co-authored-by: Binrui Dong <brett.browning.dong@gmail.com> Co-authored-by: Eric <52087122+Ramza13@users.noreply.github.com> Co-authored-by: Saicchi <47158232+Saicchi@users.noreply.github.com> Co-authored-by: Saicchi <Saicchi@users.noreply.github.com> Co-authored-by: GOFLUMPYOURSELFPORCAY <64309930+GOFLUMPYOURSELFPORCAY@users.noreply.github.com> Co-authored-by: Termineitor244 <termineitor244@hotmail.com> Co-authored-by: Marloss <78324429+MarlossCDDA@users.noreply.github.com> Co-authored-by: Zhilkin Serg <ZhilkinSerg@users.noreply.github.com> Co-authored-by: John Candlebury <johncandlebury@gmail.com> Co-authored-by: The SzQ <37194372+SzQ1@users.noreply.github.com> Co-authored-by: Joshua Chin <5422226+Joshua-Chin@users.noreply.github.com> Co-authored-by: OrenAudeles <orenaudeles@gmail.com> Co-authored-by: Tonkatsu <7764202+tenmillimaster@users.noreply.github.com> Co-authored-by: Roy Berube <royberube1965@gmail.com> Co-authored-by: ISuckM8 <87550905+ISuckM8@users.noreply.github.com> Co-authored-by: ferociousdork <78301810+ferociousdork@users.noreply.github.com>
BrettDong
added a commit
that referenced
this pull request
Nov 27, 2021
ZhilkinSerg
pushed a commit
that referenced
this pull request
Nov 27, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
<Bugfix>
This is a fix for a bug (or closes open issue)
[C++]
Changes (can be) made in C++. Previously named `Code`
SDL: Tiles / Sound
Tiles visual interface and sounds.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Bugfixes "Fixes Phantom metallic sound"
Purpose of change
Fixes "metallic" pitch-shifted sounds (e.g. walking) on some sound cards.
Describe the solution
I had to deep dive SDL2 again, and in the process noticed that Mix_OpenAudio() by default allows SDL2 to choose any number of sound channels. For some sound devices, this number may be higher than 2 (e.g. 4 or 6).
When sound is to be played, it is first loaded via SDL2, which converts it into the mixer's format. If played sound must be pitch-shifted, it is then passed to Cata's do_pitch_shift() which assumes there are always 2 channels, and in case there are not 2 channels - scrambles the sound and turns it "metallic".
You can reproduce "metallic" effect on system with any sound card by changing int audio_channels to 6 and then launching the game with SDL_AUDIODRIVER=disk environment variable. SDL2 will create a raw file in game's directory and use it as output device that matches desired specs, and afterwards the file can be imported into Audacity (16-bit PCM, LE, 6 channels, 44100) and replayed to get exactly this:
https://user-images.githubusercontent.com/74045419/112876475-48654500-9111-11eb-8fd4-b57b76d8c415.mp4
Solution is to use Mix_OpenAudioDevice(), which allows specifying what specs the device must match, and force device to be stereo.
Describe alternatives you've considered
Fixing do_pitch_shift() for non-stereo, but I doubt surround sound is something a roguelike needs to support. Also, less channels == less RAM consumed by each sound sample.
Testing
I don't have the hardware myself, but @EirenexTheDragon confirmed this patch fixes sounds.
Additional context
Code stolen from @olanti-p. Then transformed also by Olanti to work with DDA.
Title and description of PR stolen from Olanti as well, from here:
cataclysmbnteam/Cataclysm-BN#863