Introducing the SKID chip. ( Fixes #1832 - sound system.) #1859

Dunbaratu · 2016-10-29T10:45:06Z

Issue #1832, implementing a sound system.

This is the end result of what I've been working on on-stream in #Creative mode in Twitch. It can still be improved later on by adding more wave forms if we feel like it, and fancier effects (like the ADSR really should vary with frequency because IRL when an instrument is vibrating the air faster, its notes tend to die down sooner due to the higher energy it takes to keep the vibration going.)

But I feel like this is the appropriate "level" of complexity for the kinds of hardware we're trying to pretend to be.

As always, with any new feature, start from reading the RST files to get the gist of what it does and how.

The problem that sucked my time: AudioSource.pitch is apparently capped at about 20ish or so, making my original plan (record at 1 Hz then speed it up 500 times to get a 500 Hz tone) not work. Once I knew that it could only pitch up or down a limited amount, I knew to record the sample much higher.

Also had another go at making the noise wave sound better but it still sounds too "tonal". I may have to go look up how to make a proper white noise wave that still has recognizable "pitch" to it. I'm sure that information is out on the internet somewhere.

Conflicts: src/kOS/Screen/TermWindow.cs

Dunbaratu · 2016-10-31T11:56:00Z

doc/source/general/skid.rst

+step above middle C.  ``"Db4"`` is the D-flat that is in fact the same
+thing as ``"C#4"``.  ``"B3"`` is the B that is just to the left of
+middle C (Note the octave numbering starts with C at the bottom of the
+octave, not A, thus why A3 is adjacent to C4.)


Dangit, I proofread this once over but missed this. Where it says "A3 is adjacent" it should say "B3 is adjacent".

teleksterling · 2016-11-02T23:22:58Z

doc/source/general/skid.rst

+
+1. Mandatory: First character is the note letter, one of 
+   "C","D","E","F","G","A", "B", or "R"(to mean "rest").
+2. Optonal: Followed by an optional character, "#" or "b" for "sharp" or


Also a small typo here for 'optional'.

Dunbaratu · 2016-11-08T06:16:28Z

I may try to add a small change to support "slide" notes. (Like a NOTE() constructor that gives both a start and end frequency, as in "start this note at 440hz, end it at 510hz, taking 0.5 seconds to do so".) In trying to make such effects in kerboscript code I ran into the fact that the rounded-off integer-ized updating made it sound really bad. This is because whenever you change the NOTE() in Kerboscript, it starts playing a new wave from the start of a new period, regardless of where the note was before within it's period at the moment it was swapped out for the new note. That causes disjoint jumps in the sound wave as it changes sharply between two adjacent samples. When you only change notes no faster than 5 times a second or so, you don't care. But when it's meant to be a smooth transition that changes frequency 25 times a second, you start to hear those jumps that are happening 25 times a second as a secondary raspberry buzz effect underlying the main sound.

But if I have the C# code (i.e. the 'chip hardware') do the changes itself and it was told to make it a smooth slide between notes, then it knows not to start a new note over, and instead just smoothly adjust the rate at which it's playing the wave samples. Each frequency change picks up with the sample value right where the previous one left off, eliminating those jumps.

Dunbaratu · 2016-11-08T08:26:30Z

I hope you don't mind that I am pushing a new change to the PR branch while you were reviewing. I implemented the sliding note feature talked about in the above comment.

hvacengi · 2016-11-09T02:22:19Z

I don't mind. I haven't made any major changes yet anyways, or finished my full code walk (though I've made it most of the way through now).

…view/1859

hvacengi

I'll work on finishing my review changes tomorrow and push to develop.

hvacengi · 2016-11-11T02:38:11Z

src/kOS/Screen/TermWindow.cs

@@ -178,6 +181,11 @@ private void LoadFontArray()
            }
        }

+        public kOS.Safe.Sound.ISoundMaker GetSoundMaker()


Is there a reason you moved this from SharedObjects into TermWindow? It looks like it isn't technically tied to the terminal itself for function. Is the idea that audio is another form of interface and should be integrated to the terminal?

My intent was to expose GetSoundMaker() to kOS.Safe so it could be called from kOS.Safe without breaking (it would be just a dummy do-nothing if Unity's not there, but it wouldn't break the compile). But the way SharedObjects vs SafeSharedObects works, I can't do that interface abstraction at the level of SharedObjects, I have to do it on one of the items in SharedObjects, and ITermWindow made more sense than any of the others did.

But in the end I didn't end up needing to expose anything about it after all, so it didn't end up in ITermWindow. But I prefer it the way it is now because it leaves open the chance of putting it there later if we need to.

hvacengi · 2016-11-12T06:03:26Z

src/kOS/Sound/VoiceValue.cs

+
+            // Increment to next note and start playing it:
+            ++noteNum;
+            if (noteNum > song.Count())


