Skip to content

Conversation

@ryanmccartney
Copy link
Contributor

@ryanmccartney ryanmccartney commented Apr 28, 2025

📺 What

Extends subtitle functionality by adding the ability to use Dash.js rendered subtitles if declared in an MPD rather than using the existing side-chain.

Allows the rendering of subtitles delivered in fragmented chunks, as used for low-latency playback.

It does not implement the current customisation options that are available in the existing side-chain as Dash.js provides no interface to access the customisation options present in the BBC fork of imscJS.

A currently open PR imscJS #257 would enable customisation on upstream imscJS and work on Dash.js (imscJS Styling would allow this functionality to be added retrospectively.

🛠 How
By setting a new override - embeddedSubtitles, the side-chain is replaced by a new embeddedSubtitles object which enables text tracks in the MSE player

✅ Testing [Semi-optional]

Tests have been added to cover changes to the subtitle selector. In addition, coverage of the new dashsubtitles.js object is provided.

Test Guidelines

Test engineer sign off

♿ Accessibility [optional]

Provides optional subtitles when using low-latency playback through Bigscreen player without modifications to the existing side-chain rendering.

@ryanmccartney ryanmccartney marked this pull request as ready for review April 28, 2025 10:33
@ryanmccartney ryanmccartney requested a review from a team as a code owner April 28, 2025 10:33
@ryanmccartney ryanmccartney added the semver minor This PR is a semver minor release label May 2, 2025
@nigelmegitt
Copy link

I'm missing something here: why can't we use a build of bbc/dash.js that includes the customisation functionality? It seems highly sub-optimal to disable it for low latency playback when most of the code exists.

Luke-Chatburn
Luke-Chatburn previously approved these changes May 15, 2025
markhouldridge
markhouldridge previously approved these changes May 15, 2025
Copy link
Contributor

@markhouldridge markhouldridge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! The only thing I spotted is two uses of bsp_subtitles id, but that can be sorted if we pickup the override subs element work, for the Short Form Video work 👍🏻

@eirikbjornr
Copy link
Contributor

I'm missing something here: why can't we use a build of bbc/dash.js that includes the customisation functionality? It seems highly sub-optimal to disable it for low latency playback when most of the code exists.

@nigelmegitt we replace the open-source IMSC version included in Dash.js with the internal BBC version we pull in through BSP so the customisation functionality is enabled for low latency playback too

@nigelmegitt
Copy link

@nigelmegitt we replace the open-source IMSC version included in Dash.js with the internal BBC version we pull in through BSP so the customisation functionality is enabled for low latency playback too

Ah, thank you @eirikbjornr I didn't realise that. OK, that's fine then.

@eirikbjornr
Copy link
Contributor

Amazing work! You've perservered through some gnarly bugs

@ShiningTrapez ShiningTrapez merged commit c44dc84 into master May 15, 2025
4 checks passed
@ShiningTrapez ShiningTrapez deleted the embedded-subtitles branch May 15, 2025 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver minor This PR is a semver minor release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants