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

fix(texttracks): unable to disable sideloaded texttracks in the AVPlayer #2679

Merged

Conversation

nbennink
Copy link
Contributor

@nbennink nbennink commented May 12, 2022

This PR will fix #1144. I previously submitted PR #1468 but that branch become stale and is therefore closed. I forked the repo from scratch and applied the fix to the changes made in this repository in the last few years.

The issue:

The AvPlayer will freeze the last shown subtitle when you disable sideloaded text tracks. There is no way to disable them. This is only an issue with sideloaded subtitles on iOS and tvOS. Android ExoPlayer or subtitles embedded in the HLS manifest work perfectly.

Reproduce steps:

  • Load a video and set VTT texttracks using the texttracks property.
  • Select one of the subtitles
  • Disable the subtitles exactly when there is a subtitle visible (set selectedTextTrack prop to {type: 'disabled'}.
  • Notice how the last shown subtitle is frozen

The solution

The exact cause of the issue is unknown (as can be seen in #1144). All enabled states are set to false so you would expect that all tracks would disappear:
https://github.com/react-native-video/react-native-video/blob/master/ios/Video/RCTVideo.m#L1238

A solution to fix this would be to load an empty VTT file that gets selected when {type: 'disabled'} is given. This would fake disabled subtitles if there are no cues in the VTT file to display.

This PR adds the generation of the VTT file into react-native-video. This solves the problem in the Video component and it doesn't need a dependency on rn-fetch-blob. The VTT file is almost empty, it only shows a small dot for 1 ms at the 99:59:59.000 timestamp (I guess it's safe to assume that it's empty). The file is only added in the player when you are sideloading texttracks, there are no texttracks loaded when the texttracks prop is not set. The file is written into the Caches directory to make this fix work on both iOS and tvOS.

Final note: the original code was made a few years ago. I tried to look into the problem again but couldn't find another solution. If there is anyone with a suggestion please let me know. Loading this file seems like a dirty hack but there appears to be no other solution for now.

@hueniverse
Copy link
Contributor

@nbennink Before spending more time rebasing this again, let's wait for some feedback to make sure this is the correct fix.

@nickfujita @freeboub Can you take a quick look?

@freeboub
Copy link
Collaborator

Hi, i am really not an expert in ios :/ but code looks good to me and the workaround make sens i think. I just have a doubt about the data of the blank vtt track. There is no index... (Maybe i miss something i just read the code) . It is important from my experience on Android to have an index on each track. This index shall be used to select subtitles. In case you have 2 tracks with the same language you must select them by index. Can you please ensure an index is provided on api level please ? Else it is ok for me!

@gurbela
Copy link

gurbela commented May 21, 2022

Hi, I have a similar problem with iOS, Android works fine.
I use TextTrackType.VTT for textTracks. Is there any solution to fix this?

@freeboub
Copy link
Collaborator

Hi, I have a similar problem with iOS, Android works fine. I use TextTrackType.VTT for textTracks. Is there any solution to fix this?

Please test this PR and approve it if it works fine ! :)

@nbennink
Copy link
Contributor Author

Hi, i am really not an expert in ios :/ but code looks good to me and the workaround make sens i think. I just have a doubt about the data of the blank vtt track. There is no index... (Maybe i miss something i just read the code) . It is important from my experience on Android to have an index on each track. This index shall be used to select subtitles. In case you have 2 tracks with the same language you must select them by index. Can you please ensure an index is provided on api level please ? Else it is ok for me!

Thanks for checking it out! This tracks should technically not be selected manually in your own code using an index. The package handles it automatically with {type: "disabled"}. That being said, the language of the disabled track is disabled so that can technically cause an issue when you provide a track with that language as well.

I see that the Swift PR is merged to master as well. Let me check if this issue still exists in the new codebase. If it does I can refactor this with Swift and add an index as well 👍

@freeboub
Copy link
Collaborator

Thanks, i totally agee, expect that user will lost to see a track but will not be able to select it. Selecting Disable, or this fake track should have the same behavior... Or you should explicitly say in doc that this track cannot be selected (i think it 's better to allow selection of this track :) )

@gurbela
Copy link

gurbela commented May 22, 2022

textTracks = [{ title: 'English', language: 'en', type: TextTrackType.VTT, uri: "subtitleLink", }]; selectedTextTrack= {type: 'title', value: 'English'};

I tested this PR, and it works better.
But sometimes it still has a problem that automatically Fixed itself in seconds (3-4)

@freeboub
Copy link
Collaborator

Hi, i am really not an expert in ios :/ but code looks good to me and the workaround make sens i think. I just have a doubt about the data of the blank vtt track. There is no index... (Maybe i miss something i just read the code) . It is important from my experience on Android to have an index on each track. This index shall be used to select subtitles. In case you have 2 tracks with the same language you must select them by index. Can you please ensure an index is provided on api level please ? Else it is ok for me!

Thanks for checking it out! This tracks should technically not be selected manually in your own code using an index. The package handles it automatically with {type: "disabled"}. That being said, the language of the disabled track is disabled so that can technically cause an issue when you provide a track with that language as well.

I see that the Swift PR is merged to master as well. Let me check if this issue still exists in the new codebase. If it does I can refactor this with Swift and add an index as well 👍

In fact, let me correct myself, this track should not be shown to application. As it is not shown with other subtitles/player, there is no reason to show it in this use case. It should be only an internal 'mixture'. Thanks !

Copy link

@sjors98 sjors98 left a comment

Choose a reason for hiding this comment

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

Tested on both ios and tvos, works fine!

@freeboub
Copy link
Collaborator

freeboub commented Jun 3, 2022

@nbennink can you fix conflicts please ?

@nbennink
Copy link
Contributor Author

I just wanted to let you know that I'm looking into this. Sideloading text tracks is currently not working on master so I've attempted to fix that as well. The only problem I now experience is similar to what @gurbela describes.

It appears that the AVPlayer "awaits" it's current cue and clears it when it's finished. For example: you have a subtitle from 00:00 to 00:10. If you disable at 00:06 it will play the old subtitle for 4 more seconds, after that it will switch to the disabled subtitle. Maybe this is a bug with local files or something because I don't experience it with other tracks.

The issue isn't that big because it will fix itself within a few seconds but it isn't that great either.

@hueniverse
Copy link
Contributor

@nbennink Is it all ready now?

@hueniverse hueniverse removed 2 labels Jun 23, 2022
@hueniverse hueniverse added this to the 6.0.0-alpha.2 milestone Jun 23, 2022
@hueniverse hueniverse merged commit 41731a8 into TheWidlarzGroup:master Jun 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

iOS Sidecar VTT TextTrack toggling does not work
5 participants