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/memory leak #269

Merged
merged 82 commits into from
Feb 6, 2024
Merged

Fix/memory leak #269

merged 82 commits into from
Feb 6, 2024

Conversation

forgotvas
Copy link
Contributor

@forgotvas forgotvas commented Feb 1, 2024

This PR contains fix for:

Open any sequence with several video modules and start playing a first video, then choose another module and start playing second video (this step can be repeated many times). Result: first video continues to play when second (third, fourth.....) video(s) start playing.

RPReplay_Final1705666901.MP4

…uality

# Conflicts:
#	Course/Course/SwiftGen/Strings.swift
#	Course/Course/en.lproj/Localizable.strings
#	Course/Course/uk.lproj/Localizable.strings
# Conflicts:
#	Course/Course/Presentation/Outline/ContinueWithView.swift
#	Profile/Profile.xcodeproj/project.pbxproj
@forgotvas forgotvas linked an issue Feb 1, 2024 that may be closed by this pull request
rnr
rnr previously approved these changes Feb 2, 2024
currentProgress(progress, currentSeconds)
}
}

func updateUIViewController(_ playerController: AVPlayerViewController, context: Context) {
DispatchQueue.main.async {
Copy link
Contributor

Choose a reason for hiding this comment

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

@forgotvas As we are here, how about defining updateUIViewController as @MainActor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@saeedbashir to be honestly i don't think that DispatchQueue.main.async required here, because updateUIViewController it's a swiftUI view life cycle method that should guarantee that it will be called from main thread. I will try to test that without dispatch and will commit changes.

Copy link
Contributor Author

@forgotvas forgotvas Feb 5, 2024

Choose a reason for hiding this comment

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

@saeedbashir have removed DispatchQueue.main.async and tested with enabled Main thread checker (enabled by default) and Thread sanitizer diagnostic tools - no problem there.

@forgotvas forgotvas requested review from saeedbashir and rnr February 5, 2024 11:39
@rnr rnr merged commit 1309f7a into openedx:develop Feb 6, 2024
3 checks passed
@rnr rnr deleted the fix/memory-leak branch February 6, 2024 10:56
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.

[iOS] Video continues to play when another video starts
5 participants