feat(Sounds): Added Preserve Audio Channels Capability. #3329
Conversation
|
I've tested it locally on my machine (as far as I could), but I would love to test this on one of the sim computers. Is there a way for me to build a test version of Thorium? I saw some electrum scripts, but I didn't want to do something that could break any released versions. |
src/components/macros/playSound.js
Outdated
| @@ -97,6 +97,14 @@ function PlaySound({updateArgs, args, stations, clients, playSound}) { | |||
| value={sound.channel} | |||
| onChange={e => updateSound("channel", e.target.value.split(","))} | |||
| /> | |||
| <div style={{ display: "flex", gap: '1rem', padding: '20px' }}> | |||
| <Label>Preserve Audio Channels <small>Advanced</small> </Label> | |||
There was a problem hiding this comment.
I would change this to "Preserve Surround Channels Advanced" to make it a little bit clear what its purpose is.
There was a problem hiding this comment.
How would you feel about me adding a popover with some help text about what these options (Channels and Preserve Surround Channels) do?
| @@ -97,6 +97,14 @@ function PlaySound({updateArgs, args, stations, clients, playSound}) { | |||
| value={sound.channel} | |||
| onChange={e => updateSound("channel", e.target.value.split(","))} | |||
There was a problem hiding this comment.
This input should be disabled/hidden if preserveChannels is true, since it won't have any effect. I would actually move the preserveChannels checkbox above this one to have users select it before editing the channel input.
server/classes/sound.js
Outdated
| @@ -10,5 +10,6 @@ export default class Sound { | |||
| this.playbackRate = params.playbackRate || 1; | |||
| this.channel = params.channel || [0, 1]; | |||
| this.looping = params.looping || false; | |||
| this.preserveChannels = params.preserveChannels || false; | |||
There was a problem hiding this comment.
This can default to true, since it will only apply to new sounds. I think this behavior is more expected than the channel down mixing / adjusting behavior.
| this.preserveChannels = params.preserveChannels || false; | |
| this.preserveChannels = params.preserveChannels === false ? false : true; |
|
I wasn't able to properly test this. Thought I could rig up a virtual 5.1 system, but it didn't pan out how I expected, though it did behave exactly as I expected when the checkbox was ticked - eg. playing the sound through Thorium had the same effect as playing the sound through QuickTime Player.. No matter. If you've managed to test it and it's working as expected in the simulators, we can consider this good to go. |
|
@alexanderson1993, alright. PR is ready for final review. I pushed up the tooltips and the disabled states after you approved. I also noticed the codeclimate check failed due to some duplicate code. Let me know if that's something you want me to dig in and take care of. |
Description
I've added the ability to preserve the audio channels for 5.1 and above sound files. @alexanderson1993 your comments in the Thorium discord made the fix pretty simple. 👍 Let me know if you see any issues with the change.
Related Issue
#3328
Screenshots (if appropriate):
feature on the Thorium Docs
repo. (Include the issue or pull request url below.)