xbmc / Official-Kodi-Remote-iOS

Full-featured remote control for XBMC Media Center. It features library browsing, now playing informations and a direct remote control.
Other
224 stars 104 forks source link

Improve performance when starting slideshows and updating huge playlists #1077

Closed wutschel closed 3 months ago

wutschel commented 5 months ago

Description

This PR is motivated by an issue raised in this forum post, which describes excessively long loading times when starting picture slideshows with several thousand pictures.

Background:

There are two root causes for the long loading times. First is related to the way how the App start the slideshow compared to how Kodi internal seems to do. The App for any file type will first build a playlist (Playlist.Add) and then play it from the desired position (Player.Open playlistid/pos). This requires the playlist to be built before the playback starts. Kodi internally seems to use a dedicated command to start playback (Player.Open path/random/recursive) which starts the playback before the complete playlist is built. Second cause is the way how Kodi and the App interact while the playlist is being built. The App reacts to Kodi's notification events Playlist.OnClear, Playlist.OnAdd and Playlist.OnRemove. For each event it rebuilds the playlist, which means sending a request to Kodi to provide the complete playlist. This lets Kodi prepare this information and send it back to the App, which then further processes this including animation of playlist. Problem is now starting because Kodi sends multiple notifications when building huge playlists. This results in ping-pong between App and Kodi slowing down building the playlist and letting the App do playlist updates for a long time.

Typical effect seen when starting a slideshow for a folder (including subfolder) with 21k pictures is that playback needs about 150 seconds to start, including permanent and visibly animated playlist updates on App side.

Solution:

The solution has two aspects. First part addresses the playback command: The App now uses Player.Open path/random/recursive for picture playback as well. This reduces the delay until playback starts drastically. It also resolves issues with Kodi crashing after several attempts to play large slideshows via the former method. Second part is addressing the handling of playlist update notifications. The PR implements debouncing of such notifications sent from Kodi. The debouncing is done via (re-)starting the timer debounceTimer each time a new notification arrives. Only after the timer finished the playlist update is executed. For picture playlists the timeout is 1.0 seconds (to support huge playlists), for other playlist types 0.2 seconds.

With these changes the playback starts in 4 seconds and the playlist updates end after 9 seconds.

Remarks:

Additional Changes:

On top of debouncing and slideshow improvements the implementation of NowPlaying was reworked. The state machine was cleanup and now results in reduced playlist flickering and improved handling of slideshows (e.g. allowing the slideshow playlist to progress even with mixed video/picture content). Few more improvements are done:

Summary for release notes

Improvement: debounce playlist update notifications Improvement: quicker slideshow start by using Player.Open on path Improvement: show duration of very short videos as "<1 min" instead of empty string Improvement: do not show aspect ratio background, if no aspect ratio is shared Improvement: add codec icon for Motion JPEG (used by digital cameras) Improvement: do not show progress bar on any item with duration of 0 (e.g. streams or pictures) Improvement: do not show radio station name twice in NowPlaying

wutschel commented 5 months ago

@kambala-decapitator, there are two choices on how to move on with this on. Either we take this as a whole and I update the PR description (would be quite lengthy, but this is a big change), or I split off the performance improvement (first 3 commits) from the state machine / animation rework and make a 2nd PR. Please share your thoughts.

kambala-decapitator commented 3 months ago

@kambala-decapitator, there are two choices on how to move on with this on. Either we take this as a whole and I update the PR description (would be quite lengthy, but this is a big change), or I split off the performance improvement (first 3 commits) from the state machine / animation rework and make a 2nd PR. Please share your thoughts.

the PR is huge, would be great if you could split it

wutschel commented 3 months ago

I would now prefer to keep it as it is. I agree, it grew a lot since it got opened. But the changes base on each other. The only changes I could clearly split off would the first three commits, and that doesn't make a huge difference. A few days back I also updated the PR description to cover all changes.

kambala-decapitator commented 3 months ago

OK, no worries!

wutschel commented 3 months ago

If you're ok with the current mixups I will squash and rebase.

wutschel commented 3 months ago

Took me some time, as the rebase need some more code changes (invalidating timer in the newly implemented handleDidEnterBackground). Additional code changes were

Now squashed and rebased.