From bbec650131b09624355c17fc344b525e631e325e Mon Sep 17 00:00:00 2001 From: Michelle Zhang <56095982+michellewzhang@users.noreply.github.com> Date: Wed, 27 Mar 2024 14:23:09 -0700 Subject: [PATCH] ref(replay): only preload mobile replay videos when needed (#67657) - Optimize loading of videos for mobile replays - This PR modifies the `_videos` array by converting it into a dictionary that maps segment index to the video element. This makes it easier to only load videos when requested, rather than preloading them all at once. - Also fixes a bug where `currentIndex` was consistently undefined in the `createVideo` function, but it looks like it's working as expected now - Closes https://github.com/getsentry/sentry/issues/67430 --- .../components/replays/videoReplayer.spec.tsx | 107 ++++++++++++++++++ .../app/components/replays/videoReplayer.tsx | 70 ++++++++++-- 2 files changed, 166 insertions(+), 11 deletions(-) diff --git a/static/app/components/replays/videoReplayer.spec.tsx b/static/app/components/replays/videoReplayer.spec.tsx index 9834e04a9f5745..cc8c8bd735ea40 100644 --- a/static/app/components/replays/videoReplayer.spec.tsx +++ b/static/app/components/replays/videoReplayer.spec.tsx @@ -49,6 +49,32 @@ describe('VideoReplayer - no starting gap', () => { }, ]; + const extra = [ + { + id: 6, + timestamp: 40_002, + duration: 5000, + }, + { + id: 7, + timestamp: 45_002, + duration: 5000, + }, + ]; + + const skip = [ + { + id: 7, + timestamp: 45_002, + duration: 5000, + }, + { + id: 8, + timestamp: 50_002, + duration: 5000, + }, + ]; + it('plays and seeks inside of a segment', async () => { const root = document.createElement('div'); const inst = new VideoReplayer(attachments, { @@ -116,6 +142,87 @@ describe('VideoReplayer - no starting gap', () => { // @ts-expect-error private expect(inst.getVideo(inst._currentIndex)?.currentTime).toEqual(5); }); + + it('initially only loads videos from 0 to BUFFER', async () => { + const root = document.createElement('div'); + const inst = new VideoReplayer(attachments, { + videoApiPrefix: '/foo/', + root, + start: 0, + onFinished: jest.fn(), + onLoaded: jest.fn(), + }); + const playPromise = inst.play(0); + jest.advanceTimersByTime(2500); + await playPromise; + // @ts-expect-error private + expect(inst._currentIndex).toEqual(0); + // @ts-expect-error private + expect(Object.keys(inst._videos).length).toEqual(3); + }); + + it('should load the correct videos after playing at a timestamp', async () => { + const root = document.createElement('div'); + const inst = new VideoReplayer(attachments.concat(extra), { + videoApiPrefix: '/foo/', + root, + start: 0, + onFinished: jest.fn(), + onLoaded: jest.fn(), + }); + // play at segment 7 + const playPromise = inst.play(45_003); + jest.advanceTimersByTime(2500); + await playPromise; + // @ts-expect-error private + expect(inst._currentIndex).toEqual(7); + + // videos loaded should be [0, 1, 2, 4, 5, 6, 7] + // since we have [0, 1, 2] preloaded initially + // and only [4, 5, 6, 7] loaded when segment 7 is requested + + // @ts-expect-error private + const videos = inst._videos; + // @ts-expect-error private + const getVideo = index => inst.getVideo(index); + + expect(Object.keys(videos).length).toEqual(7); + expect(videos[0]).toEqual(getVideo(0)); + expect(videos[2]).toEqual(getVideo(2)); + expect(videos[3]).toEqual(undefined); + expect(videos[4]).toEqual(getVideo(4)); + expect(videos[7]).toEqual(getVideo(7)); + }); + + it('should work correctly if we have missing segments', async () => { + const root = document.createElement('div'); + const inst = new VideoReplayer(attachments.concat(skip), { + videoApiPrefix: '/foo/', + root, + start: 0, + onFinished: jest.fn(), + onLoaded: jest.fn(), + }); + // play at segment 7 + const playPromise = inst.play(45_003); + jest.advanceTimersByTime(2500); + await playPromise; + // @ts-expect-error private + expect(inst._currentIndex).toEqual(6); + + // @ts-expect-error private + const videos = inst._videos; + // @ts-expect-error private + const getVideo = index => inst.getVideo(index); + + // videos loaded should be [0, 1, 2, 3, 4, 5, 7, 8] + expect(Object.keys(videos).length).toEqual(8); + expect(videos[0]).toEqual(getVideo(0)); + expect(videos[2]).toEqual(getVideo(2)); + expect(videos[5]).toEqual(getVideo(5)); + expect(videos[6]).toEqual(getVideo(6)); + expect(videos[7]).toEqual(getVideo(7)); + }); }); describe('VideoReplayer - with starting gap', () => { diff --git a/static/app/components/replays/videoReplayer.tsx b/static/app/components/replays/videoReplayer.tsx index 994550bc6c8dbb..ce98accf7370c7 100644 --- a/static/app/components/replays/videoReplayer.tsx +++ b/static/app/components/replays/videoReplayer.tsx @@ -5,6 +5,10 @@ import {findVideoSegmentIndex} from './utils'; type RootElem = HTMLDivElement | null; +// The number of segments to load on either side of the requested segment (around 15 seconds) +// Also the number of segments we load initially +const PRELOAD_BUFFER = 3; + interface OffsetOptions { segmentOffsetMs?: number; } @@ -38,7 +42,11 @@ export class VideoReplayer { private _startTimestamp: number; private _timer = new Timer(); private _trackList: [ts: number, index: number][]; - private _videos: HTMLVideoElement[]; + /** + * _videos is a dict that maps attachment index to the video element. + * Video elements in this dict are preloaded and ready to be played. + */ + private _videos: Record; private _videoApiPrefix: string; public config: VideoReplayerConfig = { skipInactive: false, @@ -59,15 +67,16 @@ export class VideoReplayer { onFinished, onLoaded, }; + this._videos = {}; this.wrapper = document.createElement('div'); if (root) { root.appendChild(this.wrapper); } - this._videos = this._attachments.map((attachment, index) => - this.createVideo(attachment, index) - ); + // Initially, only load some videos + this.createVideoForRange({low: 0, high: PRELOAD_BUFFER}); + this._trackList = this._attachments.map(({timestamp}, i) => [timestamp, i]); this.loadSegment(0); } @@ -90,7 +99,7 @@ export class VideoReplayer { this._callbacks.onLoaded(event); } }); - // TODO: Only preload when necessary + el.preload = 'auto'; // TODO: Timer needs to also account for playback speed el.playbackRate = this.config.speed; @@ -114,6 +123,20 @@ export class VideoReplayer { this.playSegmentAtIndex(nextIndex); } + /** + * Create videos from a slice of _attachments, given the start and end index. + */ + protected createVideoForRange({low, high}: {high: number; low: number}) { + return this._attachments.slice(low, high).forEach((attachment, index) => { + const dictIndex = index + low; + + // Might be some videos we've already loaded before + if (!this._videos[dictIndex]) { + this._videos[dictIndex] = this.createVideo(attachment, dictIndex); + } + }); + } + /** * Given a relative time offset, get the segment number where the time offset would be contained in */ @@ -141,18 +164,43 @@ export class VideoReplayer { } protected getSegment(index?: number | undefined): VideoEvent | undefined { - if (typeof index === 'undefined') { + if (index === undefined) { return undefined; } return this._attachments[index]; } + /** + * Returns the video in the dictionary at the requested index. + */ protected getVideo(index: number | undefined): HTMLVideoElement | undefined { - if (typeof index === 'undefined') { + if (index === undefined || index < 0 || index >= this._attachments.length) { + return undefined; + } + + return this._videos[index]; + } + + /** + * Fetches the video if it exists, otherwise creates the video and adds to the _videos dictionary. + */ + protected getOrCreateVideo(index: number | undefined): HTMLVideoElement | undefined { + const video = this.getVideo(index); + + if (video) { + return video; + } + + if (index === undefined) { return undefined; } + // If we haven't loaded the current video yet, we should load videos on either side too + const low = Math.max(0, index - PRELOAD_BUFFER); + const high = Math.min(index + PRELOAD_BUFFER, this._attachments.length + 1); + this.createVideoForRange({low, high}); + return this._videos[index]; } @@ -236,7 +284,7 @@ export class VideoReplayer { // Hide current video this.hideVideo(this._currentIndex); - const nextVideo = this.getVideo(index); + const nextVideo = this.getOrCreateVideo(index); // Show the next video this.showVideo(nextVideo); @@ -260,7 +308,7 @@ export class VideoReplayer { const loadedSegmentIndex = await this.loadSegment(index, {segmentOffsetMs: 0}); if (loadedSegmentIndex !== undefined) { - this.playVideo(this.getVideo(loadedSegmentIndex)); + this.playVideo(this.getOrCreateVideo(loadedSegmentIndex)); } } @@ -337,7 +385,7 @@ export class VideoReplayer { return Promise.resolve(); } - return this.playVideo(this.getVideo(loadedSegmentIndex)); + return this.playVideo(this.getOrCreateVideo(loadedSegmentIndex)); } /** @@ -366,7 +414,7 @@ export class VideoReplayer { */ public pause(videoOffsetMs: number) { // Pause the current video - const currentVideo = this.getVideo(this._currentIndex); + const currentVideo = this.getOrCreateVideo(this._currentIndex); currentVideo?.pause(); this._timer.stop(videoOffsetMs);