umbrellaplug / umbrellaplug.github.io

Umbrella Kodi Addon Official
116 stars 17 forks source link

[BUG] Show Information with Continuous Playback Feature Broken on Kodi 21 #396

Closed umbrellaplug closed 3 weeks ago

umbrellaplug commented 1 month ago

Bug

Opening a bug in response to a message I received pointing me to @kodifitzwell issue page where an issue was opened and I was tagged, but the issue is locked so I'm unable to reply there. Not a thing in this response should be taken as being condescending, rude, or anything along those lines. This is all meant in good spirit and ignoring any past. I'm going to try to tag the issue below. I've never tagged an issue from another repo inside my issues so I'm not 100% sure the correct way to do this, but I'll try.

kodifitzwell/repo#7

having the kodi/settings/player/videos "Play next video automatically" set to episodes loads the whole directory of listitems into a playlist. your friends clone depends on this for PlayNext while the original Venom required the user to "Play from here" context menu. normally this isn't a problem except for when a user has show information as the "Default select action" in kodi/settings/media/videos. with both settings set, kodi will send a busted command to the addon to play. as it worked in Nexus this seems like an Omega bug. your friends clone sets this everytime an item is selected if PlayNext is enabled, but it has to be in effect when the item is selected otherwise it just affects the next time. additionally, the way it is coded clobbers any other setting aside from episodes that the user might have set.

The play from here was only applicable in the original venom when attempting to play from a widget as far as I understand. From my conversations with venom and peter the "Play next video automatically" was required for the video to jump to the next video on the playlist when at the end of a video. POV and Fen don't have to worry about this due to the playback monitoring. The playlist generation was done automatically (as you stated) from the episodes in the directory. You are correct that this didn't come into play much until Omega. Except for widgets. I previously wrote the build playlist stuff to account for widgets. Basically when an episode was launched from a widget if they didn't use "Play from Here" the episode directory wasn't built so no playlist was built and it stopped playnext from working because there were no items on the playlist to move into. I tried to adjust for that by doing some detection on the playlist and adding the items to the playlist that were missing if playnext was enabled and only 1item was sent to play. As you can see in the player code some items are resolved and some items are played. What I found was if i tried to resolve single items then add to the playlist kodi lost context for my position in the playlist and left it at -1, to get around this I was using control.player.play(control.playlist) which played the item as a playlist then allowed for me to add to the playlist without losing my context on my current playlist position.

I explain all this so you can understand the many factors I ran into along the way that made the code end up the mess it is. Also the check for if playnext is enabled was moved to play because I had an issue where users were disabling the setting after loading the addon when I had it in the services.py and then opening issues saying "playnext isn't working right" so I moved it to play because I knew the single item playlist code I had written would catch a single item passed in without a playlist and create a playlist and play it so the "play next automatically" was really only after that playlist was built when moving to the next item on the playlist to make sure it played automatically. You might be able to see from previous commits I had that in the onSettingsChange and moved it from there because it was writing/ reading settings so often in that block it was causing some users to end up with blank settings.xml. Once again, just trying to explain some of the logic behind these decisions because I can understand someone looking at it going "why would you do this?"

I pushed some updates to Infinity. removed the check that set "Play next video automatically" and I refactored player.py to simplify the building of the playlist used for PlayNext. most importantly this does not rely on the global setting (changelog encourages the user to disable this) and if the user has show information set, playback works.

I see the changes you made to the playlist building. I like it. I'll be the first to admit that needed to be looked over again. A lot of those changes were patched together over time to fix things or add features like "playnext from widget" or "playnext into multiple seasons" and it's become a mess. I have not tried moving your playlist build function over yet, but I'm interested to see how it will play with the changes i have made testing a workaround for the "show information" bug kodi created with sending blank items to the router when from the info window. sending it to the episode directory building function was a good idea though, I will be making that change for sure as I believe it could solve another problem I'm running into when meta is missing.

the way I coded the playlist building, it will still work if "Play next video automatically" is enabled, but will not work with show information set (which is where we started at).

I'm thinking with my workaround I put into the router, it may work correctly but I have not had a chance to combine it yet. As short/ sweet as I can put it, I think I might have a viable solution to that issue. You will likely consider this hacky or bandaid versus fixing the problem as you have stated publicly in the past and I understand that, but I cannot fix a problem/ quirk kodi created so I'm doing my best to work around that and keep the addon working for the users, even if it isn't the prettiest way to do it. I found that when play is clicked from the info window it was sending a blank url to the router. This was somehow causing the VIDEO_TS nonsense to show up. below is the code I added into my router to catch those empty urls and correct them if you are on an episode list.

      try:
        if argv2 == '':
            import xbmc
            argv4 = xbmc.getInfoLabel('ListItem.FileNameAndPath')
            argv5 = argv4.split('plugin://plugin.video.umbrella/?')[1]
            params = dict(parse_qsl(argv5.replace('?', '')))
            episode = params.get('episode')
            if params.get('action') == 'play_Item' and episode:
                from resources.lib.modules import log_utils
                log_utils.log('Umbrella Hacky Workaround is being applied. Thanks Adam', 1)
                argv4 = argv4.split('plugin://plugin.video.umbrella/?')[1]
                argv2 = argv4
    except:
        argv2 = argv2

your friends clone sets the addon setting episode playback to autoplay in that same function that sets the global settings. this is mostly inconsequential as the addon is hard coded to autoplay if PlayNext is enabled. this is mostly a visual and personal preference, but have chosen to leave it whatever the user sets it at independent of PlayNext setting.

This is unclear to me what you're trying to say here. I set the setting back to autoplay on purpose. I think this just may be a matter of preference or opinion, but I feel like if I'm forcing that setting it should be set that way also. I'm open to suggestions though. I don't know if this really matters much in the total scope of this entire discussion though.

ideally, I chose this path because its my opinion that the addon not have to rely on the global setting or context menu for PlayNext. hopefully, the kodi team will revert code back to Nexus so that with both settings enabled, it will work.

i don't disagree, but I would rather get it working if it's possible. I personally don't use this feature at all, but I have been asked many times now to look at this so if I can get it working and clean up playnext it's a win-win. Hopefully that's understandable.

I've looked over some of the changes you made to the player.py in the process of preparing to respond and I wanted to give you an "olive branch" of sorts. line 986 you've changed it to:

offset = str(match[1])

This is going to cause your resume to break. If you test, you likely get a resume popup, but then when you hit resume it still starts at the beginning. That needs to be a float to compare the resume times correctly. I'm not trying to pick your code apart or anything. I just thought it would be nice to help you avoid an issue I had found and already corrected that I noticed. In my player.py that has been changed to:

offset = float(match[1])

Hopefully none of this comes across the wrong way, tone is never clear in text. Honest question though, why did you tag me in your issue then lock it so I couldn't respond?

Please any typos or nonsense in this it's been a very long day.

Expected Result

Hoping to fix some bugs.

Steps To Reproduce

There's a lot of steps for different scenarios here.

Kodi Version, Type of Device, Logs, Screenshots, and Additional Info

Kodi 21

Checklist

umbrellaplug commented 1 month ago

@adamosborne83 Tagging you as you were also tagged in the original issue. I'm not sure why.... 🤣

adamosborne83 commented 1 month ago

@adamosborne83 Tagging you as you were also tagged in the original issue. I'm not sure why.... 🤣

Because...... you're the brains, and I'm the looks.... with a great deal of customer relations and love to share.... muppet

kodifitzwell commented 1 month ago

Honest question though, why did you tag me in your issue then lock it so I couldn't respond?

you did not respond to the other thread (the spaces in resolved links) I tagged you in so I didn't know if you even got notices. that was unlocked for days. honest response, I just did not want this to turn into anything like our previous arguments.

This is going to cause your resume to break.

I forgot why exactly I changed the code back to offset = str(match[1]), perhaps just for some consistency with something else I did. however, in onAVStarted I changed the code to account for that:

self.offset = Bookmarks().get(name=self.name, imdb=imdb, tmdb=tmdb, tvdb=tvdb, season=season, episode=episode, year=self.year, runtime=meta.get('duration') if meta else 0)
...

def onAVStarted
    ...
    else:
        try: self.offset = float(self.offset)
        except: self.offset = 0
    self.seekTime(self.offset)

admittedly, I didn't test it out with Infinity, so maybe I should test it. but this matches the code I use in Dradis, where I have tested it as working.

I'm thinking with my workaround I put into the router,

looking at the code it looks like it should work. I would suggest control.infoLabel given control is already imported instead of another import xbmc...xbmc.getInfoLabel. I agree this is at root a kodi issue, so there is no choice but to band-aid it or hack it.

The play from here was only applicable in the original venom when attempting to play from a widget as far as I understand.

I understand it differently, the quote from playnext.txt says nothing about widgets.

If enabled Episodes are [COLOR red][B]"hard coded to Auto Play"[/B][/COLOR].  Also note the UpNext addon has been removed
as of v6.0.0.  There is a new Venom window xml to handle this internaly now and is more reliable.
AddonSignals dependency has also been removed.  You can safely removes both if not used elsewhere.
You must use the Kodi context menu option [COLOR red][B]"Play from Here"[/B][/COLOR] for continual playback and PlayNext
popup notification.  You can also set Kodi's settings to use [COLOR red][B]"Play from Here"[/B][/COLOR] as default select method
for Episodes.  See [COLOR red][B]"System - Settings - Player - Videos - Play next video automatically"[/B][/COLOR].  Then
select [B][COLOR red]Episodes[/B][/COLOR].

NOTE: If you call up [B]Source Select[/B] continual playback will not function.  Continual playback ONLY operates in AutoPlay mode per above.

either "Play from here" or the kodi global setting will allow for continuous playback. if enabled and if the addon PlayNext setting is not enabled, the user just won't be notified by popup that it's coming. that is to say, kodi will just move from current to next in the playlist if playlist is greater than 1. I believe this because with either "Play from here" in the context menu or enable the kodi global setting, once you play an item, check the playlist with either the sideblade or "c" key; the entire listitem directory is loaded even if PlayNext in the addon is disabled. if you look at the playnext window source, it just tells the kodi player to move forward (now as opposed to waiting for the video to organically end), stop the player (now), or close depending on what the user clicks.

def onClick(self, control_id):
    if control_id == 3011: # Play Now, skip to end of current
        playerWindow.setProperty('infinity.playnextPlayPressed', str(1))
        from resources.lib.modules import log_utils
        log_utils.log('PlayNext Play Button! Playlist Position: %s Playlist Count: %s ' % (control.playlist.getposition(), control.playlist.size()), log_utils.LOGDEBUG)
        xbmc.executebuiltin('PlayerControl(BigSkipForward)')
        self.doClose()
    if control_id == 3012: # Stop playback
        xbmc.executebuiltin('PlayerControl(Playlist.Clear)')
        xbmc.executebuiltin('PlayerControl(Stop)')
        playerWindow.clearProperty('infinity.preResolved_nextUrl')
        self.doClose()
    if control_id == 3013: # Cancel/Close xml dialog
        self.doClose()

I saw where your code checks for a playlist and then plays that. I would rather just have kodi play what the heck the addon tells it to.

that's why I believe moving away from relying on either context menu or kodi global setting is the way to go forward. the updated code only builds an item to play next if the user has the addon setting enabled. if anything, add some code that checks if the global setting is set and clears the playlist (if the addon setting is disabled) so users don't start reporting "hey PlayNext is disabled but it plays continuously with no popup" because either of the other conditions are in place.

and to circle around to the band-aid or hack issue, that's not required if we can encourage users the global setting is to be disabled. what other addon requires the global setting? Fen/Venom derivatives are arguably the most popular, so if those are on the same page that would be cool I guess.

I haven't tested it, but I believe this might not break playlist creation if a user adds a few movies or episodes. checkPlaylist always clears the playlist if its a movie, so no movie playlists.

Except for widgets...

admittedly, I didn't even consider widget use. I don't test anything but the default skin so sometimes I forget widget things might be different. widgets might blow up everything or nothing.

so I guess the questions I am left with regarding widgets is why the original dev clears the playlist at the start of tvshowDirectory, you only clear it under certain conditions. and same for episodeDirectory, the original dev clears it only for a widget and you commented it out. and I only figured that out because when I tried to port the code to Dradis it didn't work at first because that line was not commented out. the fact that you commented it out is critical to my whole solution.

thanks for the collaboration,

umbrellaplug commented 1 month ago

you did not respond to the other thread (the spaces in resolved links) I tagged you in so I didn't know if you even got notices. that was unlocked for days. honest response, I just did not want this to turn into anything like our previous arguments.

i'm trying to keep all my responses in context so I apologize in advance for quoting you a bunch here. You're correct, I never responded to the other thread because I had you blocked so I never got any notifications. I didn't actually see that until several days after you had posted it. All my information for that was based around a reddit thread I was pointed to. In the spirit of moving past things, I'll leave it at "it was a disappointing read for me" but I've decided just not to respond to those types of things anymore. all that being said, i was pointed to your issues by adam yesterday and decided it would be best for the community to respond and try to move forward. i'll repeat my comments that I liked your changes to episodes. there were a few things I noticed. global setting for kodi version was one and i hadn't thought to replace all those control.getKodiVersion() calls with one global. That's a small thing that will make a noticeable difference as it's used for every episode list.. the playlist and index variables all being passed into the episode directory function were all nice as well. Credit where credit is due. Those are good changes, I'll use those, I'll make sure I give you credit for it.

I forgot why exactly I changed the code back to offset = str(match[1]), perhaps just for some consistency with something else I did. however, in onAVStarted I changed the code to account for that

Awesome, just something I wanted to point out to try to help. If you've accounted for it already, great. I don't have Infinity installed so I haven't tested it.

looking at the code it looks like it should work. I would suggest control.infoLabel given control is already imported instead of another import xbmc...xbmc.getInfoLabel. I agree this is at root a kodi issue, so there is no choice but to band-aid it or hack it.

