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 issue with older browsers erroring upon setCodecPreferences #418

Merged
merged 2 commits into from
Aug 29, 2022
Merged
Changes from 1 commit
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
Next Next commit
Fix issue with older browsers erroring upon setCodecPreferences
On Chrome 96, using setCodecPreferences in the way that we do causes
the browser to fail on setLocalDescription, even for the offer that it
had created.
davidzhao committed Aug 28, 2022
commit 9445fb3cf3d0dde13e8a872b4f091eabbe84bb26
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
@@ -40,6 +40,7 @@
"sdp-transform": "^2.14.1",
"ts-debounce": "^4.0.0",
"typed-emitter": "^2.1.0",
"ua-parser-js": "^1.0.2",
"webrtc-adapter": "^8.1.1"
},
"devDependencies": {
@@ -53,6 +54,7 @@
"@rollup/plugin-node-resolve": "13.3.0",
"@types/jest": "28.1.7",
"@types/sdp-transform": "2.4.5",
"@types/ua-parser-js": "^0.7.36",
"@types/ws": "8.5.3",
"@typescript-eslint/eslint-plugin": "5.33.1",
"@typescript-eslint/parser": "5.33.1",
69 changes: 44 additions & 25 deletions src/room/PCTransport.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { debounce } from 'ts-debounce';
import { MediaDescription, parse, write } from 'sdp-transform';
import { debounce } from 'ts-debounce';
import log from '../logger';
import { NegotiationError } from './errors';

/** @internal */
interface TrackBitrateInfo {
@@ -54,8 +55,16 @@ export default class PCTransport {
}

// debounced negotiate interface
negotiate = debounce(() => {
this.createAndSendOffer();
negotiate = debounce((onError?: (e: Error) => void) => {
try {
this.createAndSendOffer();
} catch (e) {
if (onError) {
onError(e as Error);
} else {
throw e;
}
}
}, 100);

async createAndSendOffer(options?: RTCOfferOptions) {
@@ -135,17 +144,8 @@ export default class PCTransport {
});

this.trackBitrates = [];
const originalSdp = offer.sdp;
try {
offer.sdp = write(sdpParsed);
await this.pc.setLocalDescription(offer);
} catch (e: unknown) {
log.warn('not able to set desired local description, falling back to unmodified offer', {
error: e,
});
offer.sdp = originalSdp;
await this.pc.setLocalDescription(offer);
}

await this.setMungedLocalDescription(offer, write(sdpParsed));
this.onOffer(offer);
}

@@ -157,17 +157,7 @@ export default class PCTransport {
ensureAudioNack(media);
}
});
const originalSdp = answer.sdp;
try {
answer.sdp = write(sdpParsed);
await this.pc.setLocalDescription(answer);
} catch (e: unknown) {
log.warn('not able to set desired local description, falling back to unmodified answer', {
error: e,
});
answer.sdp = originalSdp;
await this.pc.setLocalDescription(answer);
}
await this.setMungedLocalDescription(answer, write(sdpParsed));
return answer;
}

@@ -182,6 +172,35 @@ export default class PCTransport {
close() {
this.pc.close();
}

private async setMungedLocalDescription(sd: RTCSessionDescriptionInit, munged: string) {
const originalSdp = sd.sdp;
sd.sdp = munged;
try {
log.debug('setting munged local description');
await this.pc.setLocalDescription(sd);
return;
} catch (e) {
log.warn(`not able to set ${sd.type}, falling back to unmodified sdp`, {
error: e,
});
sd.sdp = originalSdp;
}

try {
await this.pc.setLocalDescription(sd);
} catch (e) {
// this error cannot always be caught.
// If the local description has a setCodecPreferences error, this error will be uncaught
let msg = 'unknown error';
if (e instanceof Error) {
msg = e.message;
} else if (typeof e === 'string') {
msg = e;
}
throw new NegotiationError(msg);
}
}
}

function ensureAudioNack(
24 changes: 20 additions & 4 deletions src/room/RTCEngine.ts
Original file line number Diff line number Diff line change
@@ -21,7 +21,12 @@ import {
TrackPublishedResponse,
} from '../proto/livekit_rtc';
import DefaultReconnectPolicy from './DefaultReconnectPolicy';
import { ConnectionError, TrackInvalidError, UnexpectedConnectionState } from './errors';
import {
ConnectionError,
NegotiationError,
TrackInvalidError,
UnexpectedConnectionState,
} from './errors';
import { EngineEvent } from './events';
import PCTransport from './PCTransport';
import type { ReconnectContext, ReconnectPolicy } from './ReconnectPolicy';
@@ -30,7 +35,13 @@ import type LocalVideoTrack from './track/LocalVideoTrack';
import type { SimulcastTrackInfo } from './track/LocalVideoTrack';
import type { TrackPublishOptions, VideoCodec } from './track/options';
import { Track } from './track/Track';
import { isWeb, sleep, supportsAddTrack, supportsTransceiver } from './utils';
import {
isWeb,
sleep,
supportsAddTrack,
supportsSetCodecPreferences,
supportsTransceiver,
} from './utils';

const lossyDataChannel = '_lossy';
const reliableDataChannel = '_reliable';
@@ -519,7 +530,7 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit
matched.push(c);
});

if ('setCodecPreferences' in transceiver) {
if (supportsSetCodecPreferences(transceiver)) {
transceiver.setCodecPreferences(matched.concat(partialMatched, unmatched));
}
}
@@ -900,7 +911,12 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit

this.hasPublished = true;

