[ASVideoPlayerNode] Fixes and improvements (#3135)

* Improvements for ASVideoPlayer, especially around asset handling and API

* Change legacy URL of videos

* Address comments

Source/ASVideoPlayerNode.h

@@ -39,7 +39,7 @@ NS_ASSUME_NONNULL_BEGIN
 
 @property (nonatomic, assign) BOOL controlsDisabled;
 
-@property (nonatomic, assign, readonly) BOOL loadAssetWhenNodeBecomesVisible;
+@property (nonatomic, assign, readonly) BOOL loadAssetWhenNodeBecomesVisible ASDISPLAYNODE_DEPRECATED_MSG("Asset is always loaded when this node enters preload state. This flag does nothing.");
 
 #pragma mark - ASVideoNode property proxy
 /**
@@ -52,6 +52,16 @@ NS_ASSUME_NONNULL_BEGIN
 @property (nonatomic, assign, readonly) ASVideoNodePlayerState playerState;
 @property (nonatomic, assign, readwrite) BOOL shouldAggressivelyRecoverFromStall;
 @property (nullable, nonatomic, strong, readwrite) NSURL *placeholderImageURL;
+
+@property (nullable, nonatomic, strong, readwrite) AVAsset *asset;
+/**
+ ** @abstract The URL with which the asset was initialized.
+ ** @discussion Setting the URL will override the current asset with a newly created AVURLAsset created from the given URL, and AVAsset *asset will point to that newly created AVURLAsset.  Please don't set both assetURL and asset.
+ ** @return Current URL the asset was initialized or nil if no URL was given.
+ **/
+@property (nullable, nonatomic, strong, readwrite) NSURL *assetURL;
+
+/// You should never set any value on the backing video node. Use exclusivively the video player node to set properties
 @property (nonatomic, strong, readonly) ASVideoNode *videoNode;
 
 //! Defaults to 100
@@ -59,12 +69,16 @@ NS_ASSUME_NONNULL_BEGIN
 //! Defaults to AVLayerVideoGravityResizeAspect
 @property (nonatomic, copy) NSString *gravity;
 
-- (instancetype)initWithUrl:(NSURL*)url;
+#pragma mark - Lifecycle
-- (instancetype)initWithAsset:(AVAsset*)asset;
+- (instancetype)initWithURL:(NSURL *)URL;
+- (instancetype)initWithAsset:(AVAsset *)asset;
 - (instancetype)initWithAsset:(AVAsset *)asset videoComposition:(AVVideoComposition *)videoComposition audioMix:(AVAudioMix *)audioMix;
-- (instancetype)initWithUrl:(NSURL *)url loadAssetWhenNodeBecomesVisible:(BOOL)loadAssetWhenNodeBecomesVisible;
+
-- (instancetype)initWithAsset:(AVAsset *)asset loadAssetWhenNodeBecomesVisible:(BOOL)loadAssetWhenNodeBecomesVisible;
+#pragma mark Lifecycle Deprecated
-- (instancetype)initWithAsset:(AVAsset *)asset videoComposition:(AVVideoComposition *)videoComposition audioMix:(AVAudioMix *)audioMix loadAssetWhenNodeBecomesVisible:(BOOL)loadAssetWhenNodeBecomesVisible;
+- (instancetype)initWithUrl:(NSURL *)url ASDISPLAYNODE_DEPRECATED_MSG("Asset is always loaded when this node enters preload state, therefore loadAssetWhenNodeBecomesVisible is deprecated and not used anymore.");
+- (instancetype)initWithUrl:(NSURL *)url loadAssetWhenNodeBecomesVisible:(BOOL)loadAssetWhenNodeBecomesVisible ASDISPLAYNODE_DEPRECATED_MSG("Asset is always loaded when this node enters preload state, therefore loadAssetWhenNodeBecomesVisible is deprecated and not used anymore.");
+- (instancetype)initWithAsset:(AVAsset *)asset loadAssetWhenNodeBecomesVisible:(BOOL)loadAssetWhenNodeBecomesVisible ASDISPLAYNODE_DEPRECATED_MSG("Asset is always loaded when this node enters preload state, therefore loadAssetWhenNodeBecomesVisible is deprecated and not used anymore.");
+- (instancetype)initWithAsset:(AVAsset *)asset videoComposition:(AVVideoComposition *)videoComposition audioMix:(AVAudioMix *)audioMix loadAssetWhenNodeBecomesVisible:(BOOL)loadAssetWhenNodeBecomesVisible ASDISPLAYNODE_DEPRECATED_MSG("Asset is always loaded when this node enters preload state, therefore loadAssetWhenNodeBecomesVisible is deprecated and not used anymore.");
 
 #pragma mark - Public API
 - (void)seekToTime:(CGFloat)percentComplete;

Source/ASVideoPlayerNode.mm

@@ -22,7 +22,7 @@
 
 static void *ASVideoPlayerNodeContext = &ASVideoPlayerNodeContext;
 
-@interface ASVideoPlayerNode() <ASVideoNodeDelegate>
+@interface ASVideoPlayerNode() <ASVideoNodeDelegate, ASVideoPlayerNodeDelegate>
 {
   __weak id<ASVideoPlayerNodeDelegate> _delegate;
 
@@ -53,11 +53,13 @@ static void *ASVideoPlayerNodeContext = &ASVideoPlayerNodeContext;
     unsigned int delegateVideoPlayerNodeDidRecoverFromStall:1;
   } _delegateFlags;
   
-  NSURL *_url;
+  // The asset passed in the initializer will be assigned as pending asset. As soon as the first
-  AVAsset *_asset;
+  // preload state happened all further asset handling is made by using the asset of the backing
-  AVVideoComposition *_videoComposition;
+  // video node
-  AVAudioMix *_audioMix;
+  AVAsset *_pendingAsset;
   
+  // The backing video node. Ideally this is the source of truth and the video player node should
+  // not handle anything related to asset management
   ASVideoNode *_videoNode;
 
   NSArray *_neededDefaultControls;
@@ -72,7 +74,6 @@ static void *ASVideoPlayerNodeContext = &ASVideoPlayerNodeContext;
   ASStackLayoutSpec *_controlFlexGrowSpacerSpec;
   ASDisplayNode *_spinnerNode;
 
-  BOOL _loadAssetWhenNodeBecomesVisible;
   BOOL _isSeeking;
   CMTime _duration;
 
@@ -95,122 +96,134 @@ static void *ASVideoPlayerNodeContext = &ASVideoPlayerNodeContext;
 
 @dynamic placeholderImageURL;
 
+#pragma mark - Lifecycle
+
 - (instancetype)init
 {
   if (!(self = [super init])) {
     return nil;
   }
 
-  [self _init];
+  [self _initControlsAndVideoNode];
-
-  return self;
-}
-
-- (instancetype)initWithUrl:(NSURL*)url
-{
-  if (!(self = [super init])) {
-    return nil;
-  }
-  
-  _url = url;
-  _asset = [AVAsset assetWithURL:_url];
-  _loadAssetWhenNodeBecomesVisible = YES;
-  
-  [self _init];
 
   return self;
 }
 
 - (instancetype)initWithAsset:(AVAsset *)asset
 {
-  if (!(self = [super init])) {
+  if (!(self = [self init])) {
     return nil;
   }
   
-  _asset = asset;
+  _pendingAsset = asset;
-  _loadAssetWhenNodeBecomesVisible = YES;
-
-  [self _init];
   
   return self;
 }
 
--(instancetype)initWithAsset:(AVAsset *)asset videoComposition:(AVVideoComposition *)videoComposition audioMix:(AVAudioMix *)audioMix
+- (instancetype)initWithURL:(NSURL *)URL
 {
-  if (!(self = [super init])) {
+  return [self initWithAsset:[AVAsset assetWithURL:URL]];
-    return nil;
-  }
-
-  _asset = asset;
-  _videoComposition = videoComposition;
-  _audioMix = audioMix;
-  _loadAssetWhenNodeBecomesVisible = YES;
-
-  [self _init];
-
-  return self;
 }
 
-- (instancetype)initWithUrl:(NSURL *)url loadAssetWhenNodeBecomesVisible:(BOOL)loadAssetWhenNodeBecomesVisible
+- (instancetype)initWithAsset:(AVAsset *)asset videoComposition:(AVVideoComposition *)videoComposition audioMix:(AVAudioMix *)audioMix
 {
-  if (!(self = [super init])) {
+  if (!(self = [self initWithAsset:asset])) {
     return nil;
   }
   
-  _url = url;
+  _videoNode.videoComposition = videoComposition;
-  _asset = [AVAsset assetWithURL:_url];
+  _videoNode.audioMix = audioMix;
-  _loadAssetWhenNodeBecomesVisible = loadAssetWhenNodeBecomesVisible;
-
-  [self _init];
 
   return self;
 }
 
-- (instancetype)initWithAsset:(AVAsset *)asset loadAssetWhenNodeBecomesVisible:(BOOL)loadAssetWhenNodeBecomesVisible
+- (void)_initControlsAndVideoNode
-{
-  if (!(self = [super init])) {
-    return nil;
-  }
-
-  _asset = asset;
-  _loadAssetWhenNodeBecomesVisible = loadAssetWhenNodeBecomesVisible;
-
-  [self _init];
-
-  return self;
-}
-
--(instancetype)initWithAsset:(AVAsset *)asset videoComposition:(AVVideoComposition *)videoComposition audioMix:(AVAudioMix *)audioMix loadAssetWhenNodeBecomesVisible:(BOOL)loadAssetWhenNodeBecomesVisible
-{
-  if (!(self = [super init])) {
-    return nil;
-  }
-
-  _asset = asset;
-  _videoComposition = videoComposition;
-  _audioMix = audioMix;
-  _loadAssetWhenNodeBecomesVisible = loadAssetWhenNodeBecomesVisible;
-
-  [self _init];
-
-  return self;
-}
-
-- (void)_init
 {
   _defaultControlsColor = [UIColor whiteColor];
   _cachedControls = [[NSMutableDictionary alloc] init];
   
   _videoNode = [[ASVideoNode alloc] init];
   _videoNode.delegate = self;
-  if (_loadAssetWhenNodeBecomesVisible == NO) {
-    _videoNode.asset = _asset;
-    _videoNode.videoComposition = _videoComposition;
-    _videoNode.audioMix = _audioMix;
-  }
   [self addSubnode:_videoNode];
 }
 
+#pragma mark Deprecated
+
+- (instancetype)initWithUrl:(NSURL *)url
+{
+  return [self initWithURL:url];
+}
+
+- (instancetype)initWithUrl:(NSURL *)url loadAssetWhenNodeBecomesVisible:(BOOL)loadAssetWhenNodeBecomesVisible
+{
+  return [self initWithURL:url];
+}
+
+- (instancetype)initWithAsset:(AVAsset *)asset loadAssetWhenNodeBecomesVisible:(BOOL)loadAssetWhenNodeBecomesVisible
+{
+  return [self initWithAsset:asset];
+}
+
+- (instancetype)initWithAsset:(AVAsset *)asset videoComposition:(AVVideoComposition *)videoComposition audioMix:(AVAudioMix *)audioMix loadAssetWhenNodeBecomesVisible:(BOOL)loadAssetWhenNodeBecomesVisible
+{
+  return [self initWithAsset:asset videoComposition:videoComposition audioMix:audioMix];
+}
+
+#pragma mark - Setter / Getter
+
+- (void)setAssetURL:(NSURL *)assetURL
+{
+  ASDisplayNodeAssertMainThread();
+  
+  self.asset = [AVAsset assetWithURL:assetURL];
+}
+
+- (NSURL *)assetURL
+{
+  NSURL *url = nil;
+  {
+    ASDN::MutexLocker l(__instanceLock__);
+    if ([_pendingAsset isKindOfClass:AVURLAsset.class]) {
+      url = ((AVURLAsset *)_pendingAsset).URL;
+    }
+  }
+  
+  return url ?: _videoNode.assetURL;
+}
+
+- (void)setAsset:(AVAsset *)asset
+{
+  ASDisplayNodeAssertMainThread();
+
+  __instanceLock__.lock();
+  
+  // Clean out pending asset
+  _pendingAsset = nil;
+  
+  // Set asset based on interface state
+  if ((ASInterfaceStateIncludesPreload(self.interfaceState))) {
+    // Don't hold the lock while accessing the subnode
+    __instanceLock__.unlock();
+    _videoNode.asset = asset;
+    return;
+  }
+  
+  _pendingAsset = asset;
+  __instanceLock__.unlock();
+}
+
+- (AVAsset *)asset
+{
+  AVAsset *asset = nil;
+  {
+    ASDN::MutexLocker l(__instanceLock__);
+    asset = _pendingAsset;
+  }
+  return asset ?: _videoNode.asset;
+}
+
+#pragma mark - ASDisplayNode
+
 - (void)didLoad
 {
   [super didLoad];
@@ -220,38 +233,25 @@ static void *ASVideoPlayerNodeContext = &ASVideoPlayerNodeContext;
   }
 }
 
-- (void)didEnterVisibleState
+- (void)didEnterPreloadState
 {
-  [super didEnterVisibleState];
+  [super didEnterPreloadState];
   
-  ASDN::MutexLocker l(__instanceLock__);
+  AVAsset *pendingAsset = nil;
-  
+  {
-  if (_loadAssetWhenNodeBecomesVisible) {
+    ASDN::MutexLocker l(__instanceLock__);
-    if (_asset != _videoNode.asset) {
+    pendingAsset = _pendingAsset;
-      _videoNode.asset = _asset;
+    _pendingAsset = nil;
-    }
-    if (_videoComposition != _videoNode.videoComposition) {
-      _videoNode.videoComposition = _videoComposition;
-    }
-    if (_audioMix != _videoNode.audioMix) {
-      _videoNode.audioMix = _audioMix;
-    }
-  }
-}
-
-- (NSArray *)createDefaultControlElementArray
-{
-  if (_delegateFlags.delegateNeededDefaultControls) {
-    return [_delegate videoPlayerNodeNeededDefaultControls:self];
   }
 
-  return @[ @(ASVideoPlayerNodeControlTypePlaybackButton),
+  // If we enter preload state we apply the pending asset to load to the video node so it can start and fetch the asset
-            @(ASVideoPlayerNodeControlTypeElapsedText),
+  if (pendingAsset != nil && _videoNode.asset != pendingAsset) {
-            @(ASVideoPlayerNodeControlTypeScrubber),
+    _videoNode.asset = pendingAsset;
-            @(ASVideoPlayerNodeControlTypeDurationText) ];
+  }
 }
 
 #pragma mark - UI
+
 - (void)createControls
 {
   ASDN::MutexLocker l(__instanceLock__);
@@ -313,6 +313,18 @@ static void *ASVideoPlayerNodeContext = &ASVideoPlayerNodeContext;
   });
 }
 
+- (NSArray *)createDefaultControlElementArray
+{
+  if (_delegateFlags.delegateNeededDefaultControls) {
+    return [_delegate videoPlayerNodeNeededDefaultControls:self];
+  }
+
+  return @[ @(ASVideoPlayerNodeControlTypePlaybackButton),
+            @(ASVideoPlayerNodeControlTypeElapsedText),
+            @(ASVideoPlayerNodeControlTypeScrubber),
+            @(ASVideoPlayerNodeControlTypeDurationText) ];
+}
+
 - (void)removeControls
 {
   for (ASDisplayNode *node in [_cachedControls objectEnumerator]) {

examples/ASDKTube/Sample/Models/VideoModel.m

@@ -24,7 +24,7 @@
 {
   self = [super init];
   if (self) {
-    NSString *videoUrlString = @"https://files.parsetfss.com/8a8a3b0c-619e-4e4d-b1d5-1b5ba9bf2b42/tfss-3045b261-7e93-4492-b7e5-5d6358376c9f-editedLiveAndDie.mov";
+    NSString *videoUrlString = @"https://www.w3schools.com/html/mov_bbb.mp4";
     NSString *avatarUrlString = [NSString stringWithFormat:@"https://api.adorable.io/avatars/50/%@",[[NSProcessInfo processInfo] globallyUniqueString]];
 
     _title      = @"Demo title";

examples/ASDKTube/Sample/Nodes/VideoContentCell.m

@@ -69,7 +69,7 @@
     _muteButtonNode.style.height = ASDimensionMakeWithPoints(22.0);
     [_muteButtonNode addTarget:self action:@selector(didTapMuteButton) forControlEvents:ASControlNodeEventTouchUpInside];
 
-    _videoPlayerNode = [[ASVideoPlayerNode alloc] initWithUrl:_videoModel.url loadAssetWhenNodeBecomesVisible:YES];
+    _videoPlayerNode = [[ASVideoPlayerNode alloc] initWithURL:_videoModel.url];
     _videoPlayerNode.delegate = self;
     _videoPlayerNode.backgroundColor = [UIColor blackColor];
     [self addSubnode:_videoPlayerNode];
@@ -142,7 +142,6 @@
 - (void)didTapVideoPlayerNode:(ASVideoPlayerNode *)videoPlayer
 {
   if (_videoPlayerNode.playerState == ASVideoNodePlayerStatePlaying) {
-    NSLog(@"TRANSITION");
     _videoPlayerNode.controlsDisabled = !_videoPlayerNode.controlsDisabled;
     [_videoPlayerNode pause];
   } else {

examples/ASDKTube/Sample/ViewController.m

@@ -87,9 +87,9 @@
     return _videoPlayerNode;
   }
   
-  NSURL *fileUrl = [NSURL URLWithString:@"https://files.parsetfss.com/8a8a3b0c-619e-4e4d-b1d5-1b5ba9bf2b42/tfss-3045b261-7e93-4492-b7e5-5d6358376c9f-editedLiveAndDie.mov"];
+  NSURL *fileUrl = [NSURL URLWithString:@"https://www.w3schools.com/html/mov_bbb.mp4"];
 
-  _videoPlayerNode = [[ASVideoPlayerNode alloc] initWithUrl:fileUrl];
+  _videoPlayerNode = [[ASVideoPlayerNode alloc] initWithURL:fileUrl];
   _videoPlayerNode.delegate = self;
 //  _videoPlayerNode.disableControls = YES;
 //