Yes, we agree here. This is one of those things I'm surprised it worked. I put it into the router to test and see if I could try to resolve the blank item being sent and it worked. This was a 5 second "let's see if I can grab the listitem if it's blank". I expected crazy results, but so far it's working pretty well. There's been several adjustments I've had to make for building the playlist with this so I'm interested to see how it plays when I remove all of that logic and just put in your cleaner buildPlaylist. I suspect there may need to be some logic done on the playlist still to make it play nice but I will test when I get a chance and change it to use the existing control method.

I understand it differently, the quote from playnext.txt says nothing about widgets.

Best reply I can give to this is I need to test it and see what the results are. This is where the real differences in Fen/ Venom/ Exodus based stuff comes out. I can think of a few scenarios I will try to test:

With "Play next video automatically" changed. Opening an episode from within the addon and seeing if at the end it moves into the next episode, if the playnext window shows, and if it does this without a playlist or with one being created.

Originally in venom if you opened an episode from within the episode list and the settings play next video automatically was set to episodes... a playlist was created from the list of episodes in the directory, the item playing on that playlist was automatically set to the correct item and playnext window popped up every time and it automatically moved into next episode till end of episodes that were in that directory. If you opened an episode from a widget (think artic fuse where every season is made into a widget in some views before you ever get into the actual addon season view) the playlist was not built, only one item was added to a single playlist and it did not automatically move into the next episode or show the playnext popup. If you used "play from here" on that same widget playnext would work as expected. Now I know that's a lot to unravel, but I'll be interested to see what happens with these changes.

that's why I believe moving away from relying on either context menu or kodi global setting is the way to go forward. the updated code only builds an item to play next if the user has the addon setting enabled. if anything, add some code that checks if the global setting is set and clears the playlist (if the addon setting is disabled) so users don't start reporting "hey PlayNext is disabled but it plays continuously with no popup" because either of the other conditions are in place.

I think you're suggesting moving away from the "Play next video automatically" being forced to be set to episodes. I guess my biggest concern here is "if play next video automatically" is not set to episodes I'm not sure any of the playnext stuff is going to work correct.

and to circle around to the band-aid or hack issue, that's not required if we can encourage users the global setting is to be disabled. what other addon requires the global setting? Fen/Venom derivatives are arguably the most popular, so if those are on the same page that would be cool I guess.

I guess this would depend on the behavior after the changes above and testing. It seems like these settings are all a house of cards type scenario where when one is removed the whole "playnext" setup starts to collapse. This is all really discussion at this point though because I need to make the code changes and see how it behaves.

I haven't tested it, but I believe this might not break playlist creation if a user adds a few movies or episodes. checkPlaylist always clears the playlist if its a movie, so no movie playlists.

Yeah, checkPlaylist was not meant to play well with movies or other playlists, but I will be removing it in the process of trying your changes. Basically, no respect or thought was ever given to a situation where a user might be trying to use a playlist. I'm not saying this is right, this is just the logic behind that. Do you have users making playlists of movies and trying to use them or scenarios where this might be applicable? I ask because if I need to put the checkPlaylist function back in but use different logic I'd like to consider these things.

admittedly, I didn't even consider widget use. I don't test anything but the default skin so sometimes I forget widget things might be different. widgets might blow up everything or nothing.

Yeah, widgets are likely going to be a whole different issue. I'll try with multiple scenarios like I mentioned above with different settings. Now you see why Peter always stayed away from skin support as much as possible. 😄

so I guess the questions I am left with regarding widgets is why the original dev clears the playlist at the start of tvshowDirectory, you only clear it under certain conditions. and same for episodeDirectory, the original dev clears it only for a widget and you commented it out. and I only figured that out because when I tried to port the code to Dradis it didn't work at first because that line was not commented out. the fact that you commented it out is critical to my whole solution.

I removed the clear because it was causing me issues and I only wanted the playlist cleared in certain conditions. I was having issues where people would play one series then switch to a different show and hit play but it would play the pre-resolved url from the last show because the playlist had not been cleared correctly. I believe the original code on the tvshow for clearing the playlist was in to avoid this happening.

thanks for the collaboration,

thanks for your response. sometimes it's good to just be able to discuss ideas or the logic behind decisions.

kodifitzwell commented 1 month ago

I pushed an update with an updated buildSeasonPlaylist. I like it better than the first version because now episodeDirectory just builds a directory or returns listitems. buildSeasonPlaylist is now responsible for adding those listitems to the playlist at the correct index.

I posted a video of a test in the issue I created on my github, the only thing I have enabled is PlayNext in the addon.

outside of yet to be determined widget issues, the only scenario that I tested where the addon will have a problem with is where PlayNext is enabled and a user creates a playlist of specific episodes. after the first episode the next will be inserted before the users selected next episode and rinse/repeat. so say if they only want episodes 1, 3, 5 out of a 10 episode season, obviously they have to disable PlayNext. otherwise they will end up with an almost never ending playlist.

Do you have users making playlists of movies and trying to use them or scenarios where this might be applicable?

I have not, but just something I noticed that now worked.

Now you see why Peter always stayed away from skin support as much as possible. 😄

yeah the fact the you created half a dozen different PlayNext windows for different skins blows my mind. much more ambitious than I.

sounds like you have a bit of coding and a bunch of testing ahead, have a good one...

adamosborne83 commented 1 month ago

sounds like you have a bit of coding and a bunch of testing ahead, have a good one...

Yes I do have a lot of testing ahead hahaha

umbrellaplug commented 1 month ago

@kodifitzwell Your issue is still locked so I'll continue replying here. Looks like it's broken from a widget. I'll do some more testing and see what's happening. Should be a pretty simple fix though.

Edited to update. It looks like when played from a widget. It's the same as what I was previously experiencing where the position of the playlist is completely lost.

kodifitzwell commented 1 month ago

what skin and how are the widgets set up? I posted another video on my issues page of Fentastic and AH2 set up with a widget.

I tried to set up a similar widget in Artic Fuse, but got so frustrated that I couldn't quickly figure it out that I just blew up my installation.

umbrellaplug commented 1 month ago

I'm using arctic fuse as it's the one I usually get issues opened for. also, YES.... I'm with you. These things are so many steps and bullshit to setup I get so frustrated. I have never understood the skins.

I'm attaching a video here showing what infinity and umbrella are both doing when launching an episode from within the addon vs a widget. Half the screen is kodi, half it setup with the remote from browser so you can see the playlist and what's happening. The video is kinda long, but it shows from widget with the issue then from within addon also so you can see what it should look like. I can try AH2 or Fentastic also and see if I get the same results. I made the widget from trending shows, but it doesn't matter. With that theme every "season" is a widget if you don't drill down into it. Easy way to spot it is the "show info" doesn't work from widgets.

https://vimeo.com/996003689

umbrellaplug commented 1 month ago

Here's AH2.

https://vimeo.com/996006250

umbrellaplug commented 1 month ago

Hey, I looked over all your videos and they are starting with a tv show widget, but moving into the addon. You have to launch the episode from a widget to see the bug. If you watch my videos you can see me do it with the "seasons" view. Sorry, I totally understand the frustration of trying to duplicate this. Hopefully you're starting to see why the code was so insane. Maybe trying it with in progress episodes with direct progress scrape. That might show you the bug.

kodifitzwell commented 1 month ago

I finally figured out how to set up some widgets in AF. heres what I found.

