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.

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

Fix various memory leaks #41

Merged
merged 2 commits into from
Dec 24, 2020
Merged

Fix various memory leaks #41

merged 2 commits into from
Dec 24, 2020

Conversation

JohannesKauffmann
Copy link
Collaborator

This PR aims to fix various memory leaks such as undisposed media and not unsubscribing from event handlers.

Related issue is #40.

VideoPlayerPageViewModel:
- chg: Instead of lambda's, event handlers are now used for all events,
including timers. The fileNameOverlayTimer has been moved to the top and
made readonly, so it is only initialized once. Also, some comments have
been split over multiple lines.
- fix: The fileNameOverlayTimer is now only once instead of every 5 seconds.

Views/VideoPlayerPage.xaml.cs:
- chg: Unsubscribe from events using event handlers instead of lambda's.
Some comments have been split over multiple lines.
Related issue is #40.

App.xaml.cs:
- add: Global UI dispatcher property, so every class can access the UI conventiently.

VideoPlayerPageViewModel.cs:
- rem: Unneeded using system.diagnostics.

VideoPlayerPage.xaml.cs:
- chg: Dispose the media after playing it by wrapping it in a using. This follows the
official LibVLCSharp UWP sample. Replaced lengthy dispatcher calls with App.Current.
@JohannesKauffmann JohannesKauffmann self-assigned this Dec 24, 2020
Copy link
Member

@TimGels TimGels left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@JohannesKauffmann JohannesKauffmann marked this pull request as ready for review December 24, 2020 13:39
@JohannesKauffmann
Copy link
Collaborator Author

Merging even though it doesn't fix #40. At least there is now a precedent of unsubscribing from event handlers.

@JohannesKauffmann JohannesKauffmann merged commit 41a02be into master Dec 24, 2020
@JohannesKauffmann JohannesKauffmann deleted the fix-memory-leak branch December 24, 2020 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants