Skip to content
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

feat: add option for segment-relative VTT timings #4083

Merged
merged 3 commits into from Apr 1, 2022

Conversation

elsassph
Copy link
Contributor

@elsassph elsassph commented Mar 30, 2022

This PR fixes #3242 where for some live streams using segmented VTT, text timings are relative to segment start instead of being absolute.

The PR introduces a new setting: manifest.segmentRelativeVttTiming: boolean allowing such alternative timing offset calculation.

The setting is off by default, preserving the current player behaviour.

@google-cla
Copy link

google-cla bot commented Mar 30, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

For more information, open the CLA check for this pull request.

@elsassph elsassph force-pushed the feature/text-offset-mode branch 2 times, most recently from e61230c to 0a1a3cd Compare March 30, 2022 14:28
Copy link
Member

@joeyparrish joeyparrish left a comment

Choose a reason for hiding this comment

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

Looks good otherwise. Thanks!

externs/shaka/player.js Outdated Show resolved Hide resolved
externs/shaka/text.js Outdated Show resolved Hide resolved
externs/shaka/player.js Outdated Show resolved Hide resolved
lib/media/media_source_engine.js Outdated Show resolved Hide resolved
lib/text/vtt_text_parser.js Show resolved Hide resolved
@elsassph elsassph force-pushed the feature/text-offset-mode branch 3 times, most recently from f26d563 to ad58e80 Compare April 1, 2022 07:12
@elsassph
Copy link
Contributor Author

elsassph commented Apr 1, 2022

I'm going to try to add proper tests for the new config option - wish me luck.

@elsassph elsassph changed the title feat: add option for alternative subtitles offset calculation feat: add option for segment-relative VTT timings Apr 1, 2022
@elsassph
Copy link
Contributor Author

elsassph commented Apr 1, 2022

@joeyparrish now with some unit tests

joeyparrish
joeyparrish previously approved these changes Apr 1, 2022
externs/shaka/player.js Outdated Show resolved Hide resolved
test/text/text_engine_unit.js Outdated Show resolved Hide resolved
@philippe-elsass-deltatre

Thanks for tidying up and approving @joeyparrish!

@joeyparrish joeyparrish merged commit f382cc7 into shaka-project:main Apr 1, 2022
@avelad avelad added this to the v4.0 milestone May 4, 2022
@github-actions github-actions bot added the status: archived Archived and locked; will not be updated label Jul 25, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: archived Archived and locked; will not be updated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

VTT cues relative segment start time is not getting applied
4 participants