this.publisher.negotiate();
this.publisher.negotiate((e) => {
if (e instanceof NegotiationError) {
this.fullReconnectOnNext = true;
}
this.handleDisconnect('negotiation');
});
}

dataChannelForKind(kind: DataPacket_Kind, sub?: boolean): RTCDataChannel | undefined {
14 changes: 10 additions & 4 deletions src/room/errors.ts
Original file line number Diff line number Diff line change
@@ -15,25 +15,31 @@ export class ConnectionError extends LivekitError {

export class TrackInvalidError extends LivekitError {
constructor(message?: string) {
super(20, message || 'Track is invalid');
super(20, message ?? 'track is invalid');
}
}

export class UnsupportedServer extends LivekitError {
constructor(message?: string) {
super(10, message || 'Unsupported server');
super(10, message ?? 'unsupported server');
}
}

export class UnexpectedConnectionState extends LivekitError {
constructor(message?: string) {
super(12, message || 'Unexpected connection state');
super(12, message ?? 'unexpected connection state');
}
}

export class NegotiationError extends LivekitError {
constructor(message?: string) {
super(13, message ?? 'unable to negotiate');
}
}

export class PublishDataError extends LivekitError {
constructor(message?: string) {
super(13, message || 'Unable to publish data');
super(13, message ?? 'unable to publish data');
}
}

42 changes: 42 additions & 0 deletions src/room/utils.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import UAParser from 'ua-parser-js';
import { ClientInfo, ClientInfo_SDK } from '../proto/livekit_models';
import { protocolVersion, version } from '../version';

@@ -33,6 +34,34 @@ export function supportsDynacast() {
return supportsTransceiver();
}

const setCodecPreferencesVersions: { [key: string]: string } = {
Chrome: '100',
Chromium: '100',
Safari: '15',
Firefox: '100',
Edge: '100',
Brave: '1.40',
};

export function supportsSetCodecPreferences(transceiver: RTCRtpTransceiver): boolean {
if (!isWeb()) {
return false;
}
if (!('setCodecPreferences' in transceiver)) {
return false;
}
const uap = UAParser();
if (!uap.browser.name || !uap.browser.version) {
// version is required
return false;
}
const v = setCodecPreferencesVersions[uap.browser.name];
if (v) {
return compareVersions(uap.browser.version, v) >= 0;
}
return false;
}

export function isBrowserSupported() {
return supportsTransceiver() || supportsAddTrack();
}
@@ -56,6 +85,19 @@ export function isWeb(): boolean {
return typeof document !== 'undefined';
}

export function compareVersions(v1: string, v2: string): number {
const parts1 = v1.split('.');
const parts2 = v2.split('.');
const k = Math.min(v1.length, v2.length);
for (let i = 0; i < k; ++i) {
const p1 = parseInt(parts1[i], 10);
const p2 = parseInt(parts2[i], 10);
if (p1 > p2) return 1;
if (p1 < p2) return -1;
}
return parts1.length == parts2.length ? 0 : parts1.length < parts2.length ? -1 : 1;
}

function roDispatchCallback(entries: ResizeObserverEntry[]) {
for (const entry of entries) {
(entry.target as ObservableMediaElement).handleResize(entry);
10 changes: 10 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
@@ -2196,6 +2196,11 @@
resolved "https://registry.yarnpkg.com/@types/stack-utils/-/stack-utils-2.0.1.tgz#20f18294f797f2209b5f65c8e3b5c8e8261d127c"
integrity sha512-Hl219/BT5fLAaz6NDkSuhzasy49dwQS/DSdu4MdggFB8zcXv7vflBI3xp7FEmkmdDkBUI2bPUNeMttp2knYdxw==

"@types/ua-parser-js@^0.7.36":
version "0.7.36"
resolved "https://registry.yarnpkg.com/@types/ua-parser-js/-/ua-parser-js-0.7.36.tgz#9bd0b47f26b5a3151be21ba4ce9f5fa457c5f190"
integrity sha512-N1rW+njavs70y2cApeIw1vLMYXRwfBy+7trgavGuuTfOd7j1Yh7QTRc/yqsPl6ncokt72ZXuxEU0PiCp9bSwNQ==

"@types/ws@8.5.3":
version "8.5.3"
resolved "https://registry.yarnpkg.com/@types/ws/-/ws-8.5.3.tgz#7d25a1ffbecd3c4f2d35068d0b283c037003274d"
@@ -7047,6 +7052,11 @@ typescript@4.7.4:
resolved "https://registry.yarnpkg.com/typescript/-/typescript-4.7.4.tgz#1a88596d1cf47d59507a1bcdfb5b9dfe4d488235"
integrity sha512-C0WQT0gezHuw6AdY1M2jxUO83Rjf0HP7Sk1DtXj6j1EwkQNZrHAg2XPWlq62oqEhYvONq5pkC2Y9oPljWToLmQ==

ua-parser-js@^1.0.2:
version "1.0.2"
resolved "https://registry.yarnpkg.com/ua-parser-js/-/ua-parser-js-1.0.2.tgz#e2976c34dbfb30b15d2c300b2a53eac87c57a775"
integrity sha512-00y/AXhx0/SsnI51fTc0rLRmafiGOM4/O+ny10Ps7f+j/b8p/ZY11ytMgznXkOVo4GQ+KwQG5UQLkLGirsACRg==

unbox-primitive@^1.0.1:
version "1.0.1"
resolved "https://registry.yarnpkg.com/unbox-primitive/-/unbox-primitive-1.0.1.tgz#085e215625ec3162574dc8859abee78a59b14471"