[Discussion] More on reset

[rromanchuk]

I have everything working pretty nicely with the reset i implemented, but i did start to realize that UITableViewCells and UICollectionViews can be pretty tricky. I have a mixed media feed and i noticed my FPS dropped with or without video. I'm starting to profile now, and i'm realizing i'm doing some really stupid things. Like calling a reset method in prepareForReuse even if the cell isn't going to play a video.

I believe replaceCurrentItemWithPlayerItem is done on the main and it's probably a bad idea to be constantly setting nil on every prepareForReuse.

Basically what i need to be able to do is stop and hide the player on prepare for reuse, not tear the entire thing down. Then when cellForRow is called it can then run through the standard setup when a new path runs through the setter. The other problem is the cell has delegation methods to get notified on cellWillAppear and didEndDisplayingCell, and when they get this message they call the appropriate play/pause methods which will hijack the current cell as the Player instance is still just sitting there. I could do a lot of juggling.

I guess it would be nice to somehow mark the controller that it needs a reset, but don't use the main thread for anything until a non nil value is passed to the setter. That means it would also have to ignore any calls between that time.

The combination of reusable views with tableView delegation methods makes this pretty hard.

[piemonte]

hey @rromanchuk

couple things i wanted to mention. i actually don't recommend "reloading" each player video unless a certain set of criteria is met. by loaded, i mean that the thumbnail image view for the video disappears and the video starts playing.

videos should "load" once the the scroll view stops scrolling. once you get that callback, the cells that are visible on screen, should be given the signal to load the URL path and then proceed to fade out the thumbnail image view.

this should make the re-use logic a lot more performant because you're not having to reset the player fully in every case unless scrolling completely stops. :+1:

[rromanchuk]

@piemonte "videos should "load" once the the scrollview stops scrolling." But according to who? You wouldn't say the same thing for photos.... That definitely makes things easier, but there is a significant improvement in user experience if the player starts loading when ios tells the cell it needs to prepare its data in cellForRow.

I also allow for playing while the user is scrolling. The problem arises when a cell is about to be reused, if i don't force a reset here other delegation methods will fire off play signals which will trigger a video that is now out of context. It's also not the responsibility of these delegation methods to manage the cell based on logic from the data source. I still need to use prepareForReuse to do exactly that, but i rather use a flag than actually tearing down the player.

[rromanchuk]

Or maybe an even simpler approach.... allow nil to be passed into path and don't force a hard reset on the player. That way, the only thing i need to do is always set the path in cellForRow, and that's basically it.

Ideally passing nil would put the player in an invalid state, without doing anything too aggressive, besides maybe setting itself into some state like NoContent. The refactoring would be making sure replaceCurrentItemWithPlayerItem is not called for a nil path, and when there is a .NoContent state the player doesn't try to interact with AVPlayer. Seems like natural and expected behavior for anyone trying to set a nil path. Sorry if i'm not making much sense, maybe an example project is needed here.

[mitchellporter]

@rromanchuk How bad is your experience when scrolling with some videos in the feed? I was trying to find a way to allow seamless scrolling w/ autoplaying videos and I didn't want to wait to play on the scrollview decelerating either.

I tried many things and there was always some lag. I then realized that even Vine's feed will catch/lag if you scroll fast enough, and if Twitter hasn't been able to figure it out then my guess is you just aren't meant to quickly scroll a feed full of auto playing videos.

The best approach is to only trigger the video when deceleration is happening.