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(FEC-8390, FEC-8246): support 608/708 captions #254

Merged
merged 8 commits into from
Jul 17, 2018
Merged

Conversation

yairans
Copy link
Contributor

@yairans yairans commented Jul 12, 2018

Description of the Changes

  • support text track of kind 'captions'.
  • handle caption on hover state (need to support upper captions) with 2 ways:
    • Native caption (safari or by configuration): by resizing the subtitle container dynamically
    • Non-native captions(not-safari or by configuration) : by moving the subtitle container dynamically (to avoid caption size change)

CheckLists

  • changes have been done against master branch, and PR does not conflict
  • new unit / functional tests have been added (whenever applicable)
  • test are passing in local environment
  • Travis tests are passing (or test results are not worse than on master branch :))
  • Docs have been updated

@yairans yairans self-assigned this Jul 12, 2018
@yairans yairans requested review from OrenMe and dan-ziv July 12, 2018 21:17
@yairans yairans changed the title fix(FEC-8390, FEC-8246): embedded captions (608/708) doesn't work feat(FEC-8390, FEC-8246): support 608/708 captions Jul 12, 2018
@@ -234,7 +234,7 @@ class LanguageControl extends BaseComponent {
const audioOptions = props.audioTracks
.filter(t => t.label || t.language)
.map(t => ({label: t.label || t.language, active: t.active, value: t}));
const textOptions = props.textTracks.filter(t => t.kind === 'subtitles').map(t => ({
Copy link
Contributor

Choose a reason for hiding this comment

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

Why remove the filter? if anything then make it tighter - subtitles or captions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we already filter the text tracks in the adapters.

yairans added a commit to kaltura/playkit-js-hls that referenced this pull request Jul 17, 2018
parse 608/708 tracks by the video element `textTrack` api
depends on 
kaltura/playkit-js#265
kaltura/playkit-js-ui#254
@yairans yairans merged commit 6a9c787 into master Jul 17, 2018
@yairans yairans deleted the FEC-8390 branch July 17, 2018 13:41
borhandarabi pushed a commit to TasvirChi/playchi-js-hls that referenced this pull request May 14, 2024
parse 608/708 tracks by the video element `textTrack` api
depends on 
kaltura/playkit-js#265
kaltura/playkit-js-ui#254
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

3 participants