From e80c59b9856b524e2971ac558255425f487bd0aa Mon Sep 17 00:00:00 2001 From: Ryan Bliss Date: Tue, 24 Sep 2024 15:09:40 -0700 Subject: [PATCH] `maxPlaybackDrift` and `positionUpdateInterval` scaling by audience size (#796) --- .../src/LiveMediaSessionCoordinator.ts | 106 ++++++++++++++++-- .../src/internals/GroupCoordinatorState.ts | 3 +- .../src/internals/GroupPlaybackPosition.ts | 8 +- .../src/internals/PriorityTimeInterval.ts | 76 +++++++++++++ .../src/test/GroupPlaybackPosition.spec.ts | 23 ++-- .../src/test/PriorityTimeInterval.spec.ts | 77 +++++++++++++ packages/live-share/src/TimeInterval.ts | 12 +- 7 files changed, 273 insertions(+), 32 deletions(-) create mode 100644 packages/live-share-media/src/internals/PriorityTimeInterval.ts create mode 100644 packages/live-share-media/src/test/PriorityTimeInterval.spec.ts diff --git a/packages/live-share-media/src/LiveMediaSessionCoordinator.ts b/packages/live-share-media/src/LiveMediaSessionCoordinator.ts index 261c44e96..3fa6e8bf5 100644 --- a/packages/live-share-media/src/LiveMediaSessionCoordinator.ts +++ b/packages/live-share-media/src/LiveMediaSessionCoordinator.ts @@ -40,7 +40,7 @@ import { TrackMetadataNotSetError, ActionBlockedError, } from "./internals/errors.js"; - +import { PriorityTimeInterval } from "./internals/PriorityTimeInterval.js"; import { LiveMediaSessionCoordinatorSuspension } from "./LiveMediaSessionCoordinatorSuspension.js"; import { TypedEventEmitter } from "@fluid-internal/client-utils"; import { IEvent } from "@fluidframework/core-interfaces"; @@ -97,8 +97,27 @@ export class LiveMediaSessionCoordinator extends TypedEventEmitter IMediaPlayerState; - private _positionUpdateInterval = new TimeInterval(2000); - private _maxPlaybackDrift = new TimeInterval(1000); + private _positionUpdateInterval = new PriorityTimeInterval( + 2000, + // Scale by function + () => { + const audience = this._liveRuntime.audience; + if (!audience) return 1; + // As the audience size gets bigger, we relax the update interval, since more variance is expected + const count = audience.getMembers().size; + return 1 + count / 5; + } + ); + private _maxPlaybackDrift = new PriorityTimeInterval( + 1500, // Scale by function + () => { + const audience = this._liveRuntime.audience; + if (!audience) return 1; + // As the audience size gets bigger, we relax the playback drift, since more variance is expected + const count = audience.getMembers().size; + return 1 + count / 25; + } + ); private _lastWaitPoint?: CoordinationWaitPoint; private initializeState: LiveDataObjectInitializeState = LiveDataObjectInitializeState.needed; @@ -222,13 +241,29 @@ export class LiveMediaSessionCoordinator extends TypedEventEmitter { LiveDataObjectNotInitializedError.assert( "LiveMediaSessionCoordinator:setPlaybackRate", @@ -400,6 +474,10 @@ export class LiveMediaSessionCoordinator extends TypedEventEmitter IMediaPlayerState ) { super(); diff --git a/packages/live-share-media/src/internals/GroupPlaybackPosition.ts b/packages/live-share-media/src/internals/GroupPlaybackPosition.ts index 0671f04d0..de8e10b70 100644 --- a/packages/live-share-media/src/internals/GroupPlaybackPosition.ts +++ b/packages/live-share-media/src/internals/GroupPlaybackPosition.ts @@ -3,7 +3,6 @@ * Licensed under the Microsoft Live Share SDK License. */ -import { TimeInterval } from "@microsoft/live-share"; import { IRuntimeSignaler, LiveShareRuntime, @@ -15,6 +14,7 @@ import { ExtendedMediaSessionPlaybackState, } from "../MediaSessionExtensions.js"; import { GroupPlaybackRate } from "./GroupPlaybackRate.js"; +import { PriorityTimeInterval } from "./PriorityTimeInterval.js"; /** * Per client position @@ -37,7 +37,7 @@ export class GroupPlaybackPosition { private _playbackRate: GroupPlaybackRate; private _runtime: IRuntimeSignaler; private _liveRuntime: LiveShareRuntime; - private _updateInterval: TimeInterval; + private _updateInterval: PriorityTimeInterval; private _positions: Map; constructor( @@ -45,7 +45,7 @@ export class GroupPlaybackPosition { playbackRate: GroupPlaybackRate, runtime: IRuntimeSignaler, liveRuntime: LiveShareRuntime, - updateInterval: TimeInterval + updateInterval: PriorityTimeInterval ) { this._transportState = transportState; this._playbackRate = playbackRate; @@ -171,7 +171,7 @@ export class GroupPlaybackPosition { ) => void ): void { const now = this._liveRuntime.getTimestamp(); - const ignoreBefore = now - this._updateInterval.milliseconds * 2; + const ignoreBefore = now - this._updateInterval.maxMilliseconds * 2; const shouldProject = !this._transportState.track.metadata?.liveStream; this._positions.forEach((position, _) => { // Ignore any old updates diff --git a/packages/live-share-media/src/internals/PriorityTimeInterval.ts b/packages/live-share-media/src/internals/PriorityTimeInterval.ts new file mode 100644 index 000000000..49b48d7e3 --- /dev/null +++ b/packages/live-share-media/src/internals/PriorityTimeInterval.ts @@ -0,0 +1,76 @@ +import { TimeInterval } from "@microsoft/live-share"; + +function getScaledPriorityTimeMs( + minMilliseconds: number, + hasPriority: boolean, + shouldPrioritize: boolean, + scaleBy: number +): number { + if (!shouldPrioritize) return minMilliseconds; + if (hasPriority) return minMilliseconds; + return minMilliseconds * scaleBy; +} + +/** + * @hidden + * Time interval that can scale based on a scaling function. + */ +export class PriorityTimeInterval extends TimeInterval { + /** + * If true, local user has priority and will have the lowest possible millesecond value. + */ + public hasPriority: boolean; + /** + * If true, milliseconds will be scaled when {@link hasPriority} is false. + */ + public shouldPrioritize: boolean; + /** + * Function to get the scale ratio. + */ + private getScaleBy: () => number; + /** + * Time interval that can scale based on a scaling function. + * + * @param defaultMilliseconds the default minimum milliseconds + * @param getScaleByFn a function to scale the milliseconds by when {@link hasPriority} is false and {@link shouldPrioritize} is true. + * @param defaultHasPriority the default {@link hasPriority} value + * @param defaultShouldPrioritize the default {@link shouldPrioritize} value + */ + constructor( + defaultMilliseconds: number, + getScaleByFn: () => number, + defaultHasPriority: boolean = false, + defaultShouldPrioritize: boolean = true + ) { + super(defaultMilliseconds); + this.hasPriority = defaultHasPriority; + this.shouldPrioritize = defaultShouldPrioritize; + this.getScaleBy = getScaleByFn; + } + + public get milliseconds(): number { + return getScaledPriorityTimeMs( + this._milliseconds, + this.hasPriority, + this.shouldPrioritize, + this.getScaleBy() + ); + } + + public set milliseconds(value: number) { + this._milliseconds = value; + } + + public get minMilliseconds(): number { + return this._milliseconds; + } + + public get maxMilliseconds(): number { + return getScaledPriorityTimeMs( + this._milliseconds, + false, + this.shouldPrioritize, + this.getScaleBy() + ); + } +} diff --git a/packages/live-share-media/src/test/GroupPlaybackPosition.spec.ts b/packages/live-share-media/src/test/GroupPlaybackPosition.spec.ts index 6dc73dd2e..f2e8a9006 100644 --- a/packages/live-share-media/src/test/GroupPlaybackPosition.spec.ts +++ b/packages/live-share-media/src/test/GroupPlaybackPosition.spec.ts @@ -19,7 +19,7 @@ import { GroupTransportState, ITransportState, } from "../internals/GroupTransportState"; -import { TestLiveShareHost, TimeInterval } from "@microsoft/live-share"; +import { TestLiveShareHost } from "@microsoft/live-share"; import { IRuntimeSignaler, LiveShareRuntime, @@ -27,6 +27,7 @@ import { } from "@microsoft/live-share/internal"; import { IMediaPlayerState } from "../LiveMediaSessionCoordinator"; import { GroupPlaybackTrack } from "../internals/GroupPlaybackTrack"; +import { PriorityTimeInterval } from "../internals/PriorityTimeInterval"; function createTransportUpdate( runtime: IRuntimeSignaler, @@ -103,7 +104,7 @@ async function getObjects(updateInterval: number = 10000) { async function getPlayBackPosition( liveRuntime: MockLiveShareRuntime, runtime1: IRuntimeSignaler, - updateInterval = new TimeInterval(10) + updateInterval = new PriorityTimeInterval(10, () => 1) ) { const track1 = { trackIdentifier: "track1", @@ -288,7 +289,7 @@ describe("GroupPlaybackPosition", () => { }); it("should enumerate client positions", async () => { - const updateInterval = new TimeInterval(10); + const updateInterval = new PriorityTimeInterval(10, () => 1); const { liveRuntime, runtime1, runtime2, dispose } = await getObjects(); const { playbackPosition } = await getPlayBackPosition( liveRuntime, @@ -327,7 +328,7 @@ describe("GroupPlaybackPosition", () => { }); it("should ignore stale positions", async () => { - const updateInterval = new TimeInterval(10); + const updateInterval = new PriorityTimeInterval(10, () => 1); const { liveRuntime, runtime1, runtime2, dispose } = await getObjects(); const { playbackPosition } = await getPlayBackPosition( liveRuntime, @@ -370,7 +371,7 @@ describe("GroupPlaybackPosition", () => { const { playbackPosition } = await getPlayBackPosition( liveRuntime, runtime1, - new TimeInterval(1000) + new PriorityTimeInterval(1000, () => 1) ); try { @@ -405,7 +406,7 @@ describe("GroupPlaybackPosition", () => { const { playbackPosition, transportState } = await getPlayBackPosition( liveRuntime, runtime1, - new TimeInterval(1000) + new PriorityTimeInterval(1000, () => 1) ); try { @@ -436,7 +437,7 @@ describe("GroupPlaybackPosition", () => { const { playbackPosition, transportState } = await getPlayBackPosition( liveRuntime, runtime1, - new TimeInterval(1000) + new PriorityTimeInterval(1000, () => 1) ); try { @@ -478,7 +479,7 @@ describe("GroupPlaybackPosition", () => { const { playbackPosition, transportState } = await getPlayBackPosition( liveRuntime, runtime1, - new TimeInterval(1000) + new PriorityTimeInterval(1000, () => 1) ); try { @@ -531,7 +532,7 @@ describe("GroupPlaybackPosition", () => { const { playbackPosition, transportState } = await getPlayBackPosition( liveRuntime, runtime1, - new TimeInterval(1000) + new PriorityTimeInterval(1000, () => 1) ); try { @@ -574,7 +575,7 @@ describe("GroupPlaybackPosition", () => { const { playbackPosition, transportState } = await getPlayBackPosition( liveRuntime, runtime1, - new TimeInterval(1000) + new PriorityTimeInterval(1000, () => 1) ); try { @@ -610,7 +611,7 @@ describe("GroupPlaybackPosition", () => { const { playbackPosition, transportState } = await getPlayBackPosition( liveRuntime, runtime1, - new TimeInterval(1000) + new PriorityTimeInterval(1000, () => 1) ); try { diff --git a/packages/live-share-media/src/test/PriorityTimeInterval.spec.ts b/packages/live-share-media/src/test/PriorityTimeInterval.spec.ts new file mode 100644 index 000000000..f5658fe1c --- /dev/null +++ b/packages/live-share-media/src/test/PriorityTimeInterval.spec.ts @@ -0,0 +1,77 @@ +/** + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the Microsoft Live Share SDK License. + */ + +import "mocha"; +import { strict as assert } from "assert"; +import { PriorityTimeInterval } from "../internals/PriorityTimeInterval"; + +describe("PriorityTimeInterval", () => { + it("Should scale properly with default values in constructor", () => { + const interval = new PriorityTimeInterval(1000, () => 2); + assert(interval.milliseconds === 2000); + assert(interval.seconds === 2); + }); + + it("Should not scale when starting with priority", () => { + const interval = new PriorityTimeInterval(1000, () => 2, true); + assert(interval.milliseconds === 1000); + assert(interval.seconds === 1); + }); + + it("Should not scale when user does not have priority but scaling is disabled", () => { + const interval = new PriorityTimeInterval(1000, () => 2, false, false); + assert(interval.milliseconds === 1000); + assert(interval.seconds === 1); + }); + + it("Should scale properly after changing values", () => { + const interval = new PriorityTimeInterval(1000, () => 2); + assert(interval.milliseconds === 2000); + assert(interval.seconds === 2); + // Changing settings will break ts checks, since we asserted above that milliseconds was 2000 and ts can't figure out relationship. + // We ignore ts for lines where this occurs. + interval.hasPriority = true; + + // @ts-ignore-next + assert(interval.milliseconds === 1000); + // @ts-ignore-next + assert(interval.seconds === 1); + + interval.hasPriority = false; + + // @ts-ignore-next + assert(interval.milliseconds === 2000); + // @ts-ignore-next + assert(interval.seconds === 2); + + interval.shouldPrioritize = false; + + // @ts-ignore-next + assert(interval.milliseconds === 1000); + // @ts-ignore-next + assert(interval.seconds === 1); + + interval.shouldPrioritize = true; + + // @ts-ignore-next + assert(interval.milliseconds === 2000); + // @ts-ignore-next + assert(interval.seconds === 2); + }); + + it("Should dynamically scale depending on scaleBy func response", () => { + let scaleBy = 2; + const interval = new PriorityTimeInterval(1000, () => scaleBy); + assert(interval.milliseconds === 2000); + assert(interval.seconds === 2); + // Changing scaleBy will break ts checks, since we asserted above that milliseconds was 2000 and ts can't figure out relationship. + // We ignore ts for lines where this occurs. + scaleBy = 3; + // @ts-ignore-next + assert(interval.milliseconds === 3000); + // @ts-ignore-next + assert(interval.seconds === 3); + }); +}); diff --git a/packages/live-share/src/TimeInterval.ts b/packages/live-share/src/TimeInterval.ts index 888347faf..815569bd2 100644 --- a/packages/live-share/src/TimeInterval.ts +++ b/packages/live-share/src/TimeInterval.ts @@ -8,25 +8,25 @@ * Manages converting time intervals from seconds-to-milliseconds and vice versa. */ export class TimeInterval { - private _value: number; + protected _milliseconds: number; constructor(defaultMilliseconds: number) { - this._value = defaultMilliseconds; + this._milliseconds = defaultMilliseconds; } public get milliseconds(): number { - return this._value; + return this._milliseconds; } public set milliseconds(value: number) { - this._value = value; + this._milliseconds = value; } public get seconds(): number { - return this._value / 1000; + return this.milliseconds / 1000; } public set seconds(value: number) { - this._value = Math.floor(value * 1000); + this.milliseconds = Math.floor(value * 1000); } }