Improve audio settings #1582

daleglass · 2022-01-26T22:16:25Z

This implements configurable config settings for audio codecs. It's still at an early stage, and only being submitted for feedback and collaboration.

TODO list:

Cleanup code
API Documentation
Figure out the final list of audio parameters
Implement settings in interface
Fix Audio.getEncoderFeatures()

Initial API for audio codecs, client side:

Audio.getCodec() -- returns current codec
Audio.getCodecs() -- list of known codecs
Audio.getAllowedCodecs() -- list of codecs the interface is willing to use. All codecs are accepted if the list is empty. Will check that the names correspond with something in getCodecs(), and filter out any unknown entries.
Audio.setAllowedCodecs() -- set list of allowed codecs for the interface.
Audio.getEncoderVBR() -- is VBR enabled
Audio.setEncoderVBR(bool) -- enable/disable Variable Bit Rate
Audio.getEncoderBitrate()
Audio.getEncoderBitrate(int bitrate) -- Bit rate, in bits/s. 2400 to 512000. Default is 128000 (128kbps)
Audio.getEncoderComplexity()
Audio.getEncoderComplexity(int complexity) -- How CPU intensive compression is, 0 to 100.
Audio.getEncoderFEC() -- is VBR enabled
Audio.setEncoderFEC(bool) -- enable/disable Forward Error Correction
Audio.getEncoderPacketLossPercent()
Audio.setEncoderPacketLossPercent(int percent) -- how much data loss to allow for

Not working:
Audio.getEncoderFeatures() -- returns a feature list for the current encoder

Testing:

You can test with yourself by using this console command: Audio.setServerEcho(true). This will bounce audio off the domain at yourself. Microphone has to be on for it to work.
Use for instance Audio.setAllowedCodecs(["zlib"]) in the console, and observe the rise in bandwidth usage.
Use Audio.setEncoderBitrate(5000) to see the effect of 5 kbps audio
Use Audio.setEncoderComplexity(1) to see the effect of less CPU intensive encoding
Use Audio.setEncoderFEC(true) and Audio.setEncoderPacketLossPercent(10) to try error correction. Needs a connection with some packet loss.

Feedback would be very welcome -- especially regarding whether the API makes sense, and if there are threading issues anywhere in this.

Unfortunately, int fields don't seem to be editable currently, though the checkboxes work.

vircadia-build-notifier · 2022-01-28T03:17:45Z

The following links are available:
build (macOS-10.15, client)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1582/Vircadia-Interface-PR1582-1fb77d1-_bc0c8222.dmg

build (ubuntu-18.04, full)

build (windows-latest, full)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1582/Vircadia-PR1582-1fb77d1-_bc0c8222.exe

build (macOS-10.15, full)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1582/Vircadia-PR1582-1fb77d1-_bc0c8222.dmg

vircadia-build-notifier · 2022-01-28T05:20:30Z

The following links are available:
build (macOS-10.15, full)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1582/Vircadia-PR1582-8022cbf-_b4abd5ec.dmg

build (ubuntu-18.04, full)

build (macOS-10.15, client)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1582/Vircadia-Interface-PR1582-8022cbf-_b4abd5ec.dmg

build (windows-latest, full)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1582/Vircadia-PR1582-8022cbf-_b4abd5ec.exe

build (self-hosted_debian-11_aarch64, full)

vircadia-build-notifier · 2022-01-28T19:48:43Z

The following links are available:
build (macOS-10.15, client)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1582/Vircadia-Interface-PR1582-011f1ab-_76c8c1a3.dmg

build (macOS-10.15, full)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1582/Vircadia-PR1582-011f1ab-_76c8c1a3.dmg

build (ubuntu-18.04, full)

build (windows-latest, full)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1582/Vircadia-PR1582-011f1ab-_76c8c1a3.exe

build (self-hosted_debian-11_aarch64, full)

vircadia-build-notifier · 2022-01-30T19:33:17Z

The following links are available:
build (macOS-10.15, client)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1582/Vircadia-Interface-PR1582-e991e13-_aa99c37c.dmg

build (ubuntu-18.04, full)

build (macOS-10.15, full)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1582/Vircadia-PR1582-e991e13-_aa99c37c.dmg

build (windows-latest, full)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1582/Vircadia-PR1582-e991e13-_aa99c37c.exe

build (self-hosted_debian-11_aarch64, full)

Experimentally verified that nothing is heard below this value.

Codec feature info doesn't actually work yet, type needs registering

vircadia-build-notifier · 2022-01-31T01:51:45Z

The following links are available:
build (macOS-10.15, client)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1582/Vircadia-Interface-PR1582-a957b0e-_8652168c.dmg

build (macOS-10.15, full)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1582/Vircadia-PR1582-a957b0e-_8652168c.dmg

build (ubuntu-18.04, full)

build (windows-latest, full)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1582/Vircadia-PR1582-a957b0e-_8652168c.exe