This needs to be >= since if noteNum == song.Count() it will cause an index out of range exception.

It looks like you're right, but what's weird is that I've used this code to make looping music and it worked just fine. Let me look into why.

hvacengi · 2016-11-12T06:04:54Z

src/kOS/Screen/TermWindow.cs

@@ -133,6 +134,8 @@ public void Awake()

            GameEvents.onHideUI.Add (OnHideUI);
 			GameEvents.onShowUI.Add (OnShowUI);


Some how this section previously used tabs instead of spaces, so the new portion that follows also uses tabs, I'll swap to spaces.

hvacengi · 2016-11-12T06:05:37Z

src/kOS/Sound/Voice.cs

+
+namespace kOS.Sound
+{
+	/// <summary>


This whole file seems to use tabs instead of spaces, I'll swap to spaces.

I swear I told it to always use spaces instead of tabs. I recently upgraded my copy of SharpDevelop. Maybe in so doing I lost that setting.

Dunbaratu · 2016-11-13T08:19:40Z

src/kOS/Sound/VoiceValue.cs

+
+            // Increment to next note and start playing it:
+            ++noteNum;
+            if (noteNum > song.Count())


It looks like you're right, but what's weird is that I've used this code to make looping music and it worked just fine. Let me look into why.

Dunbaratu · 2016-11-13T08:24:47Z

src/kOS/Sound/VoiceValue.cs

+                    noteEndTimeStamp = now + tempo*curNote.Duration;
+                    noteFreqTotalChange = curNote.EndFrequency - curNote.Frequency;
+                    voice.BeginProceduralSound(curNote.Frequency, tempo*curNote.KeyDownLength, curNote.Volume);
+                }


Ah this is why it worked despite the index off by one error. When it hits the end the exception aborts the Update(), but only just at the point where the update didn't have any work left to do anyway so aborting the Update() didn't make any noticable difference unless I looked at the output log. The only thing it did is delay the restart of the song (when looping) until the next update after this one (when noteNum was +1'ed one more time). I wasn't able to hear that 1/25th of a second extra delay so didn't see any obvious sign of error.

Yeah, I test using the debug instance of KSP, which pops up a red error message any time that there's an error, it really helps me find fringe cases like this. In fact I have some other error messages that have popped up that I want to fix eventually too.

Dunbaratu · 2016-11-13T08:34:17Z

src/kOS/Screen/TermWindow.cs

@@ -178,6 +181,11 @@ private void LoadFontArray()
            }
        }

+        public kOS.Safe.Sound.ISoundMaker GetSoundMaker()


My intent was to expose GetSoundMaker() to kOS.Safe so it could be called from kOS.Safe without breaking (it would be just a dummy do-nothing if Unity's not there, but it wouldn't break the compile). But the way SharedObjects vs SafeSharedObects works, I can't do that interface abstraction at the level of SharedObjects, I have to do it on one of the items in SharedObjects, and ITermWindow made more sense than any of the others did.

But in the end I didn't end up needing to expose anything about it after all, so it didn't end up in ITermWindow. But I prefer it the way it is now because it leaves open the chance of putting it there later if we need to.

Dunbaratu · 2016-11-14T17:28:52Z

Are you making the changes (importantly, the off-by-one error you caught with '<' instead of '<='), or do you want me to do it and push to the branch?

hvacengi · 2016-11-14T20:07:05Z

I'm making changes myself. You don't need to worry about it.

And I think I've decided that if we enable procedural sound waves as one of the things that addons can add, we'll do it in a future release. There is no sense in complicating the review of this pull request itself with feature addition.

Mostly formatting VoiceValue.cs * Fix >= issue with noteNum testskid.ks * Test script with "Mary had a Little Lamb"

Voice.cs * Try to eliminate clicking noise by not stoping the `AudioSource` but instead changing the pitch/volume

I thought I was fixing an index incriment but actually would have broken it. Oops. Fixed now in ProceduralSoundWave.cs

…832_sound_system

Suffixed.cs * Flip keyDownDuration and duration parameters * duration is now first, keyDownDuration is optional * clamp keyDownDuration to no more than the value of duration Voice.cs * Update BeginProceduralSound so that it will not update any settings for a rest note * The Release component of the ADSR envelop now defaults to 0.1 * Combined with the change to note functions this has the effect of notes sounding like individual notes when chained together, rather than running together as if they were a single note * This also reduces some of the audio artifacts (clicking) found with rapidly changing notes **DOCUMENTATION NOT UPDATED TO MATCH**

Essentially all of the changes are with the intention of supporting a STOP suffix on each VoiceValue, a STOPALLVOICES function for kerboscript, and the stopping of all voices for execution breaks (like power loss, exceptions, etc.). Rather than noting each change, this message will highlight notable changes: CPU.cs * Stop all voices when breaking execution * Stop all voices on *any* exception (both from interpreter and from program) Suffixed.cs * Rename FuncitonNote (misspelled and incorrect reference) to FunctionGetVoice * Add FunctionStopAllVoices kOSProcessor.cs * Stop all voices when the processor is turned off SharedObjects.cs * Add a dictionary for tracking all VoiceValues VoiceValue.cs * Add suffix STOP and corresponding Stop method.

