Enqueue the next episode in the playlist

[MoojMidge]

Will enqueue the next episode to the playlist in launch_popup() and, by default, will play local media and addon media (where play_url is provided) as per normal playlist handling method. Will use existing local and addon media playback methods if the next episode is not added to the playlist (not actually checking if playlist item was successfully added, files match, playlist size increased by one, etc. as it didn't seem necessary)

Note that call to self.player.seekTime() in launch_popup() never seemed to work properly (caused the player to seek and continue playing past the file end time) and I'm not sure why this method was used rather than simply skipping to the next playlist item using self.player.playnext(). In any case both methods are now utilised, without any apparent problems on my end, but this may be device/timing dependent.

Queue state is reset (previous playlist item is removed) in the player monitor in the appropriate event callbacks.

This fixes #139

[dagwieers]

@MoojMidge I think my main concern about fiddling around with playlists is that the user (or another add-on) might be changing the playlist during the time we queue and the time it plays the next item. This is probably unwarranted, especially since we do this exact same thing in another add-on (VRT NU).

@mediaminister I would like your opinion here. Would this be a good replacement of our own handling?

[MoojMidge]

@MoojMidge I think my main concern about fiddling around with playlists is that the user (or another add-on) might be changing the playlist during the time we queue and the time it plays the next item. This is probably unwarranted, especially since we do this exact same thing in another add-on (VRT NU).

@mediaminister I would like your opinion here. Would this be a good replacement of our own handling?

@dagwieers I didn't originally think it was necessary to keep track of the size of the playlist, position of the currently playing item, and position of the next enqueued item, but this can be done to ensure that upnext doesn't mess up the playlist for other addons or the user, however unlikely that may be. We are all playing in the same playground after all, tragedy of the commons and whatnot

[mediaminister]

Would this be a good replacement of our own handling?

I tested this and it doesn't interfere with our own handling.

I also tested this with add-ons/plugin.video.vrt.nu#632 but this doesn't work. This can't replace our own handling.

[dagwieers]

This can't replace our own handling.

Is this only because we remove the VRT NU resumepoints when we start the next episode?
I was hoping that seeking to the end would have the same effect, but need to dig into our player code to see if that is feasible.

[dagwieers]

@MoojMidge I would not worry about this too much. I think our pop-up notification prevents the user to modify the playlist. So only when they exit the pop-up without action (escape/return) they would be in this case. For other add-ons, I don't think we can predict how this should be handled. So I would rather wait for any reports in case this occurs. (Our current behaviour could be conflicting with other add-ons as well)

Once we release this, we probably ought to document this behaviour change in our Wiki so add-on developers know what to expect.

[mediaminister]

Is this only because we remove the VRT NU resumepoints when we start the next episode?

I don't know, I just quickly tested and found out play_url doesn't work.

[MoojMidge]

@MoojMidge I would not worry about this too much. I think our pop-up notification prevents the user to modify the playlist. So only when they exit the pop-up without action (escape/return) they would be in this case. For other add-ons, I don't think we can predict how this should be handled. So I would rather wait for any reports in case this occurs. (Our current behaviour could be conflicting with other add-ons as well)

Once we release this, we probably ought to document this behaviour change in our Wiki so add-on developers know what to expect.

@dagwieers Fair enough, but I might need to think a little bit more about this.

A number of other people who reported similar playback issues (in #170 and #199) are using Emby, and this fix wont work for them (and other similar addons) as the play_url method of integrating with UpNext is not being used.

Note that even if an addon is using the play_url method, if the addon does not handle playback in specific way to cater for the calling sequence - from script (UpNext), to playlist, to plugin via plugin:// url, to however the plugin plays media (via builtin PlayMedia, or xbmc.PlayList/Player, or xbmcplugin.setResolvedUrl, or JSON-RPC methods, or whatever) then playback can also fail.

This PR only seems to fully work for local files, might need to revert changes associated with plugin handling as it may fix some plugins but introduce regressions for others.

def queue_next_item(self, episode):
        next_item = {}
        if not self.data:
            next_item.update(episodeid=episode.get('episodeid'))
        # Dont add plugin:// urls to playlist
        # elif self.data.get('play_url'):
        #     next_item.update(file=self.data.get('play_url'))

        if next_item:
            jsonrpc(method='Playlist.Add', id=0, params=dict(playlistid=1, item=next_item))

        return bool(next_item)

Perhaps a Kodi bug report needs to be raised to address the broader issue with closing the video player also killing subsequent requests to play items that are made whilst closing?

[dagwieers]

@MoojMidge Can you report that bug upstream? They require a detailed description and a debug log. And if possible a minimal reproducer. You can report it here: https://github.com/xbmc/xbmc/issues

BTW For VRT NU we implemented playlist handling using the Python API (add-ons/plugin.video.vrt.nu#632) that might not be affected by this?