Skip to content

Commit 4cb6767

Browse files
tykus160avelad
authored andcommitted
perf(CEA): Remove captions attached to removed segments (#9068)
TextEngine was storing closed captions and releasing them only on teardown. This might be a potential memory leak for long running sessions. This PR addresses it by removing cached captions along with removed segments. Furthermore, it adds some more tweaks and improvements, such as: - simplify CEA cache structure, from `Map<string, Map<string, Array<Cue>>>` to `Map<string, Array<Cue>>` - adjust TextEngine buffered ranges with cached cues values
1 parent b951c56 commit 4cb6767

File tree

3 files changed

+67
-43
lines changed

3 files changed

+67
-43
lines changed

lib/media/media_source_engine.js

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1250,8 +1250,6 @@ shaka.media.MediaSourceEngine = class {
12501250
if (closedCaptions.length) {
12511251
this.textEngine_.storeAndAppendClosedCaptions(
12521252
closedCaptions,
1253-
reference.startTime,
1254-
reference.endTime,
12551253
timestampOffset);
12561254
}
12571255
}
@@ -1364,6 +1362,10 @@ shaka.media.MediaSourceEngine = class {
13641362
const ContentType = shaka.util.ManifestParserUtils.ContentType;
13651363
if (contentType == ContentType.VIDEO && this.captionParser_) {
13661364
this.captionParser_.remove(continuityTimelines);
1365+
// Get actual TextEngine buffer start, as it's not the same as video
1366+
// buffer and TextEngine does not support multiple buffered ranges.
1367+
const textStart = this.textEngine_.bufferStart() || 0;
1368+
this.textEngine_.remove(textStart, endTime);
13671369
}
13681370
if (contentType == ContentType.TEXT) {
13691371
await this.textEngine_.remove(startTime, endTime);
@@ -1395,6 +1397,11 @@ shaka.media.MediaSourceEngine = class {
13951397
}
13961398
await this.textEngine_.remove(0, Infinity);
13971399
} else {
1400+
// if we have CEA captions, we should clear those too.
1401+
if (contentType === ContentType.VIDEO && this.captionParser_ &&
1402+
this.textEngine_) {
1403+
await this.textEngine_.remove(0, Infinity);
1404+
}
13981405
// Note that not all platforms allow clearing to Infinity.
13991406
await this.enqueueOperation_(
14001407
contentType,

lib/text/text_engine.js

Lines changed: 52 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,7 @@ shaka.text.TextEngine = class {
6161
* when we start displaying captions or switch caption tracks, we need to be
6262
* able to get the cues for the other language and display them without
6363
* re-fetching the video segments they were embedded in.
64-
* Structure of closed caption map:
65-
* closed caption id -> {start and end time -> cues}
66-
* @private {!Map<string, !Map<string, !Array<shaka.text.Cue>>>}
64+
* @private {!Map<string, !Array<shaka.text.Cue>>}
6765
*/
6866
this.closedCaptionsMap_ = new Map();
6967
}
@@ -249,7 +247,10 @@ shaka.text.TextEngine = class {
249247
async remove(startTime, endTime) {
250248
// Start the operation asynchronously to avoid blocking the caller.
251249
await Promise.resolve();
252-
250+
if (startTime >= endTime) {
251+
return;
252+
}
253+
this.removeClosedCaptions_(startTime, endTime);
253254
if (this.displayer_ && this.displayer_.remove(startTime, endTime)) {
254255
if (this.bufferStart_ == null) {
255256
goog.asserts.assert(
@@ -279,6 +280,8 @@ shaka.text.TextEngine = class {
279280
goog.asserts.assert(
280281
false, 'removal from the middle is not supported by TextEngine');
281282
}
283+
284+
this.updateRangesWithClosedCaptions_();
282285
}
283286
}
284287
}
@@ -353,15 +356,11 @@ shaka.text.TextEngine = class {
353356
setSelectedClosedCaptionId(id, bufferEndTime) {
354357
this.selectedClosedCaptionId_ = id;
355358

356-
const captionsMap = this.closedCaptionsMap_.get(id);
357-
if (captionsMap) {
358-
for (const startAndEndTime of captionsMap.keys()) {
359-
/** @type {Array<!shaka.text.Cue>} */
360-
const cues = captionsMap.get(startAndEndTime)
361-
.filter((c) => c.endTime <= bufferEndTime);
362-
if (cues) {
363-
this.displayer_.append(cues);
364-
}
359+
const captions = this.closedCaptionsMap_.get(id);
360+
if (captions) {
361+
const cues = captions.filter((c) => c.endTime <= bufferEndTime);
362+
if (cues.length) {
363+
this.displayer_.append(cues);
365364
}
366365
}
367366
}
@@ -384,25 +383,18 @@ shaka.text.TextEngine = class {
384383
* text displayer. This is a side-channel used for embedded text only.
385384
*
386385
* @param {!Array<!shaka.extern.ICaptionDecoder.ClosedCaption>} closedCaptions
387-
* @param {?number} startTime relative to the start of the presentation
388-
* @param {?number} endTime relative to the start of the presentation
389386
* @param {number} videoTimestampOffset the timestamp offset of the video
390387
* stream in which these captions were embedded
391388
*/
392-
storeAndAppendClosedCaptions(
393-
closedCaptions, startTime, endTime, videoTimestampOffset) {
394-
const startAndEndTime = startTime + ' ' + endTime;
395-
/** @type {!Map<string, !Map<string, !Array<!shaka.text.Cue>>>} */
389+
storeAndAppendClosedCaptions(closedCaptions, videoTimestampOffset) {
390+
/** @type {!Map<string, !Array<!shaka.text.Cue>>} */
396391
const captionsMap = new Map();
397392

398393
for (const caption of closedCaptions) {
399394
const id = caption.stream;
400395
const cue = caption.cue;
401396
if (!captionsMap.has(id)) {
402-
captionsMap.set(id, new Map());
403-
}
404-
if (!captionsMap.get(id).has(startAndEndTime)) {
405-
captionsMap.get(id).set(startAndEndTime, []);
397+
captionsMap.set(id, []);
406398
}
407399

408400
// Adjust CEA captions with respect to the timestamp offset of the video
@@ -416,22 +408,53 @@ shaka.text.TextEngine = class {
416408
continue;
417409
}
418410

419-
captionsMap.get(id).get(startAndEndTime).push(cue);
411+
captionsMap.get(id).push(cue);
420412
if (id == this.selectedClosedCaptionId_) {
421413
this.displayer_.append([cue]);
422414
}
423415
}
424416

425417
for (const id of captionsMap.keys()) {
426418
if (!this.closedCaptionsMap_.has(id)) {
427-
this.closedCaptionsMap_.set(id, new Map());
419+
this.closedCaptionsMap_.set(id, []);
428420
}
429-
for (const startAndEndTime of captionsMap.get(id).keys()) {
430-
const cues = captionsMap.get(id).get(startAndEndTime);
431-
this.closedCaptionsMap_.get(id).set(startAndEndTime, cues);
421+
for (const cue of captionsMap.get(id)) {
422+
this.closedCaptionsMap_.get(id).push(cue);
432423
}
433424
}
434425

426+
this.updateRangesWithClosedCaptions_();
427+
}
428+
429+
/**
430+
* @param {number} startTime
431+
* @param {number} endTime
432+
* @private
433+
*/
434+
removeClosedCaptions_(startTime, endTime) {
435+
for (const id of this.closedCaptionsMap_.keys()) {
436+
let captions = this.closedCaptionsMap_.get(id);
437+
captions = captions.filter(
438+
(cue) => cue.startTime < startTime || cue.endTime >= endTime);
439+
this.closedCaptionsMap_.set(id, captions);
440+
}
441+
}
442+
443+
/**
444+
* @private
445+
*/
446+
updateRangesWithClosedCaptions_() {
447+
let startTime = Infinity;
448+
let endTime = -Infinity;
449+
for (const captions of this.closedCaptionsMap_.values()) {
450+
for (const cue of captions) {
451+
startTime = Math.min(startTime, cue.startTime);
452+
endTime = Math.max(endTime, cue.endTime);
453+
}
454+
}
455+
if (startTime === Infinity || endTime === -Infinity) {
456+
return;
457+
}
435458
if (this.bufferStart_ == null) {
436459
this.bufferStart_ = Math.max(startTime, this.appendWindowStart_);
437460
} else {
@@ -465,7 +488,7 @@ shaka.text.TextEngine = class {
465488
*/
466489
getNumberOfClosedCaptionsInChannel(channelId) {
467490
const channel = this.closedCaptionsMap_.get(channelId);
468-
return channel ? channel.size : 0;
491+
return channel ? channel.length : 0;
469492
}
470493
};
471494

test/text/text_engine_unit.js

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -164,8 +164,7 @@ describe('TextEngine', () => {
164164
};
165165

166166
textEngine.setSelectedClosedCaptionId('CC1', 0);
167-
textEngine.storeAndAppendClosedCaptions(
168-
[caption], /* startTime= */ 0, /* endTime= */ 2, /* offset= */ 0);
167+
textEngine.storeAndAppendClosedCaptions([caption], /* offset= */ 0);
169168
expect(mockDisplayer.appendSpy).toHaveBeenCalled();
170169
});
171170

@@ -179,8 +178,7 @@ describe('TextEngine', () => {
179178
};
180179

181180
textEngine.setSelectedClosedCaptionId('CC3', 0);
182-
textEngine.storeAndAppendClosedCaptions(
183-
[caption], /* startTime= */ 0, /* endTime= */ 2, /* offset= */ 0);
181+
textEngine.storeAndAppendClosedCaptions([caption], /* offset= */ 0);
184182
expect(mockDisplayer.appendSpy).not.toHaveBeenCalled();
185183
});
186184

@@ -201,21 +199,18 @@ describe('TextEngine', () => {
201199
textEngine.setSelectedClosedCaptionId('CC1', 0);
202200
// Text Engine stores all the closed captions as a two layer map.
203201
// {closed caption id -> {start and end time -> cues}}
204-
textEngine.storeAndAppendClosedCaptions(
205-
[caption0], /* startTime= */ 0, /* endTime= */ 1, /* offset= */ 0);
202+
textEngine.storeAndAppendClosedCaptions([caption0], /* offset= */ 0);
206203
expect(textEngine.getNumberOfClosedCaptionChannels()).toBe(1);
207204
expect(textEngine.getNumberOfClosedCaptionsInChannel('CC1')).toBe(1);
208205

209-
textEngine.storeAndAppendClosedCaptions(
210-
[caption1], /* startTime= */ 1, /* endTime= */ 2, /* offset= */ 0);
206+
textEngine.storeAndAppendClosedCaptions([caption1], /* offset= */ 0);
211207
// Caption1 has the same stream id with caption0, but different start and
212208
// end time. The closed captions map should have 1 key CC1, and two values
213209
// for two start and end times.
214210
expect(textEngine.getNumberOfClosedCaptionChannels()).toBe(1);
215211
expect(textEngine.getNumberOfClosedCaptionsInChannel('CC1')).toBe(2);
216212

217-
textEngine.storeAndAppendClosedCaptions(
218-
[caption2], /* startTime= */ 1, /* endTime= */ 2, /* offset= */ 0);
213+
textEngine.storeAndAppendClosedCaptions([caption2], /* offset= */ 0);
219214
// Caption2 has a different stream id CC3, so the closed captions map
220215
// should have two different keys, CC1 and CC3.
221216
expect(textEngine.getNumberOfClosedCaptionChannels()).toBe(2);
@@ -231,8 +226,7 @@ describe('TextEngine', () => {
231226
};
232227

233228
textEngine.setSelectedClosedCaptionId('CC1', 0);
234-
textEngine.storeAndAppendClosedCaptions(
235-
[caption], /* startTime= */ 0, /* endTime= */ 2, /* offset= */ 1000);
229+
textEngine.storeAndAppendClosedCaptions([caption], /* offset= */ 1000);
236230
expect(mockDisplayer.appendSpy).toHaveBeenCalledWith([
237231
jasmine.objectContaining({
238232
startTime: 1000,

0 commit comments

Comments
 (0)