Skip to content
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

Improve audio settings #1582

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
7 changes: 5 additions & 2 deletions assignment-client/src/Agent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,9 @@ void Agent::selectAudioFormat(const QString& selectedCodecName) {
_codec = plugin;
_receivedAudioStream.setupCodec(plugin, _selectedCodecName, AudioConstants::STEREO);
_encoder = plugin->createEncoder(AudioConstants::SAMPLE_RATE, AudioConstants::MONO);

//TODO: _codecSettings is not being set by anything -- where do we receive the settings here?
_encoder->configure(_codecSettings);
qDebug() << "Selected Codec Plugin:" << _codec.get();
break;
}
Expand Down Expand Up @@ -719,7 +722,7 @@ void Agent::processAgentAvatarAudio() {
if (isPlayingRecording && !_shouldMuteRecordingAudio) {
_shouldMuteRecordingAudio = true;
}

auto audioData = _avatarSound->getAudioData();
nextSoundOutput = reinterpret_cast<const int16_t*>(audioData->rawData()
+ _numAvatarSoundSentBytes);
Expand Down Expand Up @@ -880,7 +883,7 @@ void Agent::aboutToFinish() {
{
DependencyManager::get<ScriptEngines>()->shutdownScripting();
}

DependencyManager::destroy<ScriptEngines>();

DependencyManager::destroy<AssignmentDynamicFactory>();
Expand Down
1 change: 1 addition & 0 deletions assignment-client/src/Agent.h
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ private slots:
Encoder* _encoder { nullptr };
QTimer _avatarAudioTimer;
bool _flushEncoder { false };
std::vector<Encoder::CodecSettings> _codecSettings;
};

#endif // hifi_Agent_h
5 changes: 5 additions & 0 deletions assignment-client/src/audio/AudioMixer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ QStringList AudioMixer::_codecPreferenceOrder{};
vector<AudioMixer::ZoneDescription> AudioMixer::_audioZones;
vector<AudioMixer::ZoneSettings> AudioMixer::_zoneSettings;
vector<AudioMixer::ReverbSettings> AudioMixer::_zoneReverbSettings;
vector<Encoder::CodecSettings> AudioMixer::_codecSettings;

AudioMixer::AudioMixer(ReceivedMessage& message) :
ThreadedAssignment(message)
Expand Down Expand Up @@ -219,6 +220,7 @@ void AudioMixer::handleNodeMuteRequestPacket(QSharedPointer<ReceivedMessage> pac
// we need to set a flag so we send them the appropriate packet to mute them
AudioMixerClientData* nodeData = (AudioMixerClientData*)node->getLinkedData();
nodeData->setShouldMuteClient(true);
nodeData->setCodecSettings(_codecSettings);
} else {
qWarning() << "Node mute packet received for unknown node " << uuidStringWithoutCurlyBraces(nodeUUID);
}
Expand Down Expand Up @@ -393,6 +395,7 @@ AudioMixerClientData* AudioMixer::getOrCreateClientData(Node* node) {
node->setLinkedData(unique_ptr<NodeData> { new AudioMixerClientData(node->getUUID(), node->getLocalID()) });
clientData = dynamic_cast<AudioMixerClientData*>(node->getLinkedData());
connect(clientData, &AudioMixerClientData::injectorStreamFinished, this, &AudioMixer::removeHRTFsForFinishedInjector);
clientData->setCodecSettings(_codecSettings);
}

return clientData;
Expand Down Expand Up @@ -806,6 +809,8 @@ void AudioMixer::parseSettingsObject(const QJsonObject& settingsObject) {
}
}
}

_codecSettings = Encoder::parseSettings(audioEnvGroupObject);
}
}

Expand Down
18 changes: 17 additions & 1 deletion assignment-client/src/audio/AudioMixer.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

#include "AudioMixerStats.h"
#include "AudioMixerSlavePool.h"
#include "plugins/CodecPlugin.h"

class PositionalAudioStream;
class AvatarAudioStream;
Expand Down Expand Up @@ -52,13 +53,27 @@ class AudioMixer : public ThreadedAssignment {
float wetLevel;
};

/*
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;
};
*/
Comment on lines +56 to +68
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;
};
*/

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; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