at this point tvshowDirectory has been called and is_widget is True.

tvshow

when you click into it and then highlight a season episodeDirectory is called and is_widget is False.

episode

when an item is played from here, the playlist must be cleared somehow after the selected listitem is added to the playlist but before our function plays it. I know because I added this logging to player.py.

control.log(f"playlist size1: {control.playlist.size()}", 1)
if debridPackCall: control.player.play(url, item) # seems this is only way browseDebrid pack files will play and have meta marked as watched
else: control.resolve(int(argv[1]), True, item)
control.log(f"playlist size2: {control.playlist.size()}", 1)
if self.media_type == 'episode' and self.enable_playnext: self.buildSeasonPlaylist()
control.log(f"playlist size3: {control.playlist.size()}", 1)

and every single time I get this:

info <general>: playlist size1: 0
info <general>: playlist size2: 0
info <general>: VideoPlayer::OpenFile: plugin://plugin.video.infinity/?action=play_Item&title=Cherry...
info <general>: Creating InputStream
info <general>: playlist size3: 1

the next episode is added, PlayNext never fires because it looks for playlist size > 1. and no combination or PlayNext on/off and/or "Play next video automatically" Episodes on/off will generate a playlist with that selected item from here. so this behavior is definitely odd. also notice that in this container "Play from here" is not in the context menu.

if the code in your current version is able to build a playlist after all that, then major kudos because it really shouldn't have to be that difficult.

however when I click into the season episodeDirectory gets called again and is_widget is False. this time when playing the same item I get this:

info <general>: playlist size1: 1
info <general>: playlist size2: 1
info <general>: playlist size3: 2
info <general>: VideoPlayer::OpenFile: plugin://plugin.video.infinity/?action=play_Item&title=Cherry...

the playlist starts out with the selected item, the next episode gets added and everything behaves. also notice that in this container "Play from here" is in the context menu.

ultimately, the skin dev explain why playing from that container does not behave the same way as when you click into the season and play. I think it should be fixed to act the same or what kind of container is that even? I wonder what jurial would say if anything at all.

I might try some work arounds later weekend.

adamosborne83 commented 1 month ago

@jurialmunkey I am sorry to tag you in an issue, and I will happily put it into Arctic Fuse, but can you add anything though around this?

umbrellaplug commented 1 month ago

@kodifitzwell I was able to get around this by putting checkPlaylist back into the code. I'll attach my player.py here. I haven't pushed this to my test repo yet or I'd say just grab it from there.

let me know what you think. this is pretty much full circle back to I had it originally, just a little cleaner. I'm checking for a playlist offset of -1 and for the item to be an episode. Adam reported that when it was left open to movies and episodes it was clearing the movie from the playlist before it can be played.

basic concept is the playlist check looks for a playlist offset of -1 and episode. if it finds that, it clears the playlist and re-creates a new one correctly then sends back true if it had to make changes or false if it didn't. if the playlistchecked value is true then I send to control.play with the playlist instead of resolving the item. This results in the playlist showing the correct offset for the item playing and allows for the code to add the next episode to the playlist without incident.

~~i'm thinking about it now after typing this and I thinking it through, I'm not even su re the clear.playlist is even necessary. I need to test that. It may just be a matter of playing with control.play vs resolve if it came from a widget like that.~~

player.zip

edited

Need to walk back that last statement. The control.playlist.clear() is essential.

kodifitzwell commented 1 month ago

last night I would occasionally get a spinner in front of the resolve window while the episodes were being gathered to add to the playlist. I figured out the function could be moved up and the unified scrape/resolve window would go uninterrupted.

I was oblivious to being able to test the position -1. so I think we can put it all together like this:

        ...
        item.setProperty('IsPlayable', 'true')
        if self.media_type == 'episode' and self.enable_playnext: self.buildSeasonPlaylist(url, item)
        if debridPackCall: control.player.play(url, item) # seems this is only way browseDebrid pack files will play and have meta marked as watched
        elif control.playlist.getposition() == -1: control.player.play(control.playlist)
        else: control.resolve(int(argv[1]), True, item)
        homeWindow.setProperty('script.trakt.ids', jsdumps(self.ids))
        ...

def buildSeasonPlaylist(self, link, link_item):
    try:
        if control.playlist.getposition() == -1:
            control.player.stop()
            control.playlist.clear()
            control.playlist.add(url=link, listitem=link_item)
        from resources.lib.menus import seasons, episodes
        seasons = seasons.Seasons().tmdb_list(tvshowtitle='', imdb='', tmdb=self.tmdb, tvdb='', art=None)
        seasons = [int(i['season']) for i in seasons]
        ep_data = [episodes.Episodes().get(self.meta.get('tvshowtitle'), self.meta.get('year'), self.imdb, self.tmdb, self.tvdb, self.meta, season=i, create_directory=False) for i in seasons]
        items = [i for e in ep_data for i in e if i.get('unaired') != 'true']
        index = next((idx for idx, i in enumerate(items) if i['season'] == int(self.season) and i['episode'] == int(self.episode)), None)
        if index is None: return
        try: item = items[index+1]
        except: return
        if item.get('season') and item.get('season') > int(self.season) and not self.multi_season: return
        items = episodes.Episodes().episodeDirectory([item], next=False, playlist=True)
        for url, li, folder in items: control.playlist.add(url=url, listitem=li)
    except: log_utils.error()

even when you clear and build the new playlist, the .getposition() still remains -1, so that's what I tested for playing the playlist.

player.py: https://paste.kodi.tv/qajixilehi.kodi

hopefully jurial still responds about why...

umbrellaplug commented 1 month ago

last night I would occasionally get a spinner in front of the resolve window while the episodes were being gathered to add to the playlist. I figured out the function could be moved up and the unified scrape/resolve window would go uninterrupted.

I was oblivious to being able to test the position -1. so I think we can put it all together like this:

        ...
        item.setProperty('IsPlayable', 'true')
        if self.media_type == 'episode' and self.enable_playnext: self.buildSeasonPlaylist(url, item)
        if debridPackCall: control.player.play(url, item) # seems this is only way browseDebrid pack files will play and have meta marked as watched
        elif control.playlist.getposition() == -1: control.player.play(control.playlist)
        else: control.resolve(int(argv[1]), True, item)
        homeWindow.setProperty('script.trakt.ids', jsdumps(self.ids))
        ...

def buildSeasonPlaylist(self, link, link_item):
    try:
        if control.playlist.getposition() == -1:
            control.player.stop()
            control.playlist.clear()
            control.playlist.add(url=link, listitem=link_item)
        from resources.lib.menus import seasons, episodes
        seasons = seasons.Seasons().tmdb_list(tvshowtitle='', imdb='', tmdb=self.tmdb, tvdb='', art=None)
        seasons = [int(i['season']) for i in seasons]
        ep_data = [episodes.Episodes().get(self.meta.get('tvshowtitle'), self.meta.get('year'), self.imdb, self.tmdb, self.tvdb, self.meta, season=i, create_directory=False) for i in seasons]
        items = [i for e in ep_data for i in e if i.get('unaired') != 'true']
        index = next((idx for idx, i in enumerate(items) if i['season'] == int(self.season) and i['episode'] == int(self.episode)), None)
        if index is None: return
        try: item = items[index+1]
        except: return
        if item.get('season') and item.get('season') > int(self.season) and not self.multi_season: return
        items = episodes.Episodes().episodeDirectory([item], next=False, playlist=True)
        for url, li, folder in items: control.playlist.add(url=url, listitem=li)
    except: log_utils.error()

