Skip to content

Commit

Permalink
fix: Attempt to fix setSinkId failures.
Browse files Browse the repository at this point in the history
  • Loading branch information
@hristoterezov
hristoterezov committed Aug 29, 2023
1 parent 1adbebf commit a7c1cce
Show file tree
Hide file tree
Showing 5 changed files with 102 additions and 65 deletions.
7 changes: 6 additions & 1 deletion modules/UI/videolayout/VideoContainer.js
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* global APP, interfaceConfig */

/* eslint-disable no-unused-vars */
import Logger from '@jitsi/logger';
import $ from 'jquery';
import React from 'react';
import ReactDOM from 'react-dom';
Expand All @@ -23,6 +24,8 @@ export const VIDEO_CONTAINER_TYPE = 'camera';
// Corresponds to animation duration from the animatedFadeIn and animatedFadeOut CSS classes.
const FADE_DURATION_MS = 300;

const logger = Logger.getLogger(__filename);

/**
* Returns an array of the video dimensions, so that it keeps it's aspect
* ratio and fits available area with it's larger dimension. This method
Expand Down Expand Up @@ -489,7 +492,9 @@ export class VideoContainer extends LargeContainer {
}

if (this.video) {
stream.attach(this.video);
stream.attach(this.video).catch(error => {
logger.error(`Attaching the remote track ${stream} has failed with `, error);
});

// Ensure large video gets play() called on it when a new stream is attached to it. This is necessary in the
// case of Safari as autoplay doesn't kick-in automatically on Safari 15 and newer versions.
Expand Down
42 changes: 21 additions & 21 deletions package-lock.json
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
"js-md5": "0.6.1",
"js-sha512": "0.8.0",
"jwt-decode": "2.2.0",
"lib-jitsi-meet": "https://github.com/jitsi/lib-jitsi-meet/releases/download/v1678.0.0+77e6803f/lib-jitsi-meet.tgz",
"lib-jitsi-meet": "https://github.com/jitsi/lib-jitsi-meet/releases/download/v1680.0.0+b760f4e1/lib-jitsi-meet.tgz",
"lodash": "4.17.21",
"moment": "2.29.4",
"moment-duration-format": "2.2.2",
Expand Down
81 changes: 49 additions & 32 deletions react/features/base/media/components/web/AudioTrack.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class AudioTrack extends Component<IProps> {
/**
* Reference to the HTML audio element, stored until the file is ready.
*/
_ref: HTMLAudioElement | null;
_ref: React.RefObject<HTMLAudioElement>;

/**
* The current timeout ID for play() retries.
Expand Down Expand Up @@ -80,7 +80,7 @@ class AudioTrack extends Component<IProps> {

// Bind event handlers so they are only bound once for every instance.
this._errorHandler = this._errorHandler.bind(this);
this._setRef = this._setRef.bind(this);
this._ref = React.createRef();
this._play = this._play.bind(this);
}

Expand All @@ -94,19 +94,22 @@ class AudioTrack extends Component<IProps> {
componentDidMount() {
this._attachTrack(this.props.audioTrack);

if (this._ref) {
if (this._ref?.current) {
const audio = this._ref?.current;
const { _muted, _volume } = this.props;

if (typeof _volume === 'number') {
this._ref.volume = _volume;
audio.volume = _volume;
}

if (typeof _muted === 'boolean') {
this._ref.muted = _muted;
audio.muted = _muted;
}

// @ts-ignore
this._ref.addEventListener('error', this._errorHandler);
audio.addEventListener('error', this._errorHandler);
} else { // This should never happen
logger.error(`The react reference is null for AudioTrack ${this.props?.id}`);
}
}

Expand All @@ -121,7 +124,7 @@ class AudioTrack extends Component<IProps> {
this._detachTrack(this.props.audioTrack);

// @ts-ignore
this._ref?.removeEventListener('error', this._errorHandler);
this._ref?.current?.removeEventListener('error', this._errorHandler);
}

/**
Expand All @@ -141,19 +144,25 @@ class AudioTrack extends Component<IProps> {
this._attachTrack(nextProps.audioTrack);
}

if (this._ref) {
const currentVolume = this._ref.volume;
if (this._ref?.current) {
const audio = this._ref?.current;
const currentVolume = audio.volume;
const nextVolume = nextProps._volume;

if (typeof nextVolume === 'number' && !isNaN(nextVolume) && currentVolume !== nextVolume) {
this._ref.volume = nextVolume;
if (nextVolume === 0) {
logger.debug(`Setting audio element ${nextProps?.id} volume to 0`);
}
audio.volume = nextVolume;
}

const currentMuted = this._ref.muted;
const currentMuted = audio.muted;
const nextMuted = nextProps._muted;

if (typeof nextMuted === 'boolean' && currentMuted !== nextMuted) {
this._ref.muted = nextMuted;
logger.debug(`Setting audio element ${nextProps?.id} muted to true`);

audio.muted = nextMuted;
}
}

Expand All @@ -173,7 +182,7 @@ class AudioTrack extends Component<IProps> {
<audio
autoPlay = { autoPlay }
id = { id }
ref = { this._setRef } />
ref = { this._ref } />
);
}

Expand All @@ -185,12 +194,29 @@ class AudioTrack extends Component<IProps> {
* @returns {void}
*/
_attachTrack(track?: ITrack) {
const { id } = this.props;

if (!track?.jitsiTrack) {
logger.warn(`Attach is called on audio element ${id} without tracks passed!`);

return;
}

if (!this._ref?.current) {
logger.warn(`Attempting to attach track ${track?.jitsiTrack} on AudioTrack ${id} without reference!`);

return;
}

track.jitsiTrack.attach(this._ref);
this._play();
track.jitsiTrack.attach(this._ref.current)
.catch((error: Error) => {
logger.error(
`Attaching the remote track ${track.jitsiTrack} to video with id ${id} has failed with `,
error);
})
.finally(() => {
this._play();
});
}

/**
Expand All @@ -202,10 +228,10 @@ class AudioTrack extends Component<IProps> {
* @returns {void}
*/
_detachTrack(track?: ITrack) {
if (this._ref && track && track.jitsiTrack) {
if (this._ref?.current && track && track.jitsiTrack) {
clearTimeout(this._playTimeout);
this._playTimeout = undefined;
track.jitsiTrack.detach(this._ref);
track.jitsiTrack.detach(this._ref.current);
}
}

Expand All @@ -229,18 +255,20 @@ class AudioTrack extends Component<IProps> {
* @returns {void}
*/
_play(retries = 0) {
if (!this._ref) {
const { autoPlay, id } = this.props;

if (!this._ref?.current) {
// nothing to play.
logger.warn(`Attempting to call play on AudioTrack ${id} without reference!`);

return;
}
const { autoPlay, id } = this.props;

if (autoPlay) {
// Ensure the audio gets play() called on it. This may be necessary in the
// case where the local video container was moved and re-attached, in which
// case the audio may not autoplay.
this._ref.play()
this._ref.current.play()
.then(() => {
if (retries !== 0) {
// success after some failures
Expand All @@ -249,7 +277,7 @@ class AudioTrack extends Component<IProps> {
logger.info(`Successfully played audio track! retries: ${retries}`);
}
}, e => {
logger.error(`Failed to play audio track! retry: ${retries} ; Error: ${e}`);
logger.error(`Failed to play audio track on audio element ${id}! retry: ${retries} ; Error:`, e);

if (retries < 3) {
this._playTimeout = window.setTimeout(() => this._play(retries + 1), 1000);
Expand All @@ -264,17 +292,6 @@ class AudioTrack extends Component<IProps> {
});
}
}

/**
* Sets the reference to the HTML audio element.
*
* @param {HTMLAudioElement} audioElement - The HTML audio element instance.
* @private
* @returns {void}
*/
_setRef(audioElement: HTMLAudioElement | null) {
this._ref = audioElement;
}
}

/**
Expand Down
35 changes: 25 additions & 10 deletions react/features/base/media/components/web/Video.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React, { Component, ReactEventHandler } from 'react';

import { ITrack } from '../../../tracks/types';
import logger from '../../logger';

/**
* The type of the React {@code Component} props of {@link Video}.
Expand Down Expand Up @@ -227,21 +228,22 @@ class Video extends Component<IProps> {
this._videoElement.onplaying = this._onVideoPlaying;
}

this._attachTrack(this.props.videoTrack);
this._attachTrack(this.props.videoTrack).finally(() => {
if (this._videoElement && this.props.autoPlay) {
// Ensure the video gets play() called on it. This may be necessary in the
// case where the local video container was moved and re-attached, in which
// case video does not autoplay.

if (this._videoElement && this.props.autoPlay) {
// Ensure the video gets play() called on it. This may be necessary in the
// case where the local video container was moved and re-attached, in which
// case video does not autoplay.
this._videoElement.play()
this._videoElement.play()
.catch(error => {
// Prevent uncaught "DOMException: The play() request was interrupted by a new load request"
// when video playback takes long to start and it starts after the component was unmounted.
if (this._mounted) {
throw error;
}
});
}
}
});
}

/**
Expand Down Expand Up @@ -271,7 +273,9 @@ class Video extends Component<IProps> {

if (currentJitsiTrack !== nextJitsiTrack) {
this._detachTrack(this.props.videoTrack);
this._attachTrack(nextProps.videoTrack);
this._attachTrack(nextProps.videoTrack).catch((_error: Error) => {
// Ignore the error. We are already logging it.
});
}

if (this.props.style !== nextProps.style || this.props.className !== nextProps.className) {
Expand Down Expand Up @@ -321,11 +325,22 @@ class Video extends Component<IProps> {
* @returns {void}
*/
_attachTrack(videoTrack?: Partial<ITrack>) {
const { id } = this.props;

if (!videoTrack?.jitsiTrack) {
return;
logger.warn(`Attach is called on video element ${id} without tracks passed!`);

// returning Promise.resolve just keep the previous logic.
// TODO: Check if it make sense to call play on this element or we can just return promise.reject().
return Promise.resolve();
}

videoTrack.jitsiTrack.attach(this._videoElement);
return videoTrack.jitsiTrack.attach(this._videoElement)
.catch((error: Error) => {
logger.error(
`Attaching the remote track ${videoTrack.jitsiTrack} to video with id ${id} has failed with `,
error);
});
}

/**
Expand Down

0 comments on commit a7c1cce

Please sign in to comment.