Skip to content

Fix subtitle switching is broken when using SSTextTrackDescription #114

Open
Yousif-CS wants to merge 2 commits intomainfrom
bugfix/OPTI-1897-side-loaded-text-track-switch-does-not-work
Open

Fix subtitle switching is broken when using SSTextTrackDescription #114
Yousif-CS wants to merge 2 commits intomainfrom
bugfix/OPTI-1897-side-loaded-text-track-switch-does-not-work

Conversation

@Yousif-CS
Copy link
Copy Markdown
Contributor

There is a couple issues where when using SSTextTrackDescription and automaticTimestampSyncEnabled being false, subtitle switching ends up being stuck on the first loaded subtitle

The issue mainly is in the SubtitleTransformer using the same URL for both subtitles so we run into cache issues when switching.

The reason why it does not reproduce with TextTrackDescription is because:

    let autosync: Bool? = (trackDescription as? SSTextTrackDescription)?.automaticTimestampSyncEnabled
    let subtitlesMediaURL: String
    if (timestamp?.localTime == nil && timestamp?.pts == nil && format == .WebVTT && autosync == nil) {
        subtitlesMediaURL = originalURL.absoluteString
    } else {
        subtitlesMediaURL = self.transformer.composeTranformationUrl(with: originalURL.absoluteString, format: format, timestamp: timestamp)
    }

The cast above fails for TextTrackDescription and no transformer is used (we go into the if block). However, for SSTextTrackDescription the variable autosync ends up being false (the default value) and not nil, and therefore goes into the transformation block, and the transformer cache problem above starts showing.

The fix for the latter issue is to test against true (autosync != true) which handles the case when it is nil due to a failed cast but also guards against it being false in the case of SSTextTrackDescription

The fix for the main issue is to set the subtitle url as a query parameter in the transformer url so we don’t run into sharing the same cache between different subtitle urls.

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.

1 participant