even when you clear and build the new playlist, the .getposition() still remains -1, so that's what I tested for playing the playlist.

player.py: https://paste.kodi.tv/qajixilehi.kodi

hopefully jurial still responds about why...

With it set up like this my playlist is still losing context of the position if i call the control.play before clearing the playlist when launching from the different widgets. The only was I was able to keep context of the playlist was to clear it, add the item to the playlist, then call the control.play(). if I tried to call the control.play() on the playlist before clearing it and readding the item it still was at a -1 position. Are you getting different results?

I'm also very interested to hear jurial's input on this. I don't know if he will reply on an issue here due to my repo being one of the banned repos. We may need to open an issue with him to get his feedback.

kodifitzwell commented 1 month ago

yeah the playlist creation is in the correct order and the indicator is showing the right file playing at the correct time, every time. however if I stop E1 (where E2 is next up) too soon and jump to E7, the playlist shows E7 playing and E8 up next, but in the log the bare url curl sends is E2. so I think somewhere preResolved_nextUrl is not being cleared quick enough. I'll have to try the way you say. I tried logging the start of all the onPlayBackStopped, onPlayBackEnded, onPlayBackError and they are rarely called when you quick stop the player, that's where the clearProperties are.

regarding that container in AH2, I tried another test. if you disable PlayNext/enable Autoplay and turn on "Play next video automatically/Episodes" and play an item, no playlist is generated. Autoplay is important to have on, because there is a playlist.clear() in sources.py/sourceSelect. however if you click into the season and play an item, the entire list directory is loaded as a the playlist as expected. when that "widget" container built, is_widget returns False, so it's not a widget but its not a normal directory either. I wonder what voodoo he's using.

umbrellaplug commented 1 month ago

One thing I'm noticing is a delay from the way it's being done on my side now. I think buildPlaylist may need to be moved to be the last function so that it's not waiting on the playlist being built to play the item after it's been selected. I'll mess around with it more today and see if I can nail it down better.

kodifitzwell commented 1 month ago

these are the only changes I've made to the code I posted:

episodes.py:

if not is_widget and not playlist: control.playlist.clear()

player.py:

elif control.playlist.getposition() == -1 and control.playlist.size(): control.player.play(control.playlist)

I'm noticing is a delay from the way it's being done on my side now. I think buildPlaylist may need to be moved to be the last function so that it's not waiting on the playlist being built to play the item after it's been selected.

how long of a delay? most data should be cached. timing inside buildPlaylist is only taking me generally < 300ms max. even with a cleared cache and jumping to season 35 of the Simpsons, it was ~2.5s.

preResolved_nextUrl wasn't getting cleared because I was exiting too early, needs to be at least 180s played or FF past that point. that fixed issues when skipping ahead episodes after stopping playback.

if (int(self.current_time) > 180 and (self.getWatchedPercent() < 85)): # kodi may at times not issue "onPlayBackStopped" callback
    self.playbackStopped_triggered = True
    self.onPlayBackStopped()

so I'm thinking of not calling the prescrape until after that also. haven't decided yet.

had a terrible time with RD. curl 503 errors so the playback is slow and at times it fails. when it fails the player still jumps to the next episode. might add a control.player.stop() to onPlayBackError.

you have a lot more testers, so should be interesting to see what else pops up.

adamosborne83 commented 1 month ago

these are the only changes I've made to the code I posted:

episodes.py:

if not is_widget and not playlist: control.playlist.clear()

player.py:

elif control.playlist.getposition() == -1 and control.playlist.size(): control.player.play(control.playlist)

I'm noticing is a delay from the way it's being done on my side now. I think buildPlaylist may need to be moved to be the last function so that it's not waiting on the playlist being built to play the item after it's been selected.

how long of a delay? most data should be cached. timing inside buildPlaylist is only taking me generally < 300ms max. even with a cleared cache and jumping to season 35 of the Simpsons, it was ~2.5s.

preResolved_nextUrl wasn't getting cleared because I was exiting too early, needs to be at least 180s played or FF past that point. that fixed issues when skipping ahead episodes after stopping playback.

if (int(self.current_time) > 180 and (self.getWatchedPercent() < 85)): # kodi may at times not issue "onPlayBackStopped" callback
    self.playbackStopped_triggered = True
    self.onPlayBackStopped()

so I'm thinking of not calling the prescrape until after that also. haven't decided yet.

had a terrible time with RD. curl 503 errors so the playback is slow and at times it fails. when it fails the player still jumps to the next episode. might add a control.player.stop() to onPlayBackError.

you have a lot more testers, so should be interesting to see what else pops up.

I spent about 3 hours this morning trying to break it, I couldn't unfortunately. I did notice a slight delay but talking like 1 second. Sent a couple of logs, but literally nothing to cause drama. Seems to be solid atm.

umbrellaplug commented 1 month ago

if not is_widget and not playlist: control.playlist.clear()

interesting.. i'll see how this plays with mine.

you have a lot more testers, so should be interesting to see what else pops up.

lol. i had 4 good testers... 1 has quit using kodi for the most part to farm. (yes really) 1 decided it was super important to war with you on reddit. He was asking me things on discord then copy and pasting my reply to reddit. When I asked him to please stop driving the animosity up and posting my replies on reddit he deleted all his accounts I had to communicate with him. Adam is useless. 😄 Joking of course, he will be the one to find any issues with skin stuff and has suggested it's likely it may be the weekend before we see a reply from Jurial.

I'll ask my other tester to try it out and see what feedback we can get. Might be worth making a reddit post. (yuck)

umbrellaplug commented 1 month ago

@kodifitzwell I've implemented your changes. I had a question though. What's the logic behind? elif control.playlist.getposition() == -1 and control.playlist.size(): control.player.play(control.playlist)

My understanding of the control.playlist.size() is that it will return an int. Are you looking for any value or should that be control.playlist.size() == 1?

kodifitzwell commented 1 month ago

yeah I saw that account was deleted.

if you wanted to shorten up the seasons, only current and next are really needed:

season_range = (int(self.season), int(self.season) + 1)
seasons = seasons.Seasons().tmdb_list(tvshowtitle='', imdb='', tmdb=self.tmdb, tvdb='', art=None)
seasons = [int(i['season']) for i in seasons if int(i['season']) in season_range]

speed wise it might make fractions of seconds difference only for a show like the Simpsons where there are dozen of eps per season and 30+ seasons.

What's the logic behind?

yes to an int. int 0 is False-ish in python. could have just control.playlist.size() > 0. if the function fails to create a playlist size will be 0, which will evaluate to False in that test. so it will not try and play an empty playlist and the else will just play the item. personal pref, maybe its better just to not play anything.

umbrellaplug commented 1 month ago

yeah I saw that account was deleted.

yeah, shame. he was a good tester. i think there's some mental health issues in the kodi community. that's not meant as a dig towards anyone.

if you wanted to shorten up the seasons, only current and next are really needed:

season_range = (int(self.season), int(self.season) + 1)
seasons = seasons.Seasons().tmdb_list(tvshowtitle='', imdb='', tmdb=self.tmdb, tvdb='', art=None)
seasons = [int(i['season']) for i in seasons if int(i['season']) in season_range]

speed wise it might make fractions of seconds difference only for a show like the Simpsons where there are dozen of eps per season and 30+ seasons.

thanks, I don't see a point i think it's fast enough building the episode playlist first. it's only noticeable to me because I'm looking for it.

What's the logic behind?

yes to an int. int 0 is False-ish in python. could have just control.playlist.size() > 0. if the function fails to create a playlist size will be 0, which will evaluate to False in that test. so it will not try and play an empty playlist and the else will just play the item. personal pref, maybe its better just to not play anything.

Cool, I was just trying to understand if you were looking for a zero or none there or what. Since it's going to return an int of zero for no playlist i'll stick with > 0 on my side. Thanks for explaining.

jurialmunkey commented 1 month ago

ultimately, the skin dev explain why playing from that container does not behave the same way as when you click into the season and play. I think it should be fixed to act the same or what kind of container is that even? I wonder what jurial would say if anything at all.

Skins dont really have anything to do with playback.

The key difference for the combined view versus a more "standard" view is that for the combined view, only the buttons underneath (i.e. the seasons) are the actual view. The container on top (i.e. the episodes) is an entirely independent container with content filled using ListItem.FolderPath of the currently focused item in the view.

Since the container on top is not the "view", then the onclick action needs to be defined manually. There manual onclick uses the Container(ID).ListItem.IsFolder boolean to determine which action to do:

There's nothing much special about it. PlayMedia is the only way a skin can initiate playback manually via the skin engine. It's expected that plugins will work with this builtin -- and they should if using setResolvedURL -- but of course it gets murky once you start layering on workarounds to implement playlist creation plugin side instead of Kodi side.

When clicking an item in a container, there's three different levels a skin might interact with Kodi

  1. In Media "Nav" window "View" containers, Kodi will handle the click entirely automatically. The exact action it does depends on the properties of the ListItem (e.g. IsFolder IsPlayable etc.) and which Nav window is open (videos/music/pictures/programs/games/tvguide etc.).
  2. In Widgets on Home (or other custom windows), Kodi will handle the click mostly automatically based on the ListItem properties. However, since we are not in a "Nav" window, the skin must supply a "target" value for the container to give Kodi a hint about which Nav window the content belongs in.
  3. In Widgets in a Media "Nav" window (i.e. a "combined" view), the skin must define the onclick manually. That's because if the skin just defines a target value, Kodi will try to call ActivateWindow(target,path,return) for a folder. Problem is that since the videos Nav window is already open, that command will do nothing (we want to do Container.Update instead to update the current view, so it needs to be manually defined - which then means the Play action must also be manually defined).

But there really isn't much to it beyond that.

xbmc.getInfoLabel('ListItem.FileNameAndPath')

You're probably going to end up having a bad time using this (though granted you've noted it as a hacky workaround, so perhaps you're already aware of the below).

ListItem infolabels won't necessarily be the item that was clicked (e.g. if focus gets sent elsewhere before the plugin callback happens either by the skin or a mouse bump etc. then ListItem will be something else, or if the skin called the plugin using PlayMedia instead).

Additionally, what exactly is the "ListItem" currently depends on the Window.

In a Media Nav View it will be the focused item in the main VIEW regardless of which container has current focus (e.g. in a combined view, ListItem will always be the "Seasons" portion even if an episode has focused / was clicked). The info dialog also works this way. Referencing another container other than main view requires Container(ID). prefix to tell Kodi which one.