Dunbaratu added 10 commits October 6, 2016 18:22

Merge branch 'develop' into issue_1832_sound_system

a3359e1

Conflicts: src/kOS/Screen/TermWindow.cs

Merge branch 'hvacengi_1_2_experimentals' into issue_1832_sound_system

74ec5c4

Renamed SinWave to SineWave, and upped the base frequency.

d970910

Merge branch 'develop' into issue_1832_sound_system

861bf5d

Much of the way there. Don't have songs yet, but have everything else.

c6541ce

Added tempo, and playing a song list.

28c7ce5

First draft SKID docs, and removed unused method.

9b32eb0

SKID docs fixups after seeing Sphinx output.

ded31f2

Dunbaratu added the enhancement Something new (not a bug fix) is being requested label Oct 29, 2016

Dunbaratu commented Oct 31, 2016

View reviewed changes

teleksterling reviewed Nov 2, 2016

View reviewed changes

Dunbaratu added 2 commits November 8, 2016 02:22

Implement slidiing notes.

bfd1f5c

added minor doc typo fixes

7365312

Merge branch 'develop' of https://github.com/KSP-KOS/KOS into pull-re…

68d8cb8

…view/1859

hvacengi reviewed Nov 12, 2016

View reviewed changes

Dunbaratu commented Nov 13, 2016

View reviewed changes

hvacengi added 2 commits November 14, 2016 22:52

Initial Review

d4fb63d

Mostly formatting VoiceValue.cs * Fix >= issue with noteNum testskid.ks * Test script with "Mary had a Little Lamb"

First blush at addressing clicking sound

98ab4eb

Voice.cs * Try to eliminate clicking noise by not stoping the `AudioSource` but instead changing the pitch/volume

hvacengi mentioned this pull request Nov 15, 2016

Pull review/1859 Dunbaratu/KOS-1#20

Merged

hvacengi and others added 4 commits November 15, 2016 00:18

Fix my mistake

2e24a0f

I thought I was fixing an index incriment but actually would have broken it. Oops. Fixed now in ProceduralSoundWave.cs

Merge branch 'develop' into issue_1832_sound_system

b9f49a0

Merge remote-tracking branch 'hvacengi/pull-review/1859' into issue_1…

57ec817

…832_sound_system

fixed obi-wan error added in hvac's PR.

1cc42ba

Dunbaratu and others added 7 commits November 15, 2016 12:30

Merge branch 'develop' into issue_1832_sound_system

4b6f7e2

Changed Mary Had A Little Lamb example a bit.

81d37c8

Switched to use Update() and unscaledTime.

dc3961a

Forgot to adjust noteStartTimeStamp too.

12b0045

Added some doc changes for hvacengi's changes.

1ff6759

Dunbaratu merged commit 1ff6759 into KSP-KOS:develop Nov 17, 2016

Dunbaratu mentioned this pull request Nov 17, 2016

Procedural simple sound system #1832

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introducing the SKID chip. ( Fixes #1832 - sound system.) #1859

Introducing the SKID chip. ( Fixes #1832 - sound system.) #1859

Dunbaratu commented Oct 29, 2016

Dunbaratu Oct 31, 2016

teleksterling Nov 2, 2016

Dunbaratu commented Nov 8, 2016

Dunbaratu commented Nov 8, 2016

hvacengi commented Nov 9, 2016

hvacengi left a comment

hvacengi Nov 11, 2016

Dunbaratu Nov 13, 2016

hvacengi Nov 12, 2016

Dunbaratu Nov 13, 2016

hvacengi Nov 12, 2016

hvacengi Nov 12, 2016

Dunbaratu Nov 13, 2016

Dunbaratu Nov 13, 2016

Dunbaratu Nov 13, 2016

hvacengi Nov 13, 2016

Dunbaratu Nov 13, 2016

Dunbaratu commented Nov 14, 2016

hvacengi commented Nov 14, 2016

		@@ -133,6 +134,8 @@ public void Awake()

		GameEvents.onHideUI.Add (OnHideUI);
		GameEvents.onShowUI.Add (OnShowUI);

Introducing the SKID chip. ( Fixes #1832 - sound system.) #1859

Introducing the SKID chip. ( Fixes #1832 - sound system.) #1859

Conversation

Dunbaratu commented Oct 29, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dunbaratu commented Nov 8, 2016

Dunbaratu commented Nov 8, 2016

hvacengi commented Nov 9, 2016

hvacengi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dunbaratu commented Nov 14, 2016

hvacengi commented Nov 14, 2016