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: expose CEA708 window position in the cue's region #5924

Merged
merged 1 commit into from
Nov 23, 2023

Conversation

gkatsev
Copy link
Contributor

@gkatsev gkatsev commented Nov 22, 2023

CEA708 captions have positioning data available in their windows. However, this isn't currently translated and exposed by shaka though it is parsed from the bitstream.

Translates the windows into WebVTT regions and uses the mappings outlined https://dvcs.w3.org/hg/text-tracks/raw-file/default/608toVTT/608toVTT.html#positioning-in-cea-708

This is also partially implements #2583.

Copy link

google-cla bot commented Nov 22, 2023

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).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

CEA708 captions have positioning data available in their windows.
However, this isn't currently translated and exposed by shaka though it
is parsed from the bitstream.

Translates the windows into WebVTT regions and uses the mappings
outlined https://dvcs.w3.org/hg/text-tracks/raw-file/default/608toVTT/608toVTT.html#positioning-in-cea-708

This is also partially implements shaka-project#2583.
@shaka-bot
Copy link
Collaborator

Incremental code coverage: 79.45%

@avelad avelad added type: enhancement New feature or request component: captions/subtitles The issue involves captions or subtitles priority: P2 Smaller impact or easy workaround labels Nov 23, 2023
@avelad avelad added this to the v5.0 milestone Nov 23, 2023
Copy link
Collaborator

@theodab theodab left a comment

Choose a reason for hiding this comment

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

Well, I don't know a lot about CEA708, but I don't see anything obviously wrong with this.

@avelad avelad merged commit 2a524bf into shaka-project:main Nov 23, 2023
17 of 22 checks passed
@gkatsev gkatsev deleted the 708-regions branch November 23, 2023 16:21
gkatsev added a commit to sky-hugolima/shaka-player-contrib that referenced this pull request Nov 27, 2023
With shaka-project#5924, 708 windows were exposed as Cue regions. Since the region ID
was getting set now, the UITextDisplayer was trying to position the
regions but it doesn't properly support LINES units.

This PR skips positioning the region if the values are in LINES units
until that can be properly implemented.
gkatsev added a commit to sky-hugolima/shaka-player-contrib that referenced this pull request Nov 27, 2023
With shaka-project#5924, 708 windows were exposed as Cue regions. Since the region ID
was getting set now, the UITextDisplayer was trying to position the
regions but it doesn't properly support LINES units.

This PR skips positioning the region if the values are in LINES units
until that can be properly implemented.
Robloche pushed a commit to Robloche/shaka-player that referenced this pull request Nov 30, 2023
…t#5924)

CEA708 captions have positioning data available in their windows.
However, this isn't currently translated and exposed by shaka though it
is parsed from the bitstream.

Translates the windows into WebVTT regions and uses the mappings
outlined
https://dvcs.w3.org/hg/text-tracks/raw-file/default/608toVTT/608toVTT.html#positioning-in-cea-708

This is also partially implements shaka-project#2583.
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Jan 22, 2024
@shaka-project shaka-project locked as resolved and limited conversation to collaborators Jan 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
component: captions/subtitles The issue involves captions or subtitles priority: P2 Smaller impact or easy workaround status: archived Archived and locked; will not be updated type: enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants