Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix issues with notification observer management #62

Closed
wants to merge 4 commits into from
Closed

Fix issues with notification observer management #62

wants to merge 4 commits into from

Conversation

isair
Copy link
Contributor

@isair isair commented Jun 25, 2015

Fixes #61
Fixes #60

@isair isair changed the title Fix attachListeners method's threading issue Fix threading issues in notification observer management Jun 25, 2015
@isair isair changed the title Fix threading issues in notification observer management [WIP] Fix threading issues in notification observer management Jun 25, 2015
@isair
Copy link
Contributor Author

isair commented Jun 25, 2015

@brentvatne The only remaining problem is that the removeObserver call in setRepeatDisabled also removes the observer set for firing the onEnd event.

@isair
Copy link
Contributor Author

isair commented Jun 25, 2015

@brentvatne Let me know what you think about 0e944d0. Basically, I merged the two AVPlayerItemDidPlayToEndTimeNotification handlers into a single method to fix the problem.

@isair isair changed the title [WIP] Fix threading issues in notification observer management Fix threading issues in notification observer management Jun 25, 2015
@isair isair changed the title Fix threading issues in notification observer management Fix issues with notification observer management Jun 25, 2015
@brentvatne
Copy link
Contributor

Looks good to me! I tried it on the example project and it worked as expected, code looks good (these are unfortunately the only "tests" I have in place for this project) squashed into one commit and merged!

44b1711

Also, I really appreciate your help so I gave you a commit bit - you'll see that you're now a "Collaborator" 😄 🎉 🎈

@brentvatne brentvatne closed this Jun 25, 2015
This was referenced Jun 25, 2015
Closed
Closed
@isair
Copy link
Contributor Author

isair commented Jun 25, 2015

Ah, no problem. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@isair @brentvatne