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

Fix audio mode command. Disable panner nodes in safari. #4594

Merged
merged 4 commits into from
Sep 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 24 additions & 2 deletions src/components/avatar-audio-source.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,23 @@ async function getMediaStream(el) {

AFRAME.registerComponent("avatar-audio-source", {
createAudio: async function() {
APP.supplementaryAttenuation.delete(this.el);

this.isCreatingAudio = true;
const stream = await getMediaStream(this.el);
this.isCreatingAudio = false;
const isRemoved = !this.el.parentNode;
if (!stream || isRemoved) return;

APP.sourceType.set(this.el, SourceType.AVATAR_AUDIO_SOURCE);
const { audioType } = getCurrentAudioSettings(this.el);
const audioListener = this.el.sceneEl.audioListener;
const audio = new THREE.PositionalAudio(audioListener);
let audio;
if (audioType === AudioType.PannerNode) {
audio = new THREE.PositionalAudio(audioListener);
} else {
audio = new THREE.Audio(audioListener);
}

this.audioSystem.removeAudio(audio);
this.audioSystem.addAudio(SourceType.AVATAR_AUDIO_SOURCE, audio);
Expand All @@ -69,7 +78,6 @@ AFRAME.registerComponent("avatar-audio-source", {
this.el.emit("sound-source-set", { soundSource: destinationSource });

APP.audios.set(this.el, audio);
APP.sourceType.set(this.el, SourceType.AVATAR_AUDIO_SOURCE);
updateAudioSettings(this.el, audio);
},

Expand All @@ -82,6 +90,7 @@ AFRAME.registerComponent("avatar-audio-source", {

APP.audios.delete(this.el);
APP.sourceType.delete(this.el);
APP.supplementaryAttenuation.delete(this.el);
},

init() {
Expand All @@ -90,6 +99,17 @@ AFRAME.registerComponent("avatar-audio-source", {
// This could happen in case there is an ICE failure that requires a transport recreation.
APP.dialog.on("stream_updated", this._onStreamUpdated, this);
this.createAudio();

let audioOutputModePref = APP.store.state.preferences.audioOutputMode;
this.onPreferenceChanged = () => {
const newPref = APP.store.state.preferences.audioOutputMode;
const shouldRecreateAudio = audioOutputModePref !== newPref && !this.isCreatingAudio;
audioOutputModePref = newPref;
if (shouldRecreateAudio) {
this.createAudio();
}
};
APP.store.addEventListener("statechanged", this.onPreferenceChanged);
},

async _onStreamUpdated(peerId, kind) {
Expand Down Expand Up @@ -259,6 +279,7 @@ AFRAME.registerComponent("audio-target", {
},

createAudio: function() {
APP.supplementaryAttenuation.delete(this.el);
APP.sourceType.set(this.el, SourceType.AUDIO_TARGET);
const audioListener = this.el.sceneEl.audioListener;
let audio = null;
Expand Down Expand Up @@ -306,6 +327,7 @@ AFRAME.registerComponent("audio-target", {
this.audioSystem.removeAudio(this.audio);
this.el.removeObject3D(this.attrName);

APP.supplementaryAttenuation.delete(this.el);
APP.audios.delete(this.el);
APP.sourceType.delete(this.el);
}
Expand Down
12 changes: 6 additions & 6 deletions src/components/media-video.js
Original file line number Diff line number Diff line change
Expand Up @@ -163,17 +163,16 @@ AFRAME.registerComponent("media-video", {
evt.detail.cameraEl.getObject3D("camera").add(sceneEl.audioListener);
});

// TODO Probably we will get rid of this at some point
this.audioOutputModePref = window.APP.store.state.preferences.audioOutputMode;
let audioOutputModePref = APP.store.state.preferences.audioOutputMode;
this.onPreferenceChanged = () => {
const newPref = window.APP.store.state.preferences.audioOutputMode;
const shouldRecreateAudio = this.audioOutputModePref !== newPref && this.audio && this.mediaElementAudioSource;
this.audioOutputModePref = newPref;
const newPref = APP.store.state.preferences.audioOutputMode;
const shouldRecreateAudio = audioOutputModePref !== newPref && this.audio && this.mediaElementAudioSource;
audioOutputModePref = newPref;
if (shouldRecreateAudio) {
this.setupAudio();
}
};
window.APP.store.addEventListener("statechanged", this.onPreferenceChanged);
APP.store.addEventListener("statechanged", this.onPreferenceChanged);
},

isMineOrLocal() {
Expand Down Expand Up @@ -789,6 +788,7 @@ AFRAME.registerComponent("media-video", {
APP.gainMultipliers.delete(this.el);
APP.audios.delete(this.el);
APP.sourceType.delete(this.el);
APP.supplementaryAttenuation.delete(this.el);

if (this.audio) {
this.el.removeObject3D("sound");
Expand Down
1 change: 1 addition & 0 deletions src/hub.js
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ APP.zoneOverrides = new Map(); // el -> AudioSettings
APP.audioDebugPanelOverrides = new Map(); // SourceType -> AudioSettings
APP.sceneAudioDefaults = new Map(); // SourceType -> AudioSettings
APP.gainMultipliers = new Map(); // el -> Number
APP.supplementaryAttenuation = new Map(); // el -> Number
APP.clippingState = new Set();
APP.linkedMutedState = new Set();
APP.isAudioPaused = new Set();
Expand Down
9 changes: 9 additions & 0 deletions src/message-dispatch.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ export default class MessageDispatch extends EventTarget {
uiRoot = uiRoot || document.getElementById("ui-root");
const isGhost = !entered && uiRoot && uiRoot.firstChild && uiRoot.firstChild.classList.contains("isGhost");

// TODO: Some of the commands below should be available without requiring
// room entry. For example, audiomode should not require room entry.
if (!entered && (!isGhost || command === "duck")) {
this.log(LogMessageType.roomEntryRequired);
return;
Expand Down Expand Up @@ -174,8 +176,15 @@ export default class MessageDispatch extends EventTarget {
{
const shouldEnablePositionalAudio = window.APP.store.state.preferences.audioOutputMode === "audio";
window.APP.store.update({
// TODO: This should probably just be a boolean to disable panner node settings
// and even if it's not, "audio" is a weird name for the "audioOutputMode" that means
// "stereo" / "not panner".
preferences: { audioOutputMode: shouldEnablePositionalAudio ? "panner" : "audio" }
});
// TODO: The user message here is a little suspicious. We might be ignoring the
// user preference (e.g. if panner nodes are broken in safari, then we never create
// panner nodes, regardless of user preference.)
// Warning: This comment may be out of date when you read it.
this.log(
shouldEnablePositionalAudio ? LogMessageType.positionalAudioEnabled : LogMessageType.positionalAudioDisabled
);
Expand Down
18 changes: 15 additions & 3 deletions src/systems/audio-gain-system.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { CLIPPING_THRESHOLD_ENABLED, CLIPPING_THRESHOLD_DEFAULT } from "../react-components/preferences-screen";
import { updateAudioSettings } from "../update-audio-settings";
import { getCurrentAudioSettings, updateAudioSettings } from "../update-audio-settings";

function isClippingEnabled() {
const { enableAudioClipping } = window.APP.store.state.preferences;
Expand Down Expand Up @@ -36,20 +36,32 @@ const calculateAttenuation = (() => {
audio.panner.rolloffFactor,
audio.panner.refDistance,
audio.panner.maxDistance
// TODO: Why are coneInnerAngle, coneOuterAngle and coneOuterGain not used?
);
} else {
return 1.0;
const { distanceModel, rolloffFactor, refDistance, maxDistance } = getCurrentAudioSettings(el);
return distanceModels[distanceModel](distance, rolloffFactor, refDistance, maxDistance);
}
};
})();

// TODO: Rename "GainSystem" because the name is suspicious
export class GainSystem {
tick() {
const clippingEnabled = isClippingEnabled();
const clippingThreshold = getClippingThreshold();
for (const [el, audio] of APP.audios.entries()) {
const attenuation = calculateAttenuation(el, audio);

if (!audio.panner) {
// For Audios that are not PositionalAudios, we reintroduce
// distance-based attenuation manually.
APP.supplementaryAttenuation.set(el, attenuation);
updateAudioSettings(el, audio);
}

const isClipped = APP.clippingState.has(el);
const shouldBeClipped = clippingEnabled && calculateAttenuation(el, audio) < clippingThreshold;
const shouldBeClipped = clippingEnabled && attenuation < clippingThreshold;
if (isClipped !== shouldBeClipped) {
if (shouldBeClipped) {
APP.clippingState.add(el);
Expand Down
21 changes: 11 additions & 10 deletions src/systems/audio-settings-system.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,17 @@ export class AudioSettingsSystem {
constructor(sceneEl) {
sceneEl.addEventListener("reset_scene", this.onSceneReset);

// TODO: Remove these hacks
if (
!window.APP.store.state.preferences.audioOutputMode ||
window.APP.store.state.preferences.audioOutputMode === "audio"
) {
//hack to always reset to "panner"
window.APP.store.update({
preferences: { audioOutputMode: "panner" }
});
}
// HACK We are scared that users are going to set this preference and then
// forget about it and have a bad time, so we always remove the preference
// whenever the user refreshes the page.
// TODO: This is pretty weird and surprising. If the preference is exposed
// in the preference screen, then we would not be so scared about this.
// Also, if we feel so concerned about people using it, we should consider
// ways to make it safer or remove it.
window.APP.store.update({
preferences: { audioOutputMode: undefined }
});

if (window.APP.store.state.preferences.audioNormalization !== 0.0) {
//hack to always reset to 0.0 (disabled)
window.APP.store.update({
Expand Down
3 changes: 2 additions & 1 deletion src/systems/sound-effects-system.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import URL_MEDIA_LOADED from "../assets/sfx/A_bendUp.mp3";
import URL_MEDIA_LOADING from "../assets/sfx/suspense.mp3";
import URL_SPAWN_EMOJI from "../assets/sfx/emoji.mp3";
import { setMatrixWorld } from "../utils/three-utils";
import { isSafari } from "../utils/detect-safari";

let soundEnum = 0;
export const SOUND_HOVER_OR_GRAB = soundEnum++;
Expand Down Expand Up @@ -137,7 +138,7 @@ export class SoundEffectsSystem {
const audioBuffer = this.sounds.get(sound);
if (!audioBuffer) return null;

const disablePositionalAudio = window.APP.store.state.preferences.audioOutputMode === "audio";
const disablePositionalAudio = isSafari() || window.APP.store.state.preferences.audioOutputMode === "audio";
const positionalAudio = disablePositionalAudio
? new THREE.Audio(this.scene.audioListener)
: new THREE.PositionalAudio(this.scene.audioListener);
Expand Down
27 changes: 25 additions & 2 deletions src/update-audio-settings.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
import { SourceType, MediaAudioDefaults, AvatarAudioDefaults, TargetAudioDefaults } from "./components/audio-params";
import {
AudioType,
SourceType,
MediaAudioDefaults,
AvatarAudioDefaults,
TargetAudioDefaults
} from "./components/audio-params";
import { isSafari } from "./utils/detect-safari";

const defaultSettingsForSourceType = Object.freeze(
new Map([
Expand Down Expand Up @@ -28,14 +35,30 @@ export function getCurrentAudioSettings(el) {
const audioDebugPanelOverrides = APP.audioDebugPanelOverrides.get(sourceType);
const audioOverrides = APP.audioOverrides.get(el);
const zoneSettings = APP.zoneOverrides.get(el);
const settings = Object.assign({}, defaults, sceneOverrides, audioDebugPanelOverrides, audioOverrides, zoneSettings);
const preferencesOverrides =
APP.store.state.preferences.audioOutputMode === "audio" ? { audioType: AudioType.Stereo } : {};
const safariOverrides = isSafari() ? { audioType: AudioType.Stereo } : {};
const settings = Object.assign(
{},
defaults,
sceneOverrides,
audioDebugPanelOverrides,
audioOverrides,
zoneSettings,
preferencesOverrides,
safariOverrides
);

if (APP.clippingState.has(el) || APP.linkedMutedState.has(el)) {
settings.gain = 0;
} else if (APP.gainMultipliers.has(el)) {
settings.gain = settings.gain * APP.gainMultipliers.get(el);
}

if (APP.supplementaryAttenuation.has(el)) {
settings.gain = settings.gain * APP.supplementaryAttenuation.get(el);
}

return settings;
}

Expand Down
6 changes: 6 additions & 0 deletions src/utils/detect-safari.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { detect } from "detect-browser";

export function isSafari() {
const browser = detect();
return ["iOS", "Mac OS"].includes(browser.os) && ["safari", "ios"].includes(browser.name);
}