Skip to content

IMSC.js Styling #95

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
May 14, 2025
Merged

IMSC.js Styling #95

merged 9 commits into from
May 14, 2025

Conversation

ryanmccartney
Copy link

@ryanmccartney ryanmccartney commented Jan 24, 2025

This PR exposes the bbc/ImscJS customisation options to the Dash.js settings in a way consistent with other subtitle settings already within Dash.js.

bbc/ImscJS customisation has been already been PR'd back to mainline sandflow/imscJS #257. However, there hasn't been any indication when this may be merged.

The changes here are compatible with those proposed to mainline sandflow/imscJS should the PR be merged.

These changes are required to allow customisation of subtitles on TV Platforms through Bigscreen Player for an upcoming low-latency trial. As the content is delivered in chunks for low-latency DASH it is would not be possible to use the existing side-chain mechanism in BSP without significant modification.

package.json Outdated
"localforage": "^1.7.1",
"path-browserify": "^1.0.1",
"ua-parser-js": "^1.0.2"
},
"peerDependencies": {
"imsc": "^1.1.4"

Choose a reason for hiding this comment

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

Why not use the bbc fork?

Copy link
Author

Choose a reason for hiding this comment

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

Hi @nigelmegitt, we are using the latest BBC smp-imsc fork for this. However, it's swapped out in Bigscreen player rather than here — so I've reverted the change here to avoid it being in the bundle twice.

@ryanmccartney ryanmccartney requested a review from a team April 30, 2025 10:52
Copy link

@eirikbjornr eirikbjornr left a comment

Choose a reason for hiding this comment

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

LGTM! Left one comment on docs but I'm not going to block merging on that :)

* When true, only those captions where itts:forcedDisplay="true" will be displayed.
* @property {boolean} [imsc.enableRollUp=true]
* Enable/disable rollUp style display of IMSC captions.
* @property {number} [imsc.options.sizeAdjust=1]

Choose a reason for hiding this comment

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

What's thoughts on declaring a more generic "put your IMSC options here" type rather than calling out a few specific ones? My concern is as written this documentation suggests IMSC options mentioned are the only ones supported.

Copy link
Author

@ryanmccartney ryanmccartney May 12, 2025

Choose a reason for hiding this comment

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

Outcome of some pairing on this — we've tried to create a generic imscJS options object. However, Dash.js uses the default settings as a way of validating the settings. As a result, we opted to create a more comprehensive list of imscJS render options instead. There are a couple of options have been left out (that are not in use) that require further nested objects. If needed in the future, these could be passed via a new customiseSubtitles() function on the media player rather than via updateSettings().

@ryanmccartney ryanmccartney force-pushed the rd-imsc-customisation branch from a9348fa to a162738 Compare May 12, 2025 12:57
@ryanmccartney ryanmccartney merged commit 8b7c98d into bbc:smp-v4.7 May 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants