iOS Side loading for captions and offline support #1109

ashnfb · 2018-07-04T22:01:12Z

Adds support for side-loading captions in iOS for both streaming and offline resources.

fix to respect User settings for captions.

cobarx · 2018-07-06T18:40:43Z

@ashnfb Would you mind merging in the latest master? There's quite a bit that's changed in the main tree.

# Conflicts: # ios/RCTVideo.m # package.json

ashnfb · 2018-07-06T23:09:33Z

@cobarx I've merged upstream into this fork. Should be good to go!

…on iOS 9.3)

cobarx

Here are some initial notes. I haven't gotten it to work quite yet.

Let me know on any of these if you need me to make the changes.

cobarx · 2018-07-06T18:18:54Z

ios/RCTVideo.m

  }
-
-  return [AVPlayerItem playerItemWithURL:url];
+  else if (!isNetwork && !isAsset) // this is a relative file hardcoded in the app?


This can be a simple else clause

I believe files that are loaded via require in JS are not treated as network or assets.

cobarx · 2018-07-08T19:44:03Z

ios/RCTVideo.m

@@ -809,7 +941,6 @@ - (void)usePlayerLayer
      // resize mode must be set before layer is added
      [self setResizeMode:_resizeMode];
      [_playerLayer addObserver:self forKeyPath:readyForDisplayKeyPath options:NSKeyValueObservingOptionNew context:nil];
-      _playerLayerObserverSet = YES;


This is part of a player layer crash fix PR and needs to be reverted.

cobarx · 2018-07-08T19:45:52Z

ios/RCTVideo.m

@@ -18,7 +20,6 @@ @implementation RCTVideo
  BOOL _playerItemObserversSet;
  BOOL _playerBufferEmpty;
  AVPlayerLayer *_playerLayer;
-  BOOL _playerLayerObserverSet;


This is part of a player layer crash fix PR and needs to be reverted.

cobarx · 2018-07-08T19:46:05Z

ios/RCTVideo.m

