From c014d445a4c73237c5f530ddd67cef4d4e52aad4 Mon Sep 17 00:00:00 2001 From: Jacob Trimble Date: Thu, 12 Sep 2019 13:34:16 -0700 Subject: [PATCH] Fix Period transitions with embedded captions. We would incorrectly initialize the embedded captions multiple times during a Period transition, which caused duplicate cues to be given to the displayer. We also wouldn't handle the case when switching between embedded captions and external text during a Period transition. This fixes both cases and adds tests for them. This also avoids passing an empty cue list to the displayer. Fixes #2076 Change-Id: I89add3eb86ad8d93644bba14eabd11f98d57bc5e --- lib/media/streaming_engine.js | 26 +++++- lib/text/text_engine.js | 4 +- test/media/streaming_engine_unit.js | 138 ++++++++++++++++++++++++++++ 3 files changed, 164 insertions(+), 4 deletions(-) diff --git a/lib/media/streaming_engine.js b/lib/media/streaming_engine.js index 2ffa53656..89b2c58da 100644 --- a/lib/media/streaming_engine.js +++ b/lib/media/streaming_engine.js @@ -2181,11 +2181,32 @@ shaka.media.StreamingEngine = class { // Because we are going to modify the map, we need to create a copy of the // keys, so copy the iterable to an array first. for (const type of Array.from(this.mediaStates_.keys())) { + const state = this.mediaStates_.get(type); const stream = streamsByType.get(type); if (stream) { + const wasEmbeddedText = + shaka.media.StreamingEngine.isEmbeddedText_(state); + if (wasEmbeddedText) { + // If this was an embedded text track, we'll need to update the + // needPeriodIndex so it doesn't try to do a Period transition once + // we switch. + state.needPeriodIndex = needPeriodIndex; + state.resumeAt = needPeriod.startTime; + } + this.switchInternal_(stream, /* clearBuffer= */ false, /* safeMargin= */ 0, /* force= */ false); - this.scheduleUpdate_(this.mediaStates_.get(type), 0); + + // Don't schedule an update when changing from embedded text to + // another embedded text since the update will try to load existing + // captions, which are already loaded. + // + // But we do want to schedule an update if we switch to a non-embedded + // text track of if we didn't have an embedded text track before. + if (!wasEmbeddedText || + !shaka.media.StreamingEngine.isEmbeddedText_(state)) { + this.scheduleUpdate_(this.mediaStates_.get(type), 0); + } } else { goog.asserts.assert(type == ContentType.TEXT, 'Invalid streams chosen'); @@ -2206,7 +2227,8 @@ shaka.media.StreamingEngine = class { */ static isEmbeddedText_(mediaState) { const MimeUtils = shaka.util.MimeUtils; - return mediaState.type == shaka.util.ManifestParserUtils.ContentType.TEXT && + return mediaState && + mediaState.type == shaka.util.ManifestParserUtils.ContentType.TEXT && mediaState.stream.mimeType == MimeUtils.CLOSED_CAPTION_MIMETYPE; } diff --git a/lib/text/text_engine.js b/lib/text/text_engine.js index 013859710..fb00a8ae5 100644 --- a/lib/text/text_engine.js +++ b/lib/text/text_engine.js @@ -363,9 +363,9 @@ shaka.text.TextEngine = class { if (captionsMap) { for (const startAndEndTime of captionsMap.keys()) { /** @type {Array.} */ - let cues = captionsMap.get(startAndEndTime); + const cues = captionsMap.get(startAndEndTime) + .filter((c) => c.endTime <= bufferEndTime); if (cues) { - cues = cues.filter((c) => c.endTime <= bufferEndTime); this.displayer_.append(cues); } } diff --git a/test/media/streaming_engine_unit.js b/test/media/streaming_engine_unit.js index 685e69822..44a505a12 100644 --- a/test/media/streaming_engine_unit.js +++ b/test/media/streaming_engine_unit.js @@ -3070,6 +3070,144 @@ describe('StreamingEngine', () => { } }); + describe('embedded text tracks', () => { + beforeEach(() => { + // Set up a manifest with multiple Periods and text streams. + manifest = shaka.test.ManifestGenerator.generate((manifest) => { + manifest.addPeriod(0, (period) => { + period.addVariant(0, (variant) => { + variant.addVideo(110, (stream) => { + stream.useSegmentTemplate('video-110-%d.mp4', 10); + }); + }); + period.addTextStream(120, (stream) => { + stream.useSegmentTemplate('video-120-%d.mp4', 10); + }); + period.addTextStream(121, (stream) => { + stream.mimeType = shaka.util.MimeUtils.CLOSED_CAPTION_MIMETYPE; + }); + }); + manifest.addPeriod(10, (period) => { + period.addVariant(1, (variant) => { + variant.addVideo(210, (stream) => { + stream.useSegmentTemplate('video-210-%d.mp4', 10); + }); + }); + period.addTextStream(220, (stream) => { + stream.useSegmentTemplate('text-220-%d.mp4', 10); + }); + period.addTextStream(221, (stream) => { + stream.mimeType = shaka.util.MimeUtils.CLOSED_CAPTION_MIMETYPE; + }); + }); + }); + + // For these tests, we don't care about specific data appended. + // Just return any old ArrayBuffer for any requested segment. + netEngine = { + request: (requestType, request) => { + const buffer = new ArrayBuffer(0); + const response = {uri: request.uris[0], data: buffer, headers: {}}; + return shaka.util.AbortableOperation.completed(response); + }, + }; + + // For these tests, we also don't need FakeMediaSourceEngine to verify + // its input data. + mediaSourceEngine = new shaka.test.FakeMediaSourceEngine({}); + mediaSourceEngine.clear.and.returnValue(Promise.resolve()); + mediaSourceEngine.bufferedAheadOf.and.returnValue(0); + mediaSourceEngine.bufferStart.and.returnValue(0); + mediaSourceEngine.setStreamProperties.and.returnValue(Promise.resolve()); + mediaSourceEngine.remove.and.returnValue(Promise.resolve()); + mediaSourceEngine.setSelectedClosedCaptionId = + /** @type {?} */ (jasmine.createSpy('setSelectedClosedCaptionId')); + + const bufferEnd = {audio: 0, video: 0, text: 0}; + mediaSourceEngine.appendBuffer.and.callFake( + (type, data, start, end) => { + bufferEnd[type] = end; + return Promise.resolve(); + }); + mediaSourceEngine.bufferEnd.and.callFake((type) => { + return bufferEnd[type]; + }); + mediaSourceEngine.bufferedAheadOf.and.callFake((type, start) => { + return Math.max(0, bufferEnd[type] - start); + }); + mediaSourceEngine.isBuffered.and.callFake((type, time) => { + return time >= 0 && time < bufferEnd[type]; + }); + + const config = shaka.util.PlayerConfiguration.createDefault().streaming; + config.rebufferingGoal = 20; + config.bufferingGoal = 20; + config.alwaysStreamText = true; + config.ignoreTextStreamFailures = true; + + playing = false; + presentationTimeInSeconds = 0; + createStreamingEngine(config); + + onStartupComplete.and.callFake(() => setupFakeGetTime(0)); + }); + + describe('period transition', () => { + it('initializes new embedded captions', async () => { + onChooseStreams.and.callFake((period) => { + if (period == manifest.periods[0]) { + return {variant: period.variants[0]}; + } else { + return {variant: period.variants[0], text: period.textStreams[1]}; + } + }); + await runEmbeddedCaptionTest(); + }); + + it('initializes embedded captions from external text', async () => { + onChooseStreams.and.callFake((period) => { + if (period == manifest.periods[0]) { + return {variant: period.variants[0], text: period.textStreams[0]}; + } else { + return {variant: period.variants[0], text: period.textStreams[1]}; + } + }); + await runEmbeddedCaptionTest(); + }); + + it('switches to external text after embedded captions', async () => { + onChooseStreams.and.callFake((period) => { + if (period == manifest.periods[0]) { + return {variant: period.variants[0], text: period.textStreams[1]}; + } else { + return {variant: period.variants[0], text: period.textStreams[0]}; + } + }); + await runEmbeddedCaptionTest(); + }); + + it('doesn\'t re-initialize', async () => { + onChooseStreams.and.callFake((period) => { + return {variant: period.variants[0], text: period.textStreams[1]}; + }); + await runEmbeddedCaptionTest(); + }); + + async function runEmbeddedCaptionTest() { + streamingEngine.start().catch(fail); + await Util.fakeEventLoop(10); + + // We have buffered through the Period transition. + expect(onChooseStreams).toHaveBeenCalledTimes(2); + expect(Util.invokeSpy(mediaSourceEngine.bufferEnd, 'video')) + .toBeGreaterThan(12); + + expect(mediaSourceEngine.setSelectedClosedCaptionId) + .toHaveBeenCalledTimes(1); + } + }); + }); + /** * Verifies calls to NetworkingEngine.request(). Expects every segment * in the given Period to have been requested.