build (self-hosted_debian-11_aarch64, full)

vircadia-build-notifier · 2022-02-01T02:06:50Z

The following links are available:
build (macOS-10.15, client)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1582/Vircadia-Interface-PR1582-e050759-_9b06c907.dmg

build (ubuntu-18.04, full)

build (macOS-10.15, full)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1582/Vircadia-PR1582-e050759-_9b06c907.dmg

build (self-hosted_debian-11_aarch64, full)

namark · 2022-02-06T20:50:24Z

These two constants are no longer used
https://github.com/vircadia/vircadia/blob/9b06c9070dd61ba97c075b0de08b71ad84374ec0/plugins/opusCodec/src/OpusEncoder.h#L67-L68
DEFAULT_APPLICATION is technically used in the constructor, but then it's immediately overwritten.

namark

Regarding the scripting interface, I would generally prefer to deal objects for this kind of things in JavaScript, instead of bunch of get/set functions, something similar in spirit to the settings JSON API we have, but that also seems like something way out of scope for this PR. I'm also not sure what the intended use cases are, so might be overthinking it.

namark · 2022-02-06T17:23:01Z

assignment-client/src/audio/AudioMixer.h

+/*
+    struct CodecSettings {
+        QString codec;
+        Encoder::Bandpass bandpass = Encoder::Bandpass::Fullband;
+        Encoder::ApplicationType applicationType = Encoder::ApplicationType::Audio;
+        Encoder::SignalType signalType = Encoder::SignalType::Auto;
+        int bitrate = 128000;
+        int complexity = 100;
+        bool enableFEC = 0;
+        int packetLossPercent = 0;
+        bool enableVBR = 1;
+    };
+*/


This is identical to the definition in the included plugins/CodecPlugin.h, not sure if it's here for reference, but seems out of place anyway

Suggested change

/*

struct CodecSettings {

QString codec;

Encoder::Bandpass bandpass = Encoder::Bandpass::Fullband;

Encoder::ApplicationType applicationType = Encoder::ApplicationType::Audio;

Encoder::SignalType signalType = Encoder::SignalType::Auto;

int bitrate = 128000;

int complexity = 100;

bool enableFEC = 0;

int packetLossPercent = 0;

bool enableVBR = 1;

};

*/

namark · 2022-02-06T17:28:13Z

assignment-client/src/audio/AudioMixer.h

    static int getStaticJitterFrames() { return _numStaticJitterFrames; }
    static bool shouldMute(float quietestFrame) { return quietestFrame > _noiseMutingThreshold; }
    static float getAttenuationPerDoublingInDistance() { return _attenuationPerDoublingInDistance; }
    static const std::vector<ZoneDescription>& getAudioZones() { return _audioZones; }
    static const std::vector<ZoneSettings>& getZoneSettings() { return _zoneSettings; }
    static const std::vector<ReverbSettings>& getReverbSettings() { return _zoneReverbSettings; }
    static const std::pair<QString, CodecPluginPointer> negotiateCodec(std::vector<QString> codecs);
+    static const std::vector<Encoder::CodecSettings> getCodecSettings() { return _codecSettings; }


I guess this comes from the line above, but return const value seems odd, unless I'm missing some coding standard. Hard to think of any actual use cases for it though in general, outside of some really weird proxy types maybe.

Suggested change

static const std::vector<Encoder::CodecSettings> getCodecSettings() { return _codecSettings; }

static const std::vector<Encoder::CodecSettings>& getCodecSettings() { return _codecSettings; }

I would change negotiateCodec as well to return a plain value without const, but that would be out of scope of this PR.

namark · 2022-02-06T18:25:01Z

domain-server/resources/describe-settings.json

