vimeo / VIMVideoPlayer

Deprecated: Please use [PlayerKit]( https://github.com/vimeo/PlayerKit) instead.
MIT License
281 stars 63 forks source link

replaceCurrentItemWithPlayerItem:nil on dealloc - performance considerations #56

Closed richardtop closed 3 years ago

richardtop commented 7 years ago

Issue Summary

replaceCurrentItemWithPlayerItem: on AVPlayer is an expensive operation. It is being called during player's deallocation. Skipping this method improves performance.

Reproduction Steps

To compare performance with and without calling replaceCurrentItemWithPlayerItem: with nil argument on dealloc just comment out that method. It is a part of resetPlayerItemIfNecessary.

Comments

I was not able to find evidence that removing player's item is necessary before deallocating the player, neither I found any bugs by skipping this step. The performance gain will be noticeable when using multiple players in UITableView / UICollectionView or reloading many players frequently (related to issue #39).

I may also assume, that this line of code was put in dealloc intentionally, so I would like to ask whether it can cause any bugs. If there is nothing wrong, I could submit a pull request.

Code

- (void)resetPlayerItemIfNecessary
{
    if (self.item)
    {
        [self removePlayerItemObservers:self.item];
        // I propose to skip following line during -dealloc
        [self.player replaceCurrentItemWithPlayerItem:nil];

        self.item = nil;
    }

    _volumeFadeDuration = DefaultVolumeFadeDuration;
    _playableBufferLength = DefaultPlayableBufferLength;

    _playing = NO;
    _isAtEndTime = NO;
    _scrubbing = NO;
}