[ASVideoNode] Improve playerStatus behaviour and image generation (#2024)

* [ASVideoNode] Use the videoComposition when generating images.

* [ASVideoNode] Improve playerState behaviour.

* [ASVideoNode] Use KVO on _player.rate to determine the playerState.

AsyncDisplayKit/ASVideoNode.mm

@@ -37,6 +37,7 @@ static void *ASVideoNodeContext = &ASVideoNodeContext;
 static NSString * const kPlaybackLikelyToKeepUpKey = @"playbackLikelyToKeepUp";
 static NSString * const kplaybackBufferEmpty = @"playbackBufferEmpty";
 static NSString * const kStatus = @"status";
+static NSString * const kRate = @"rate";
 
 @interface ASVideoNode ()
 {
@@ -208,6 +209,25 @@ static NSString * const kStatus = @"status";
   [notificationCenter removeObserver:self name:AVPlayerItemNewErrorLogEntryNotification object:playerItem];
 }
 
+- (void)addPlayerObservers:(AVPlayer *)player
+{
+  if (player == nil) {
+    return;
+  }
+
+  [player addObserver:self forKeyPath:kRate options:NSKeyValueObservingOptionNew context:ASVideoNodeContext];
+}
+
+- (void) removePlayerObservers:(AVPlayer *)player
+{
+  @try {
+    [player removeObserver:self forKeyPath:kRate context:ASVideoNodeContext];
+  }
+  @catch (NSException * __unused exception) {
+    NSLog(@"Unnecessary KVO removal");
+  }
+}
+
 - (void)layout
 {
   [super layout];
@@ -267,6 +287,7 @@ static NSString * const kStatus = @"status";
 
     AVAssetImageGenerator *previewImageGenerator = [AVAssetImageGenerator assetImageGeneratorWithAsset:asset];
     previewImageGenerator.appliesPreferredTrackTransform = YES;
+    previewImageGenerator.videoComposition = _videoComposition;
 
     [previewImageGenerator generateCGImagesAsynchronouslyForTimes:@[[NSValue valueWithCMTime:imageTime]]
                                                 completionHandler:^(CMTime requestedTime, CGImageRef image, CMTime actualTime, AVAssetImageGeneratorResult result, NSError *error) {
@@ -291,21 +312,28 @@ static NSString * const kStatus = @"status";
 {
   ASDN::MutexLocker l(__instanceLock__);
 
-  if (object != _currentPlayerItem) {
+  if (object == _currentPlayerItem) {
-    return;
-  }
-
     if ([keyPath isEqualToString:kStatus]) {
       if ([change[NSKeyValueChangeNewKey] integerValue] == AVPlayerItemStatusReadyToPlay) {
+        if (self.playerState != ASVideoNodePlayerStatePlaying) {
           self.playerState = ASVideoNodePlayerStateReadyToPlay;
+        }
         // If we don't yet have a placeholder image update it now that we should have data available for it
         if (self.image == nil && self.URL == nil) {
           [self generatePlaceholderImage];
         }
       }
     } else if ([keyPath isEqualToString:kPlaybackLikelyToKeepUpKey]) {
+      BOOL likelyToKeepUp = [change[NSKeyValueChangeNewKey] boolValue];
+      if (likelyToKeepUp && self.playerState == ASVideoNodePlayerStatePlaying) {
+        return;
+      }
+      if (!likelyToKeepUp) {
+        self.playerState = ASVideoNodePlayerStateLoading;
+      } else if (self.playerState != ASVideoNodePlayerStateFinished) {
         self.playerState = ASVideoNodePlayerStatePlaybackLikelyToKeepUpButNotPlaying;
-    if (_shouldBePlaying && (_shouldAggressivelyRecoverFromStall || [change[NSKeyValueChangeNewKey] boolValue]) && ASInterfaceStateIncludesVisible(self.interfaceState)) {
+      }
+      if (_shouldBePlaying && (_shouldAggressivelyRecoverFromStall || likelyToKeepUp) && ASInterfaceStateIncludesVisible(self.interfaceState)) {
         if (self.playerState == ASVideoNodePlayerStateLoading && _delegateFlags.delegateVideoNodeDidRecoverFromStall) {
           [_delegate videoNodeDidRecoverFromStall:self];
         }
@@ -316,6 +344,17 @@ static NSString * const kStatus = @"status";
         self.playerState = ASVideoNodePlayerStateLoading;
       }
     }
+  } else if (object == _player) {
+    if ([keyPath isEqualToString:kRate]) {
+      if ([change[NSKeyValueChangeNewKey] floatValue] == 0.0) {
+        if (self.playerState == ASVideoNodePlayerStatePlaying) {
+          self.playerState = ASVideoNodePlayerStatePaused;
+        }
+      } else {
+        self.playerState = ASVideoNodePlayerStatePlaying;
+      }
+    }
+  }
 }
 
 - (void)tapped
@@ -345,14 +384,11 @@ static NSString * const kStatus = @"status";
   ASDN::MutexLocker l(__instanceLock__);
   AVAsset *asset = self.asset;
   // Return immediately if the asset is nil;
-  if (asset == nil || self.playerState == ASVideoNodePlayerStateInitialLoading) {
+  if (asset == nil || self.playerState != ASVideoNodePlayerStateUnknown) {
       return;
   }
 
-  // FIXME: Nothing appears to prevent this method from sending the delegate notification / calling load on the asset
+  self.playerState = ASVideoNodePlayerStateLoading;
-  // multiple times, even after the asset is fully loaded and ready to play.  There should probably be a playerState
-  // for NotLoaded or such, besides Unknown, so this can be easily checked before proceeding.
-  self.playerState = ASVideoNodePlayerStateInitialLoading;
   if (_delegateFlags.delegateVideoNodeDidStartInitialLoading) {
       [_delegate videoNodeDidStartInitialLoading:self];
   }
@@ -396,6 +432,7 @@ static NSString * const kStatus = @"status";
 
     self.player = nil;
     self.currentItem = nil;
+    self.playerState = ASVideoNodePlayerStateUnknown;
   }
 }
 
@@ -603,12 +640,6 @@ static NSString * const kStatus = @"status";
   
   [_player play];
   _shouldBePlaying = YES;
-
-  if (![self ready]) {
-    self.playerState = ASVideoNodePlayerStateLoading;
-  } else {
-    self.playerState = ASVideoNodePlayerStatePlaying;
-  }
 }
 
 - (BOOL)ready
@@ -622,7 +653,6 @@ static NSString * const kStatus = @"status";
   if (![self isStateChangeValid:ASVideoNodePlayerStatePaused]) {
     return;
   }
-  self.playerState = ASVideoNodePlayerStatePaused;
   [_player pause];
   _shouldBePlaying = NO;
 }
@@ -731,9 +761,16 @@ static NSString * const kStatus = @"status";
 - (void)setPlayer:(AVPlayer *)player
 {
   ASDN::MutexLocker l(__instanceLock__);
+
+  [self removePlayerObservers:_player];
+
   _player = player;
   player.muted = _muted;
   ((AVPlayerLayer *)_playerNode.layer).player = player;
+
+  if (player != nil) {
+    [self addPlayerObservers:player];
+  }
 }
 
 - (BOOL)shouldBePlaying
@@ -755,6 +792,7 @@ static NSString * const kStatus = @"status";
   [_player removeTimeObserver:_timeObserver];
   _timeObserver = nil;
   [self removePlayerItemObservers:_currentPlayerItem];
+  [self removePlayerObservers:_player];
 }
 
 @end