@@ -848,10 +979,7 @@ - (void)setProgressUpdateInterval:(float)progressUpdateInterval
 - (void)removePlayerLayer
 {
    [_playerLayer removeFromSuperlayer];
-    if (_playerLayerObserverSet) {


This is part of a player layer crash fix PR and needs to be reverted.

cobarx · 2018-07-08T19:46:39Z

ios/RCTVideo.m

 }

- (void)setSeek:(NSDictionary *)info


All of the changes in this function are part of a PR that supports seek tolerance, so the changes need to be reverted.

cobarx · 2018-07-09T05:04:56Z

android-exoplayer/src/main/java/com/brentvatne/exoplayer/ReactExoplayerView.java

+            int sdk = android.os.Build.VERSION.SDK_INT;
+            if (sdk>18 && groups.length>0) {
+                CaptioningManager captioningManager = (CaptioningManager) themedReactContext.getSystemService(Context.CAPTIONING_SERVICE);
+                if (captioningManager.isEnabled()) trackIndex=0;


Thanks for figuring out this exists! I either forgot or didn't know this was available.

We should look for the track that matches the language the UI is set to. I believe we can use Locale.getDefault.getDisplayLanguage(); as stated here:
https://stackoverflow.com/questions/4212320/get-the-current-language-in-device

cobarx · 2018-07-09T05:18:45Z

ios/RCTVideo.m

@@ -666,17 +721,87 @@ - (void)setRepeat:(BOOL)repeat {
  _repeat = repeat;
 }

+- (NSDictionary*) selectedTextTrack {


This needs to get handled in some other way. It's not good practice to have getter methods that have side effects like modifying variables.

cobarx · 2018-07-09T05:24:14Z

ios/RCTVideo.m

 - (NSArray *)getTextTrackInfo
 {
+
+  // if sideloaded, textTracks will already be set
+  if (_textTracks) return _textTracks;


On ExoPlayer, it's possible to use both sideloaded and streaming/embedded captions at the same time. Can we do the same here? As much as possible, I don't want to have to document exceptions or have functionality that varies between different platforms as it's confusing for people.

I'm unclear how this works, so I've not attempted to do this.

cobarx · 2018-07-09T05:30:46Z

ios/RCTVideo.m

  NSMutableArray *textTracks = [[NSMutableArray alloc] init];
  AVMediaSelectionGroup *group = [_player.currentItem.asset
                                  mediaSelectionGroupForMediaCharacteristic:AVMediaCharacteristicLegible];
  for (int i = 0; i < group.options.count; ++i) {
    AVMediaSelectionOption *currentOption = [group.options objectAtIndex:i];
-    NSString *title = @"";


When I was adding caption support, I ran into some videos that don't have any items in the metadata array, so grabbing index 0 will throw an exception. You can add a comment explaining that.

For the language, I think it makes more sense to return an empty string than a null value if that's not set.

cobarx · 2018-07-09T05:46:21Z

ios/RCTVideo.m

-    [[NSURL alloc] initFileURLWithPath:[[NSBundle mainBundle] pathForResource:uri ofType:type]];
+
+  AVURLAsset *asset;
+  AVURLAsset *subAsset;


I tried to avoid using subtitle as the name since text tracks can be subtitles or captions. Imo, text makes more sense. Not a big deal to leave this as is.

cobarx · 2018-07-09T06:16:57Z

Thank you for all your hard work on this!

Ok, I started testing this and it's getting too late to figure out what I'm missing.

A couple questions:
Do you know which formats are supported? The ExoPlayer versions currently supports VTT, TTML, and SRT. If not, I have test files for all 3 and can figure it out.

Is it feasible to support more than one mutable text track? It's a pretty common situation where you have a menu and can select between English, French, Spanish, etc.

Eventually, I'd like to see about making it possible to support sideloaded and streaming captions at the same time. I'm not sure it's super useful so it's not important atm. I can do the work to handle both at some point in the future.

ashnfb · 2018-07-09T17:25:35Z

@cobarx thanks for the detailed feedback. Making the fixes and pushing them today. Cheers, Ash

ashnfb · 2018-07-09T18:47:01Z

Hi @cobarx, I've pushed fixes in the PR for iOS and Android.

Replies to some of your questions:

Do you know which formats are supported? The ExoPlayer versions currently supports VTT, TTML, and SRT. If not, I have test files for all 3 and can figure it out.

It seems like only VTT. I also tried .txtt and that didn't work.

Is it feasible to support more than one mutable text track? It's a pretty common situation where you have a menu and can select between English, French, Spanish, etc.

From what I read, AVMutableCompositionTrack doesn't support AVMediaSelectionOption, which is what is used to switch between languages for Streaming video (AVMediaSelection also has built-in UI support, so that's a bummer we can't use it). We would have to build our own UI for switching tracks, and swap out the Mutable track for one language with another.

cobarx · 2018-07-09T21:26:48Z

Thanks for the quick turnaround on the changes. This is working for me. I tried with ttml and srt files and neither is working, so it does appear to be vtt only.

I've been doing a little digging to see if we can support multiple tracks and figured out how to do this. Let's say you have 2 text tracks loaded via playerItemForSource. Once the item is initialized, you can call the following code from a function like setSideloadedText:

[[[_player.currentItem tracks] objectAtIndex:2] setEnabled:NO];
[[[_player.currentItem tracks] objectAtIndex:3] setEnabled:YES];

And I get the second set of captions. If I flip the enabled values, I get the first set of captions.

So it looks like the approach would be to load all the items in textTracks in playerItemFromSource. Then we need a way to figure out the track indexes in setSideloadedText and disable and enable the right ones. People have already requested multiple audio & video track support, so I don't want to assume that it's always 0: video, 1: audio, 2+: text. This code works to find the first text track index:

int firstTextIndex = 0;
for (firstTextIndex = 0; firstTextIndex < _player.currentItem.tracks.count; ++firstTextIndex) {
   if ([_player.currentItem.tracks[firstTextIndex].assetTrack hasMediaCharacteristic:AVMediaCharacteristicLegible]) {
    break;
  }
}

We can update the logic in setSideloadedText to disable all the text tracks but the one that matches the selectedTextTrack criteria.

Do you want to implement this or should I?

Edit: Make it more clear what setEnabled does.

cobarx · 2018-07-09T21:45:37Z

android-exoplayer/src/main/java/com/brentvatne/exoplayer/ReactExoplayerView.java

@@ -103,7 +105,7 @@
    private boolean loadVideoStarted;
    private boolean isFullscreen;
    private boolean isInBackground;
-    private boolean isPaused;
+    private boolean isPaused = true;


This looks like a change that got wiped out during the merge. Please revert.

cobarx · 2018-07-09T21:46:25Z

ios/RCTVideo.m

+
+    [self addPlayerTimeObserver];
+
+    dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(0 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{


We are already in a delayed run loop, so I don't think we need a second one.

Agreed, the extra dispatch can be removed.

cobarx · 2018-07-09T21:51:52Z

android-exoplayer/src/main/java/com/brentvatne/exoplayer/ReactExoplayerView.java

@@ -762,7 +764,26 @@ public void setSelectedTextTrack(String type, Dynamic value) {
            trackIndex = value.asInt();
        } else { // default. invalid type or "system"
            trackSelector.clearSelectionOverrides(index);


Do we need this or the setSelectionOverride anymore? If we are SDK <= 18 or no groups, the trackIndex will be C.INDEX_UNSET and we'll clear the overrides further down.

@cobarx we need the setSelectionOverride in cases where captions were previously enabled, and are now explicitly disabled (or the system settings are disabled). I've moved this up further in the code for clarity.

cobarx · 2018-07-09T22:46:19Z

I have a few hours free this afternoon, so I'm going to start working on the text track selection for iOS piece. I wanted to make sure we don't duplicate effort. I'll post what I have at the end of the day.

ashnfb · 2018-07-09T22:50:08Z

@cobarx thanks for the heads up. I'll comment on some of the iOS changes above, and I'll look into the Android changes then.

clearSelectionOverride is still required for "default" and "disabled" case as it otherwise continues to show a selected track if it was previously selected

cobarx · 2018-07-09T23:31:04Z

Ok I have the selection part working in 93ce4d6. Can you merge that into this PR.

I am seeing one issue which is that if you specify textTracks it screws up HLS playlists since they can contain video, audio, and text tracks. I'm looking to see if there is a way to fix that.

cobarx · 2018-07-09T23:38:40Z

@ashnfb Ok found a Stack Overflow that suggests it's not possible to use HLS streams in a mutable composition since tracks can be added & removed throughout playback:
https://stackoverflow.com/questions/8318422/inserting-an-http-stream-into-a-avmutablecomposition

So I think this is ready to merge as soon as you pull in my commit and remove the redundant dispatch call.

cobarx · 2018-07-09T23:39:36Z

Pretty sure it all works, but would appreciate a review on my changes as well.

cobarx · 2018-07-09T23:43:44Z

I will go ahead and document everything after we're done.

ashnfb · 2018-07-09T23:44:25Z

Ok, reviewing and merging now.

ashnfb · 2018-07-10T00:02:03Z

@cobarx I've merged your changes, but unfortunately sideloading isn't working for me. It looks like _player is null when setSideloadedText is being called, which results in no text being enabled?

cobarx · 2018-07-10T00:47:38Z

What I was seeing is that setSelectedTrack was called multiple times. At first, _player is null but eventually everything is set, applyModifiers gets called, and then it enables the right stuff.

cobarx · 2018-07-10T01:00:05Z

Is it feasible to send me a copy of the code you use on the React side to create the player? Props get set in the native code in different orders depending on how you pass the props in. If not, I can probably trial and error it until I get the right combination.

ashnfb · 2018-07-10T02:17:58Z

@cobarx invite sent to the repo. Thanks for offering to look into the problem. With the app, turn on Captions through device settings, and use the Search button to find film "Am Stram" (not all films have captions). In the React code, the props are sent by FilmDetail.js

cobarx · 2018-07-10T04:13:08Z

@ashnfb Got it, thanks. This is helpful.

I found one issue. I have English set on my simulator and the captions are French. It was setting the wrong default index when there wasn't a caption matching the system language. Once I made the change, the captions showed up. I've committed my fix to this PR.

I checked the code and setSelectedTextTrack is called as part of applyModifiers once the onLoad event fires, so I'm guessing the issue that I encountered is the same as yours and that _player being nil is not the problem. Please test against and let me know if there's still a problem.

ashnfb · 2018-07-10T16:05:30Z

@cobarx thanks for investigating and solving the problem! works fine here too, Ready for merge 👍

cobarx · 2018-07-11T03:03:30Z

Awesome, it's great to have full subtitle support!

cvaldezscse · 2018-10-31T14:20:02Z

@cobarx Thanks for the hardwork, but i haven't understood where do we put the Url of the subtitle file, i mean, could you make an example or demo, of how does it work? or the correct steps to implement this feature

iOS Sidecar loading for captions and offline support (isAsset). Android

3e2e16e

fix to respect User settings for captions.

ashnfb force-pushed the master branch from 8d7d4b9 to 3e2e16e Compare July 5, 2018 20:37

ashnfb mentioned this pull request Jul 5, 2018

[discussion] Caption & subtitle support #1044

Closed

9 tasks

Merge remote-tracking branch 'upstream/master'

05d4be2

# Conflicts: # ios/RCTVideo.m # package.json

Fix for textTracks not being set before selectedTextTrack (bug found …

fa63a9e

…on iOS 9.3)

cobarx suggested changes Jul 9, 2018

View reviewed changes

ashnfb added 2 commits July 9, 2018 11:20

PR fixes for iOS sideloaded captions

c9b7524

Android device locale support for captions

572c11a

cobarx reviewed Jul 9, 2018

View reviewed changes

ashnfb and others added 2 commits July 9, 2018 16:18

isPaused reverted

6339b9a

clearSelectionOverride is still required for "default" and "disabled" case as it otherwise continues to show a selected track if it was previously selected

Support loading multiple sideloaded text tracks

93ce4d6

Merge branch 'nfb-onf-master'

ea7bc15

Merged, but text sideloading is not working

0d32a31

Fix default sideload text track for system caption setting

fe26f54

cobarx approved these changes Jul 10, 2018

View reviewed changes

cobarx merged commit 91ba07c into TheWidlarzGroup:master Jul 11, 2018

danielmcmillan mentioned this pull request Jul 12, 2018

Fix UI block on iOS during initial load #1086

Closed


		[self addPlayerTimeObserver];

		dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(0 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{

iOS Side loading for captions and offline support #1109

iOS Side loading for captions and offline support #1109

Conversation

ashnfb commented Jul 4, 2018

cobarx commented Jul 6, 2018

ashnfb commented Jul 6, 2018

cobarx left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cobarx commented Jul 9, 2018

ashnfb commented Jul 9, 2018

ashnfb commented Jul 9, 2018 • edited Loading

cobarx commented Jul 9, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cobarx commented Jul 9, 2018

ashnfb commented Jul 9, 2018

cobarx commented Jul 9, 2018

cobarx commented Jul 9, 2018

cobarx commented Jul 9, 2018

cobarx commented Jul 9, 2018

ashnfb commented Jul 9, 2018

ashnfb commented Jul 10, 2018

cobarx commented Jul 10, 2018

cobarx commented Jul 10, 2018

ashnfb commented Jul 10, 2018

cobarx commented Jul 10, 2018 • edited Loading

ashnfb commented Jul 10, 2018

cobarx commented Jul 11, 2018

cvaldezscse commented Oct 31, 2018 • edited Loading

ashnfb commented Jul 9, 2018 •

edited

Loading

cobarx commented Jul 9, 2018 •

edited

Loading

cobarx commented Jul 10, 2018 •

edited

Loading

cvaldezscse commented Oct 31, 2018 •

edited

Loading