static bool shouldReplicateTo(const Node& from, const Node& to) {
return to.getType() == NodeType::DownstreamAudioMixer &&
Expand All @@ -67,7 +82,7 @@ class AudioMixer : public ThreadedAssignment {
}

virtual void aboutToFinish() override;

public slots:
void run() override;
void sendStatsPacket() override;
Expand Down Expand Up @@ -149,6 +164,7 @@ private slots:
static std::vector<ZoneDescription> _audioZones;
static std::vector<ZoneSettings> _zoneSettings;
static std::vector<ReverbSettings> _zoneReverbSettings;
static std::vector<Encoder::CodecSettings> _codecSettings;

float _throttleStartTarget = 0.9f;
float _throttleBackoffTarget = 0.44f;
Expand Down
6 changes: 4 additions & 2 deletions assignment-client/src/audio/AudioMixerClientData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ void AudioMixerClientData::optionallyReplicatePacket(ReceivedMessage& message, c

packet->write(message.getMessage());
}

nodeList->sendUnreliablePacket(*packet, *downstreamNode);
}
});
Expand Down Expand Up @@ -375,7 +375,7 @@ int AudioMixerClientData::parseData(ReceivedMessage& message) {
qWarning() << "Received AudioStreamStats of wrong size" << message.getBytesLeftToRead()
<< "instead of" << sizeof(AudioStreamStats) << "from"
<< message.getSourceID() << "at" << message.getSenderSockAddr();

return message.getPosition();
}

