Skip to content

Commit

Permalink
ref(replay): only preload mobile replay videos when needed (#67657)
Browse files Browse the repository at this point in the history
- 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 #67430
  • Loading branch information
michellewzhang committed Mar 27, 2024
1 parent 90db0a0 commit bbec650
Show file tree
Hide file tree
Showing 2 changed files with 166 additions and 11 deletions.
107 changes: 107 additions & 0 deletions static/app/components/replays/videoReplayer.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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, {
Expand Down Expand Up @@ -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', () => {
Expand Down
70 changes: 59 additions & 11 deletions static/app/components/replays/videoReplayer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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<number, HTMLVideoElement>;
private _videoApiPrefix: string;
public config: VideoReplayerConfig = {
skipInactive: false,
Expand All @@ -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);
}
Expand All @@ -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;
Expand All @@ -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
*/
Expand Down Expand Up @@ -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];
}

Expand Down Expand Up @@ -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);

Expand All @@ -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));
}
}

Expand Down Expand Up @@ -337,7 +385,7 @@ export class VideoReplayer {
return Promise.resolve();
}

return this.playVideo(this.getVideo(loadedSegmentIndex));
return this.playVideo(this.getOrCreateVideo(loadedSegmentIndex));
}

/**
Expand Down Expand Up @@ -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);

Expand Down

0 comments on commit bbec650

Please sign in to comment.