In a Home window (or other custom window), I think it will be empty (because there's no main view) - or possibly it will be aliased automatically to Container.ListItem.FileNameAndPath, which is currently focused container -- I'm not 100% certain.

Either way, point is that you shouldn't be using this value assuming that it in anyway relates to the item that was clicked as there are plenty of ways it might not be.

kodifitzwell commented 1 month ago

It's expected that plugins will work with this builtin -- and they should if using setResolvedURL -- but of course it gets murky once you start layering on workarounds to implement playlist creation plugin side instead of Kodi side.

thanks @jurialmunkey for the explanation. the addon uses isPlayable/setResolvedURL and workaround wasn't the first choice, maybe if issues start to occur we can develop different solution. I understand in a layman kind of way why Kodi cannot create the playlist from there.

can you give any insight on the root cause that led to seeking an alt? when both the Kodi/Settings/Media/Default select action is "Show Information" and Kodi/Settings/Player/Videos/Play next video automatically "Episodes" (or Movies) is enabled, selecting "Play" from the info window, the attempt calls <plugin id>/VIDEO_TS/VIDEO_TS.IFO? this started with Omega, is this the direction going forward? this seems independent of skin.

in Nexus it worked "correctly" or at least attempted the plugin defined listitem url, <plugin id>/?action=....

umbrellaplug commented 1 month ago

Thanks for the response, I know it was a lot to read through. I 100% agree the workaround is not idea. It was an attempt at addressing a change in kodi 21. I think removing the Kodi/Settings/Player/Videos/Play next video automatically "Episodes" has resolved a lot of the issues and allows for the use of the show information default action to be set again.

There's nothing much special about it. PlayMedia is the only way a skin can initiate playback manually via the skin engine. It's expected that plugins will work with this builtin -- and they should if using setResolvedURL -- but of course it gets murky once you start layering on workarounds to implement playlist creation plugin side instead of Kodi side.

I can remove all playlist creation code from my addon and launch from a one of the mentioned views in your skin vs launching from within the addon and still have an item playing where the position for the playlist is -1. I'm trying to understand your playmedia vs resolving statement. The addon would be using setResolvedURL for media passed in from widget or the addon directly, so is playmedia being used causing this behavior or is it based around the settings in kodi?

can you give any insight on the root cause that led to seeking an alt? when both the Kodi/Settings/Media/Default select action is "Show Information" and Kodi/Settings/Player/Videos/Play next video automatically "Episodes" (or Movies) is enabled, selecting "Play" from the info window, the attempt calls /VIDEO_TS/VIDEO_TS.IFO? this started with Omega, is this the direction going forward? this seems independent of skin.

I'd be interested in the response on this as well.

in Nexus it worked "correctly" or at least attempted the plugin defined listitem url, /?action=....

This is correct, I didn't see any of these issues until Kodi 21.

Thanks to everyone for this bit of collaboration. I think we all just want to make it a better experience to use kodi.

stathis95194 commented 1 month ago

@umbrellaplug not sure if i should open a separate issue about this, but i thought since it has to do with Playnext, i should keep it here. My issue is that if/when i have the Popup Trigger set to Percentage or Seconds everything works properly. If i select Subtitle End then no matter which one is the backup trigger nothing works. No popup at all.

Using Kodi 21 and attaching the umbrella.log after playing 1 episode from the playlist with subtitles set at 20 and seconds remaining at 60

umbrella.log

adamosborne83 commented 1 month ago

@umbrellaplug not sure if i should open a separate issue about this, but i thought since it has to do with Playnext, i should keep it here. My issue is that if/when i have the Popup Trigger set to Percentage or Seconds everything works properly. If i select Subtitle End then no matter which one is the backup trigger nothing works. No popup at all.

Using Kodi 21 and attaching the umbrella.log after playing 1 episode from the playlist with subtitles set at 20 and seconds remaining at 60

umbrella.log

Are you on the newest test version? I have just tried myself with young Sheldon, and the 6 episode I tested all had the pop up, sometimes went a few seconds into the credits.

I only ask, as I was having the same issue but UD put a fix into place.

stathis95194 commented 1 month ago

@umbrellaplug not sure if i should open a separate issue about this, but i thought since it has to do with Playnext, i should keep it here. My issue is that if/when i have the Popup Trigger set to Percentage or Seconds everything works properly. If i select Subtitle End then no matter which one is the backup trigger nothing works. No popup at all. Using Kodi 21 and attaching the umbrella.log after playing 1 episode from the playlist with subtitles set at 20 and seconds remaining at 60 umbrella.log

Are you on the newest test version? I have just tried myself with young Sheldon, and the 6 episode I tested all had the pop up, sometimes went a few seconds into the credits.

I only ask, as I was having the same issue but UD put a fix into place.

I actually am not. Still on 6.6.64 @adamosborne83 Where can i find the test version? I would be glad to give it a try as i like using Playnext a lot

jurialmunkey commented 1 month ago

an you give any insight on the root cause that led to seeking an alt? when both the Kodi/Settings/Media/Default select action is "Show Information" and Kodi/Settings/Player/Videos/Play next video automatically "Episodes" (or Movies) is enabled, selecting "Play" from the info window, the attempt calls /VIDEO_TS/VIDEO_TS.IFO? this started with Omega, is this the direction going forward?

@kodifitzwell - No idea. My C++ is very rudimentary and I don't write core code - I'm just a skinner who also makes a few addons to help with skinning.

IMHO I can't see how this is anything other than a bug which should have been reported so that it could be fixed. If you don't report bugs, they don't get fixed. It's quite simple.

It's a little bizzare to me that you'd go to all this effort to write workarounds for a bug instead of just taking some time to create a bug report. It could've been fixed by now in core and benefiting everyone instead of spending all this time and effort.

If you're worried about your logs, make a clean portable install using a basic test plugin to demonstrate the issue. Here's one I which took me less time to make than it did for me to read all the comments in this thread https://github.com/jurialmunkey/plugin.video.teststream

I can remove all playlist creation code from my addon and launch from a one of the mentioned views in your skin vs launching from within the addon and still have an item playing where the position for the playlist is -1. I'm trying to understand your playmedia vs resolving statement. The addon would be using setResolvedURL for media passed in from widget or the addon directly, so is playmedia being used causing this behavior or is it based around the settings in kodi?

@umbrellaplug

My point is that the skin is not doing anything special and it shouldn't have anything to do with the skin used. You could test this by using PlayMedia(plugin://whatever your callback url is) in any skin and you should get the same behaviour.

If a plugin is using setResolvedURL (and not doing anything too weird), then PlayMedia should work fine.

If it is not, then it indicates an issue with either the plugin or Kodi. Again, this can be addressed by using a simple test plugin like the one above and seeing if it still causes the same issue.

If it causes the same issue with a simple test plugin then it's a bug in Kodi that should be reported. If it doesn't cause the same issue, then the problem lies somewhere in your code.

The other thing you can test is to see if adding the playlist_type_hint=1 param to the PlayMedia command does anything (it gives Kodi a hint whether to use music (0) or videos (1) playlist). Possibly playlist position is -1 because Kodi is incorrectly using music playlist

The hint is not meant to matter for single items, only for playlists (and even that is meant to be a temporary fix until code can be implemented to handle it by mimetype instead) -- but a possible related cause I guess (kinda makes sense if Kodi is defaulting to using music playlist that videos will be -1 in that case). I can always add an extra check in the skin for the onclick based on content type so that the hint can be added if it is needed even for a single item

kodifitzwell commented 1 month ago

No idea. My C++ is very rudimentary and I don't write core code...Here's one I which took me less time to make than it did for me to read all the comments in this thread

well clearly what you feel is rudimentary C++ is more than made up for in not only python but the inner workings of kodi.

IMHO I can't see how this is anything other than a bug which should have been reported so that it could be fixed.

well IMO (and maybe others too), I couldn't imagine this is anything other than that too. but given my lack of in-depth kodi knowledge and the nature of my repository, didn't feel I had anyone to ask that would actually give an answer. thankful that @adamosborne83 felt it was okay to tag you. but now that its more than just MO, and thanks to the code you so quickly put together, someone without a banned repo or addon can report it to be fixed and benefited by everyone.

It's a little bizzare to me that you'd go to all this effort to write workarounds for a bug instead of just taking some time to create a bug report.

"To a man with a hammer, everything looks like a nail."

If you're worried about your logs,

well that's exactly why...

thanks @jurialmunkey, was looking for a little insight got so much more...

jurialmunkey commented 1 month ago

thanks to the code you so quickly put together, someone without a banned repo or addon can report it to be fixed and benefited by everyone.

There are plenty of legitimate plugins available on the official repo that you could use to test with and attempt to recreate the issue.

If you're worried about your logs, well that's exactly why...

The reason clean logs are asked for is to rule out external causes so that someone isn't wasting their time trying to fix what often ends up as an external issue.

The main point of making a test plugin instead of using an existing one is to simplify the code down to the bare essentials to rule out other causes because then someone will be more willing to spend their free time fixing it -- but its not necessary to report a bug, it's just a kindness to make it a little easier for the person on the other end.

You have to remember that Kodi is nothing more than a loosely organised group of people contributing code. People work on the features/fixes/improvements that they are interested in. It's not some top-down centrally organised project.

Obviously some internal discussion occurs, especially if someone is considering starting on a larger project/idea/improvement; and obviously there is some coordination in terms of managing releases; but for the most part what you see publicly in PRs and issues and on the forums is the extent of any "planning". It's just a bunch of people working on their pet projects and the sum of all those parts comes together to make Kodi.

kodifitzwell commented 1 month ago

I understand all that. and as someone with a lot of knowledge like you, there are probs a lot of "simple" questions asked to you and that gets tiresome.

just understand that sometimes newer people just need some validation about something before they figure out how to proceed.

and to me it was pretty obvious, but just to be clear, I was referring to myself as the person with the hammer.

umbrellaplug commented 1 month ago

I appreciate you spending the time to write something simple to help illustrate the problem. I'm glad it was easier for you to do than read through all this. 😄

umbrellaplug commented 1 month ago

I've been looking over all the comments here and trying to get my code in a position where I'm not "overstepping" what the plugin should be doing. It appears, this is two separate issues in my opinion:

  1. Possible kodi bug issue of the combination of the settings playing automatically using episodes and show information set as a default behavior.
  2. Items played from the skin using a playmedia seem to be losing the play position in the playlist.

For 1 I'm going to remove my workaround and work with our users or use another alias to open a bug report with kodi. This was already opened, but this repo was tagged in the issue so I imagine that will get shut down and ignored pretty rapidly. From working with kodifitzwell, I have figured out the main culprit causing this issue is the "Play next video automatically" setting being set to episodes and I have removed that.

For 2, I'm going to remove any playlist building code I have and attempt to change my local install to change the skin to call the playmedia with playlist_type_hint=1 and see if that makes any difference for kodi actually having the correct position of the item playing in the playlist or if it remains -1. If that does not make a difference, I guess a second bug/issue would need to be opened with the kodi team for that as well.

I'll see what the results I get are with these changes.

umbrellaplug commented 1 month ago

@jurialmunkey I'm testing the playlist_type_hint=1 and I'm not seeing a difference for playlist generation unfortunately. It's still plays the item, but the kodi playlist shows nothing added and the getPosition returns -1. I want to make sure I'm checking all this correctly, can you confirm for me the correct file I should be making the change for playmedia in the skin for this view?

image

Thanks for your time, I know it can be frustrating answering questions for us but please bear with me. Your feedback is really appreciated.

Edited I came back to respond here because I wanted to let you know what I had found so far. I found an addon someone had made in the kodi forums to assist with the change to getVideoInfoTag(). This allows for a context menu option to output VideoInfoTagInfo. I found that the playlist (single item) is being set before any resolve code is run. Something within kodi is automatically creating a single-item playlist before anything is started. I believe this behavior has to do with the content-type.

Here's a link for the getVideoInfoTag() addon I found. When I run this addon on the view referenced above it's showing the ListItem.DBTYPE as 'season' and Container.Content as 'seasons'. This is only occurring on combined views. I think this is the root cause of the issue. I'm not sure if you could do anything about that or not, but this appears to be the issue. I'm attaching a screenshot of what the videoinfotag showed for the combined landscape vs the landscape.

image Combined Landscape

image Landscape

jurialmunkey commented 1 month ago

It should be this line https://github.com/jurialmunkey/skin.arctic.fuse/blob/7cfdd0b1ea7137434983dba066d1d22be033bcbd/1080i/Includes_Views_Combined.xml#L75

Eg you want it to be:

PlayMedia($ESCINFO[Container($PARAM[id]3).ListItem.FileNameAndPath], playlist_type_hint=1)

Don't forget to reloadskin after making modifications.

It's a bit of a long shot anyway as like I said it's only meant to matter when playing a playlist (eg a strm or xsp m3u etc file) rather than a single item.

umbrellaplug commented 1 month ago

It should be this line https://github.com/jurialmunkey/skin.arctic.fuse/blob/7cfdd0b1ea7137434983dba066d1d22be033bcbd/1080i/Includes_Views_Combined.xml#L75

Eg you want it to be:

PlayMedia($ESCINFO[Container($PARAM[id]3).ListItem.FileNameAndPath], playlist_type_hint=1)

Don't forget to reloadskin after making modifications.

It's a bit of a long shot anyway as like I said it's only meant to matter when playing a playlist (eg a strm or xsp m3u etc file) rather than a single item.

I think you were replying at the same time I was updating my comment. I think the content type of the view is the issue.

jurialmunkey commented 1 month ago

Yeah I was.

Content type and listitem will relate to the view container, which is seasons, so that is the correct value - this relates back to what I was saying before about not using get infolabel etc as it might not be the clicked item. That's also why I the combined view I have to explicitly reference the container ID for the widget portion.

If something in your code is trying to pull values from the listitem infolabel or container content the you will get the wrong values.

If you are saying kodi does incorrect behaviour for playmedia here, you'd need to recreate with a simplified plugin to rule out plugin code. Sounds very unusual though

umbrellaplug commented 1 month ago

Yeah I was.

Content type and listitem will relate to the view container, which is seasons, so that is the correct value - this relates back to what I was saying before about not using get infolabel etc as it might not be the clicked item. That's also why I the combined view I have to explicitly reference the container ID for the widget portion.

If something in your code is trying to pull values from the listitem infolabel or container content the you will get the wrong values.

Nothing in the addon code is doing this that I can see. I have removed all code that does anything with playlist. I can actually see the playlist created automatically as soon as the item is clicked from an episode view. This is a behavior that is happening automatically from kodi based on the content view type. I was able to see it by watching the playlist. When an item is played from the combined view because it's content type is seasons instead of episodes that automatic action of adding the item clicked as the item in the playlist is not happening. I'm happy to make a recording if it's easier for you to see a video with the playlist pulled up so you can see the behavior. I put some breakpoints in the addon and I can see the playlist is created from an episode view before it ever even gets anywhere near the resolve code.

umbrellaplug commented 4 weeks ago

If you are saying kodi does incorrect behaviour for playmedia here, you'd need to recreate with a simplified plugin to rule out plugin code. Sounds very unusual though

I will look for a plugin that deals with seasons now that would be acceptable to use to test and submit logs to kodi. If there's some type of property I need to be setting or something I'd love to know.

jurialmunkey commented 4 weeks ago

Sorry maybe I'm misunderstanding here. Playmedia will not create a playlist on its own. The command plays a single file (unless explicitly passed a playlist). It doesn't matter what the kodi setting is. This is the downside of a combined view. If a user wants to play all episodes then they have to either do play all on the season or open up the season folder to update the view to show episodes from the season (or use more "classic" seasons view like posters to force the user to open the folder into episodes view)

If the issue is just that kodi is not making a playlist natively in the combined view, then that's correct behaviour. I thought the issue was that playlist position was -1 and so no playlist existed to add items manually afterwards - as to why that occurs (vs a playlist with just one item and position being 0) I'm not certain as I thought the omega changes had made everything via setresolvedurl start in a playlist (but I could be mistaken).

umbrellaplug commented 4 weeks ago

Sorry maybe I'm misunderstanding here. Playmedia will not create a playlist on its own. The command plays a single file (unless explicitly passed a playlist). It doesn't matter what the kodi setting is. This is the downside of a combined view. If a user wants to play all episodes then they have to either do play all on the season or open up the season folder to update the view to show episodes from the season (or use more "classic" seasons view like posters to force the user to open the folder into episodes view)

If the issue is just that kodi is not making a playlist natively in the combined view, then that's correct behaviour. I thought the issue was that playlist position was -1 and so no playlist existed to add items manually afterwards - as to why that occurs (vs a playlist with just one item and position being 0) I'm not certain as I thought the omega changes had made everything via setresolvedurl start in a playlist (but I could be mistaken).

Yeah, this is the issue. The item is being played but even when the item is playing the playlist position is still -1 when we resolve an item from the season content view like that. If I add it to a playlist then use player.play() on that item it correctly shows the item as a single item in a playlist we can then add to. I'm including a video with the playlist in the screen so you can see the behavior.

https://vimeo.com/998017557?share=copy

epidoma commented 3 weeks ago

seems to be fixed in kodi https://github.com/xbmc/xbmc/pull/25610

adamosborne83 commented 3 weeks ago

It does indeed.

umbrellaplug commented 3 weeks ago

Thanks everyone for your contributions to this topic. Closing the issue now that kodi was pushed an update to resolve the bug.