added error reporting callback to ASVideoNode #260
Conversation
|
|
|
🚫 CI failed with log |
Source/ASVideoNode.h
Outdated
| ASVideoNodePlayerStatePlaying, | ||
| ASVideoNodePlayerStateLoading, | ||
| ASVideoNodePlayerStatePaused, | ||
| ASVideoNodePlayerStateFinished |
There was a problem hiding this comment.
Please reserve the old indentation.
Source/ASVideoNode.h
Outdated
| * @param videoNode The videoNode. | ||
| * @param currentItem The error that occurs | ||
| */ | ||
| - (void)videoNodeDidFailToInitAssetFor:(ASVideoNode *)videoNode withError:(NSError *)error; |
There was a problem hiding this comment.
To follow convention, let's make it -(void)videoNode:(ASVideoNode *)videoNode didFailToLoadAssetWithError:(NSError *)error.
Source/ASVideoNode.h
Outdated
| * @param currentItem The error that occurs | ||
| */ | ||
| - (void)videoNodeDidFailToInitAssetFor:(ASVideoNode *)videoNode withError:(NSError *)error; | ||
|
|
There was a problem hiding this comment.
There is a newline already. Please remove this one.
Source/ASVideoNode.mm
Outdated
| @@ -152,7 +152,8 @@ - (void)prepareToPlayAsset:(AVAsset *)asset withKeys:(NSArray<NSString *> *)requ | |||
| NSError *error = nil; | |||
| AVKeyValueStatus keyStatus = [asset statusOfValueForKey:key error:&error]; | |||
| if (keyStatus == AVKeyValueStatusFailed) { | |||
| NSLog(@"Asset loading failed with error: %@", error); | |||
| NSLog(@"Asset loading failed with error: %@", error); | |||
| [self.delegate videoNodeDidFailToInitAssetFor:self withError:error]; | |||
There was a problem hiding this comment.
Should we pass along the asset and maybe the key at hand? I imagine those objects might be helpful for investigations. Should be something like -(void)videoNode:(ASVideoNode *)videoNode didFailToLoadValueForKey:(NSString *)key asset:(AVAsset *)asset error:(NSError *)error.
There was a problem hiding this comment.
Btw, please use 2 spaces for indentation.
There was a problem hiding this comment.
Everything is updated.
Source/ASVideoNode.h
Outdated
| @@ -147,6 +147,13 @@ NS_ASSUME_NONNULL_BEGIN | |||
| * @param videoNode The videoNode | |||
| */ | |||
| - (void)videoNodeDidRecoverFromStall:(ASVideoNode *)videoNode; | |||
| /** | |||
| * @abstract Delegate method invoked when an error occurs while trying to play a video | |||
There was a problem hiding this comment.
How about "while trying to load an asset".
|
I've updated the code according to your notes. The only thing that bothers me is that once it's going to swift it gets not pretty:
anyway, swift developers are used to objective-c to swift bridging and naming adventures. |
- preserved two-spaces indentation; - extended error reporting callback with key and asset, updated method call;
|
@petrachkov Good point regarding the Swift API. How can we improve it? |
|
@nguyenhuy Well, you'll have to redesign the whole concept of namings in Texture, e.g. every method of ASVideoNodeDelegate comes with
I don't see a problem here for swift developers as long as there is going to be swift4 and who knows how will it look like then. |
|
OK. Let's merge this PR then. Thanks again! |
|
@nguyenhuy It would be a huge effort, but we could use macros like |
here is a simple implementation of error reporter to ASVideoNodeDelegate. Once asset is failed to load an item - it reports to it's delegate that the error has occured
(huy: Closes #252)