@@ -1417,10 +1417,109 @@
        {
          "name": "codec_preference_order",
          "label": "Audio Codec Preference Order",
-          "help": "List of codec names in order of preferred usage",
+          "help": "List of codec names in order of preferred usage -- mod2",


I can't figure out what mod2 means here...

namark · 2022-02-06T19:32:31Z

libraries/plugins/src/plugins/CodecPlugin.h

+    /**
+     * @brief Represents the settings loaded from the domain's configuration page
+     *
+     * Encoder classes are created EntityScriptServer, Agent, AudioMixerClientData and AudioClient,


Suggested change

* Encoder classes are created EntityScriptServer, Agent, AudioMixerClientData and AudioClient,

* Encoder classes are created by EntityScriptServer, Agent, AudioMixerClientData and AudioClient,

namark · 2022-02-06T20:38:16Z

plugins/opusCodec/src/OpusEncoder.cpp

@@ -55,8 +55,8 @@ AthenaOpusEncoder::AthenaOpusEncoder(int sampleRate, int numChannels) {

    setBitrate(DEFAULT_BITRATE);
    setComplexity(DEFAULT_COMPLEXITY);
-    setApplication(DEFAULT_APPLICATION);
-    setSignal(DEFAULT_SIGNAL);
+    setApplication(Encoder::ApplicationType::Audio);


It seems like the default application type was VOIP before, not sure if this change is intentional. If I understand correctly this gets overwritten by CodecSettings' default which is also set to Audio.

namark · 2022-02-06T21:01:22Z

libraries/plugins/src/plugins/CodecPlugin.h

@@ -10,14 +10,196 @@
 //
 #pragma once

+#include <algorithm>


Unnecessary include?

namark · 2022-02-07T07:44:48Z

I'm thinking something like single set/get pair for all encoder settings, that would accept and return an object. The returned object would contain only what the current encoder supports, removing the need for getEncoderFeatures, and the object that the setter accepts could be partial, only setting whatever is specified.

namark · 2022-07-05T19:33:16Z

I've introduced some conflicts with this PR by moving the opus and PCM codec code into a library and decoupling the plugin and codec interfaces to make it all easier to reuse in context of the client library and Unity SDK. It's might be a while before that needs to be merged with master, so ideally we should get this finished up and merged first, if there aren't any major issues.

stale · 2023-01-01T23:31:09Z

Hello! Is this still an issue?

daleglass added 5 commits January 20, 2022 23:06

Partial changes

0613b8e

Partial implementation

7aae31f

Implement in zlib and PCM codecs

b86cb4a

Add missing implementation file

bc0c822

Pre-fill codec settings

b4abd5e

Unfortunately, int fields don't seem to be editable currently, though the checkboxes work.

Make codec settings modifiable

76c8c1a

daleglass added 2 commits January 29, 2022 15:15

Implement opus value bounds and scaling

81dfff7

Initial implementation of audio scripting interface for codecs

aa99c37

daleglass added 2 commits January 31, 2022 00:42

Bump minimum Opus bitrate to 2400

cb62e3f

Experimentally verified that nothing is heard below this value.

Implement support for VBR, FEC, and codec feature info

8652168

Codec feature info doesn't actually work yet, type needs registering

digisomni added the enhancement New feature or request label Jan 30, 2022

Add JSDoc

9b06c90

digisomni added the scripting api change Change is made in the scripting API label Feb 3, 2022

daleglass marked this pull request as ready for review February 3, 2022 22:21

digisomni added the needs CR (code review) label Feb 3, 2022

digisomni requested a review from namark February 5, 2022 19:08

namark reviewed Feb 6, 2022

View reviewed changes

namark mentioned this pull request Jul 3, 2022

Native client library audio mixer APIs. #1675

Merged

stale bot added the stale Issue / PR has not had activity label Jan 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve audio settings #1582

Improve audio settings #1582

daleglass commented Jan 26, 2022 •

edited

Loading

vircadia-build-notifier commented Jan 28, 2022

vircadia-build-notifier commented Jan 28, 2022

vircadia-build-notifier commented Jan 28, 2022

vircadia-build-notifier commented Jan 30, 2022

vircadia-build-notifier commented Jan 31, 2022

vircadia-build-notifier commented Feb 1, 2022

namark commented Feb 6, 2022

namark left a comment

namark Feb 6, 2022

namark Feb 6, 2022

namark Feb 6, 2022

namark Feb 6, 2022

namark Feb 6, 2022

namark Feb 6, 2022

namark commented Feb 7, 2022

namark commented Jul 5, 2022

stale bot commented Jan 1, 2023

	static const std::vector<Encoder::CodecSettings> getCodecSettings() { return _codecSettings; }
	static const std::vector<Encoder::CodecSettings>& getCodecSettings() { return _codecSettings; }

	* Encoder classes are created EntityScriptServer, Agent, AudioMixerClientData and AudioClient,
	* Encoder classes are created by EntityScriptServer, Agent, AudioMixerClientData and AudioClient,

Improve audio settings #1582

Are you sure you want to change the base?

Improve audio settings #1582

Conversation

daleglass commented Jan 26, 2022 • edited Loading

vircadia-build-notifier commented Jan 28, 2022

vircadia-build-notifier commented Jan 28, 2022

vircadia-build-notifier commented Jan 28, 2022

vircadia-build-notifier commented Jan 30, 2022

vircadia-build-notifier commented Jan 31, 2022

vircadia-build-notifier commented Feb 1, 2022

namark commented Feb 6, 2022

namark left a comment

Choose a reason for hiding this comment

namark Feb 6, 2022

Choose a reason for hiding this comment

namark Feb 6, 2022

Choose a reason for hiding this comment

namark Feb 6, 2022

Choose a reason for hiding this comment

namark Feb 6, 2022

Choose a reason for hiding this comment

namark Feb 6, 2022

Choose a reason for hiding this comment

namark Feb 6, 2022

Choose a reason for hiding this comment

namark commented Feb 7, 2022

namark commented Jul 5, 2022

stale bot commented Jan 1, 2023

daleglass commented Jan 26, 2022 •

edited

Loading