kean / Nuke

Image loading system
https://kean.blog/nuke
MIT License
8.09k stars 526 forks source link

VideoPlayerView notifications don't appear to be removed #818

Open sureshjoshi opened 3 days ago

sureshjoshi commented 3 days ago

I noticed that registerNotifications is called every time the player is set, which happens both on play and on reset.

    private var player: AVPlayer? {
        didSet {
            registerNotifications()
        }
    }

Should this didSet have an unregisterNotifications for when you set nil maybe?

I'm running into a problem where I incessantly have onVideoFinished called every time reset and play a video.

    func play() {
        if let player = videoView.playerLayer.player{
            player.play()
        } else {
            // First run
            videoView.isLooping = configuration?.shouldLoop ?? videoView.isLooping
            videoView.onVideoFinished = { 
                print("onVideoFinished")
            }
            videoView.play()
        }

Every time I close and open/play, I get progressively more logs of onVideoFinished

sureshjoshi commented 3 days ago

As an example, I just scrolled through a handful of videos in a collection view, and when I played my next video, I had about 50 onVideoFinished queued up and printed 🤯

I'm sure this has to do with my views being cached, but I'm also wiping out the handler during cleanup, so it's all strange 😆

sureshjoshi commented 3 days ago

Note: I don't have a full fix yet, but as a quick workaround, I'm manually calling NotificationCenter.default.removeObserver(videoView) after I reset the videoView, and also when I'm doing any cleanup in prepareForReuse or elsewhere.

That has mitigated the problem for the most part today. Will come back to this when I get to re-factor that part of my code in the next few months.