Skip to content
This repository has been archived by the owner on Feb 2, 2023. It is now read-only.

Automatically resume ASVideoNode after returning from background #3271

Conversation

plarson
Copy link
Contributor

@plarson plarson commented Apr 13, 2017

AVPlayerLayer pauses video when the application goes into the background. This change resumes the video playing when the app returns.

I'm not sure if there is a way to write tests for application state changes?

AVPlayerLayer pauses video when the application goes into the background. This change resumes the video playing when the app returns.
Copy link
Contributor

@Adlai-Holler Adlai-Holler left a comment

Choose a reason for hiding this comment

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

Hi thanks for the diff @plarson! I'd like to hear from people who know video node better as well @rcancro @binl

@@ -101,6 +101,9 @@ - (instancetype)initWithCache:(id<ASImageCacheProtocol>)cache downloader:(id<ASI
[self addTarget:self action:@selector(tapped) forControlEvents:ASControlNodeEventTouchUpInside];
_lastPlaybackTime = kCMTimeZero;

NSNotificationCenter *notificationCenter = [NSNotificationCenter defaultCenter];
[notificationCenter addObserver:self selector:@selector(applicationDidBecomeActive:) name:UIApplicationDidBecomeActiveNotification object:nil];
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's be sure to remove ourself as an observer in dealloc. Heads up, it's best to remove using the specific method removeObserver:name:object: rather than simply removeObserver: which is much slower 😱

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, got into the habit since iOS9+ automatically deregisters, I'll add the remove step.

{
ASDN::MutexLocker l(__instanceLock__);

if (_shouldBePlaying && ASInterfaceStateIncludesVisible(self.interfaceState)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We should avoid calling -interfaceState while holding the lock, since that method takes the lock. Let's remove the explicit lock, and just call self.shouldBePlaying to keep things simple. We can also use self.visible as a shorthand for ASInterfaceStateIncludesVisible(self.interfaceState).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Other places in ASVideoNode seem to behave the same is why I did it this way, happy to remove it.

- (void)applicationDidBecomeActive:(NSNotification *)notification
{
if (self.shouldBePlaying && self.isVisible) {
[self play];
Copy link
Contributor

Choose a reason for hiding this comment

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

What if the video is paused when the user backgrounds the app? Should we save that state and only start playing on resume if the video wasn't paused?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My understanding is shouldBePlaying will be NO if the video is paused.

@garrettmoon
Copy link
Contributor

Hey, sorry we didn't get this in yet. AsyncDisplayKit is becoming Texture! Would you mind reopening your pull request there: https://github.com/TextureGroup/Texture/pulls

More info about the rename here: https://medium.com/@Pinterest_Engineering/introducing-texture-a-new-home-for-asyncdisplaykit-e7c003308f50

@Adlai-Holler
Copy link
Contributor

Diff has moved!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants