From ac04cc0a5e98a79a37916e2bc13b065796cea47f Mon Sep 17 00:00:00 2001 From: Joe Wearing Date: Mon, 20 Oct 2025 17:15:24 +0100 Subject: [PATCH 1/2] BADGER-16: hide subtitles after seeking to un-subbed region --- src/subtitles/imscsubtitles.js | 8 ++++++++ src/subtitles/imscsubtitles.test.js | 19 +++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/src/subtitles/imscsubtitles.js b/src/subtitles/imscsubtitles.js index 21cd2291..10713c46 100644 --- a/src/subtitles/imscsubtitles.js +++ b/src/subtitles/imscsubtitles.js @@ -24,6 +24,8 @@ function IMSCSubtitles( let currentSubtitlesElement let updateInterval + let currentSubtitlePresentationTime = null + if (autoStart) start() function hasOffset() { @@ -299,6 +301,10 @@ function IMSCSubtitles( function renderSubtitle(xml, currentTime, subsElement, styleOpts, renderHeight, renderWidth) { try { const isd = generateISD(xml, currentTime) + + // track active subtitle's presentation time + currentSubtitlePresentationTime = isd?.contents?.length ? currentTime : null + renderHTML(isd, subsElement, null, renderHeight, renderWidth, false, null, null, false, styleOpts) } catch (error) { error.name = "SubtitlesRenderError" @@ -372,6 +378,8 @@ function IMSCSubtitles( if (segment) { render(currentTime, segment.xml) + } else if (Math.abs(currentTime - (currentSubtitlePresentationTime || currentTime)) > 9) { + removeCurrentSubtitlesElement() } } diff --git a/src/subtitles/imscsubtitles.test.js b/src/subtitles/imscsubtitles.test.js index a72d062e..338f36aa 100644 --- a/src/subtitles/imscsubtitles.test.js +++ b/src/subtitles/imscsubtitles.test.js @@ -1324,6 +1324,25 @@ describe("IMSC Subtitles", () => { expect(generateISD).not.toHaveBeenCalled() expect(renderHTML).not.toHaveBeenCalled() }) + + it("removes subtitle element after seeking to an unsubtitled region", () => { + captions = [{ url: "mock://some.media/captions/subtitles.xml", cdn: "foo" }] + + subtitles = IMSCSubtitles(mockMediaPlayer, targetElement, mockMediaSources, { + autoStart: true, + alwaysOnTop: false, + }) + + setTime(20) + + const preSeekAwayContainer = document.querySelector("#bsp_subtitles") + expect(preSeekAwayContainer).not.toBeNull() + + setTime(0) + + const postSeekAwayContainer = document.querySelector("#bsp_subtitles") + expect(postSeekAwayContainer).toBeNull() + }) }) }) From d3cde61c7171ca405da7aa1d1597c346d3bceb54 Mon Sep 17 00:00:00 2001 From: Joe Wearing Date: Tue, 21 Oct 2025 13:21:48 +0100 Subject: [PATCH 2/2] fix: improved implementation & added tests --- src/subtitles/imscsubtitles.js | 16 ++-- src/subtitles/imscsubtitles.test.js | 116 ++++++++++++++++++++++++++-- 2 files changed, 120 insertions(+), 12 deletions(-) diff --git a/src/subtitles/imscsubtitles.js b/src/subtitles/imscsubtitles.js index 10713c46..e7aeddb6 100644 --- a/src/subtitles/imscsubtitles.js +++ b/src/subtitles/imscsubtitles.js @@ -24,7 +24,7 @@ function IMSCSubtitles( let currentSubtitlesElement let updateInterval - let currentSubtitlePresentationTime = null + let previousTime = null if (autoStart) start() @@ -302,9 +302,6 @@ function IMSCSubtitles( try { const isd = generateISD(xml, currentTime) - // track active subtitle's presentation time - currentSubtitlePresentationTime = isd?.contents?.length ? currentTime : null - renderHTML(isd, subsElement, null, renderHeight, renderWidth, false, null, null, false, styleOpts) } catch (error) { error.name = "SubtitlesRenderError" @@ -374,12 +371,19 @@ function IMSCSubtitles( } function update(currentTime) { + // clears state to ensure we always check if subtitles should be rendered after a seek + if (typeof previousTime === "number" && (previousTime > currentTime || currentTime - previousTime > 2)) { + for (const segment of segments) { + segment.previousSubtitleIndex = undefined + } + removeCurrentSubtitlesElement() + } + previousTime = currentTime + const segment = getSegmentToRender(currentTime) if (segment) { render(currentTime, segment.xml) - } else if (Math.abs(currentTime - (currentSubtitlePresentationTime || currentTime)) > 9) { - removeCurrentSubtitlesElement() } } diff --git a/src/subtitles/imscsubtitles.test.js b/src/subtitles/imscsubtitles.test.js index 338f36aa..dee02a37 100644 --- a/src/subtitles/imscsubtitles.test.js +++ b/src/subtitles/imscsubtitles.test.js @@ -635,6 +635,42 @@ describe("IMSC Subtitles", () => { expect(Plugins.interface.onSubtitlesRenderError).toHaveBeenCalledTimes(1) }) + + it("removes subtitle element after seeking to an unsubtitled region", () => { + captions = [{ url: "mock://some.media/captions/subtitles.xml", cdn: "foo" }] + + subtitles = IMSCSubtitles(mockMediaPlayer, targetElement, mockMediaSources, { + autoStart: true, + }) + + setTime(20) + + const preSeekAwayContainer = document.querySelector("#bsp_subtitles") + expect(preSeekAwayContainer).not.toBeNull() + + setTime(0) + + const postSeekAwayContainer = document.querySelector("#bsp_subtitles") + expect(postSeekAwayContainer).toBeNull() + }) + + it("keeps subtitle element rendered after seeking forward within the same subtitle", () => { + captions = [{ url: "mock://some.media/captions/subtitles.xml", cdn: "foo" }] + + subtitles = IMSCSubtitles(mockMediaPlayer, targetElement, mockMediaSources, { + autoStart: true, + }) + + setTime(20) + + const preSeekAwayContainer = document.querySelector("#bsp_subtitles") + expect(preSeekAwayContainer).not.toBeNull() + + setTime(30) + + const postSeekAwayContainer = document.querySelector("#bsp_subtitles") + expect(postSeekAwayContainer).not.toBeNull() + }) }) describe("subtitles delivered as segments", () => { @@ -1325,23 +1361,91 @@ describe("IMSC Subtitles", () => { expect(renderHTML).not.toHaveBeenCalled() }) - it("removes subtitle element after seeking to an unsubtitled region", () => { - captions = [{ url: "mock://some.media/captions/subtitles.xml", cdn: "foo" }] + it("keeps subtitle element rendered after seeking between segments", () => { + captions = [ + { + type: "application/ttml+xml", + url: "mock://some.media/captions/$segment$.m4s", + cdn: "foo", + segmentLength: 3.84, + }, + ] + + const epochStartTimeSeconds = 1614769200 + + const convertSecondsToEpoch = (...seconds) => + seconds.map((time) => (time === 0 ? 0 : epochStartTimeSeconds + time)) + + const buildMockSegment = ({ beginTimes, id } = {}) => ({ + _mockedSegmentID: id, + body: { + contents: ["stub"], + }, + head: { + styling: {}, + }, + getMediaTimeEvents: () => beginTimes, + }) + + fromXML.mockReturnValueOnce( + buildMockSegment({ + id: 1, + beginTimes: convertSecondsToEpoch(0, 1, 2, 3.84), + }) + ) + + fromXML.mockReturnValueOnce( + buildMockSegment({ + id: 2, + beginTimes: convertSecondsToEpoch(0, 3.84, 4, 7.68), + }) + ) + + fromXML.mockReturnValueOnce( + buildMockSegment({ + id: 3, + beginTimes: convertSecondsToEpoch(0, 7.68, 9, 9.7, 11.52), + }) + ) + + fromXML.mockReturnValueOnce( + buildMockSegment({ + id: 4, + beginTimes: convertSecondsToEpoch(0, 11.52, 14, 16), + }) + ) + + // back to segment 1 + fromXML.mockReturnValueOnce( + buildMockSegment({ + id: 1, + beginTimes: convertSecondsToEpoch(0, 1, 2, 3.84), + }) + ) + + fromXML.mockReturnValue(buildMockSegment({ id: null })) + + generateISD.mockReturnValue({ contents: ["mockContents"] }) subtitles = IMSCSubtitles(mockMediaPlayer, targetElement, mockMediaSources, { autoStart: true, - alwaysOnTop: false, }) - setTime(20) + progressTime(3.5) const preSeekAwayContainer = document.querySelector("#bsp_subtitles") expect(preSeekAwayContainer).not.toBeNull() - setTime(0) + progressTime(2.5) const postSeekAwayContainer = document.querySelector("#bsp_subtitles") - expect(postSeekAwayContainer).toBeNull() + expect(postSeekAwayContainer).not.toBeNull() + + // back to segment 1 + progressTime(-2.5) + + const postSeekAwayContainer2 = document.querySelector("#bsp_subtitles") + expect(postSeekAwayContainer2).not.toBeNull() }) }) })