Expand Down Expand Up @@ -783,6 +783,8 @@ void AudioMixerClientData::setupCodec(CodecPluginPointer codec, const QString& c
if (codec) {
_encoder = codec->createEncoder(AudioConstants::SAMPLE_RATE, AudioConstants::STEREO);
_decoder = codec->createDecoder(AudioConstants::SAMPLE_RATE, AudioConstants::MONO);

_encoder->configure(_codecSettings);
}

auto avatarAudioStream = getAvatarAudioStream();
Expand Down
2 changes: 2 additions & 0 deletions assignment-client/src/audio/AudioMixerClientData.h
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ class AudioMixerClientData : public NodeData {
bool shouldFlushEncoder() { return _shouldFlushEncoder; }

QString getCodecName() { return _selectedCodecName; }
void setCodecSettings(const std::vector<Encoder::CodecSettings> &settings) { _codecSettings = settings; }

bool shouldMuteClient() { return _shouldMuteClient; }
void setShouldMuteClient(bool shouldMuteClient) { _shouldMuteClient = shouldMuteClient; }
Expand Down Expand Up @@ -201,6 +202,7 @@ public slots:
QString _selectedCodecName;
Encoder* _encoder{ nullptr }; // for outbound mixed stream
Decoder* _decoder{ nullptr }; // for mic stream
std::vector<Encoder::CodecSettings> _codecSettings;

bool _shouldFlushEncoder { false };

Expand Down
16 changes: 12 additions & 4 deletions assignment-client/src/scripts/EntityScriptServer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,13 @@ void EntityScriptServer::handleSettings() {
_maxEntityPPS = std::max(0, entityScriptServerSettings[MAX_ENTITY_PPS_OPTION].toInt());
_entityPPSPerScript = std::max(0, entityScriptServerSettings[ENTITY_PPS_PER_SCRIPT].toInt());

static const QString AUDIO_ENV_GROUP_KEY = "audio_env";
if ( settingsObject.contains(AUDIO_ENV_GROUP_KEY)) {
_codecSettings = Encoder::parseSettings(settingsObject[AUDIO_ENV_GROUP_KEY].toObject());
} else {
qWarning() << "Failed to find" << AUDIO_ENV_GROUP_KEY << "key in settings. Dump: " << settingsObject;
}

qDebug() << QString("Received entity script server settings, Max Entity PPS: %1, Entity PPS Per Entity Script: %2")
.arg(_maxEntityPPS).arg(_entityPPSPerScript);
}
Expand Down Expand Up @@ -288,7 +295,7 @@ void EntityScriptServer::run() {
entityScriptingInterface->init();

_entityViewer.init();

// setup the JSON filter that asks for entities with a non-default serverScripts property
QJsonObject queryJSONParameters;
queryJSONParameters[EntityJSONQueryProperties::SERVER_SCRIPTS_PROPERTY] = EntityQueryFilterSymbol::NonDefault;
Expand All @@ -299,7 +306,7 @@ void EntityScriptServer::run() {
queryFlags[EntityJSONQueryProperties::INCLUDE_DESCENDANTS_PROPERTY] = true;

queryJSONParameters[EntityJSONQueryProperties::FLAGS_PROPERTY] = queryFlags;

// setup the JSON parameters so that OctreeQuery does not use a frustum and uses our JSON filter
_entityViewer.getOctreeQuery().setJSONParameters(queryJSONParameters);

Expand Down Expand Up @@ -376,7 +383,7 @@ void EntityScriptServer::nodeKilled(SharedNodePointer killedNode) {
if (!hasAnotherEntityServer) {
clear();
}

break;
}
case NodeType::Agent: {
Expand Down Expand Up @@ -438,6 +445,7 @@ void EntityScriptServer::selectAudioFormat(const QString& selectedCodecName) {
if (_selectedCodecName == plugin->getName()) {
_codec = plugin;
_encoder = plugin->createEncoder(AudioConstants::SAMPLE_RATE, AudioConstants::MONO);
_encoder->configure(_codecSettings);
qCDebug(entity_script_server) << "Selected Codec Plugin:" << _codec.get();
break;
}
Expand Down Expand Up @@ -579,7 +587,7 @@ void EntityScriptServer::sendStatsPacket() {
}
scriptEngineStats["number_running_scripts"] = numberRunningScripts;
statsObject["script_engine_stats"] = scriptEngineStats;


auto nodeList = DependencyManager::get<NodeList>();
QJsonObject nodesObject;
Expand Down
2 changes: 2 additions & 0 deletions assignment-client/src/scripts/EntityScriptServer.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include <SimpleEntitySimulation.h>
#include <ThreadedAssignment.h>
#include "../entities/EntityTreeHeadlessViewer.h"
#include "plugins/CodecPlugin.h"

class EntityScriptServer : public ThreadedAssignment {
Q_OBJECT
Expand Down Expand Up @@ -90,6 +91,7 @@ private slots:
QString _selectedCodecName;
CodecPluginPointer _codec;
Encoder* _encoder { nullptr };
std::vector<Encoder::CodecSettings> _codecSettings;
};

#endif // hifi_EntityScriptServer_h
103 changes: 101 additions & 2 deletions domain-server/resources/describe-settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -1277,7 +1277,7 @@
{
"name": "audio_env",
"label": "Audio Environment",
"assignment-types": [ 0 ],
"assignment-types": [ 0, 5 ],
"settings": [
{
"name": "attenuation_per_doubling_in_distance",
Expand Down Expand Up @@ -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",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

"placeholder": "opus, hifiAC, zlib, pcm",
"default": "opus,hifiAC,zlib,pcm",
"advanced": true
},
{
"name": "codec_settings",
"label": "Audio Codec Settings",
"help": "Advanced settings for audio codecs",
"advanced": true,
"type": "table",
"numbered": false,
"content_setting": false,
"can_add_new_rows": true,
"columns" : [
{
"name": "codec",
"label": "Codec",
"can_set": true,
"placeholder": "(codec)",
"editable": false
},
{
"name": "complexity",
"label": "Complexity",
"can_set": true,
"type": "int",
"placeholder": "(percentage)",
"default": 0,
"editable": true
},
{
"name": "bitrate",
"label": "Bitrate",
"can_set": true,
"type": "int",
"placeholder": "(bps)",
"default" : 0,
"editable": true
},
{
"name": "vbr",
"label": "Variable Bitrate",
"can_set": true,
"type": "checkbox",
"default": false,
"editable": true
},
{
"name": "fec",
"label": "Error correction",
"can_set": true,
"type": "checkbox",
"default" : false,
"editable": true
},
{
"name": "packet_loss",
"label": "Expected loss %",
"can_set": true,
"type": "int",
"placeholder": "(percentage)",
"default": 0,
"editable": true
}

],
"non-deletable-row-key": "codec",
"non-deletable-row-values": [ "pcm", "zlib", "hifiAC", "opus" ],
"default": [
{
"codec": "pcm",
"complexity": 0,
"bitrate": 0,
"vbr": false,
"fec": false,
"packet_loss": 0
},
{
"codec": "zlib",
"complexity": 6,
"bitrate": 0,
"vbr": false,
"fec": false,
"packet_loss": 0
},
{
"codec": "hifiAC",
"complexity": 0,
"bitrate": 0,
"vbr": false,
"fec": false,
"packet_loss": 0
},
{
"codec": "opus",
"complexity": 10,
"bitrate": 128000,
"vbr": true,
"fec": false,
"packet_loss": 0
}
]
}
]
},
Expand Down
8 changes: 7 additions & 1 deletion domain-server/resources/web/js/base-settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ function reloadSettings(callback) {
Settings.pendingChanges = 0;

// call the callback now that settings are loaded
if (callback) {
if (callback) {
callback(true);
}
}).fail(function() {
Expand Down Expand Up @@ -591,6 +591,12 @@ function makeTable(setting, keypath, setting_value) {
"<input type='time' class='form-control table-time' name='" + colName + "' " +
"value='" + (colValue || col.default || "00:00") + "'/>" +
"</td>";
} else if (isArray && col.type === "int" && col.editable) {
html +=
"<td class='" + Settings.DATA_COL_CLASS + "'name='" + col.name + "'>" +
"<input type='int' class='form-control table-time' name='" + colName + "' " +
"value='" + (colValue || col.default || "0") + "'/>" +
"</td>";
} else {
// Use a hidden input so that the values are posted.
html +=
Expand Down
Loading