Add support to only retrieve the current chunk of media items

[bowlingbeeg]

• Currently the code will retrieve all the media information up front when viewing a library. This change will only request the data for a specific chunk if the user has navigated to a media item that hasn't already been fetched.

• I'm also removing the old chunked view stuff. It has been disabled for some time now and is broken. It also only supports list view. The new code to only request the media items for the ones being displayed should alleviate the need for the old chunk mode

GHI (If applicable): #

Description:

Checklist:

• [X] I have based this PR against the develop branch

[bowlingbeeg]

Take a look at this change to how we request media items from the plex server and let me know what you think. This should only request the media items that the user actually navigates to. I think this was what you were hoping the chunked mode stuff would do but when I started looking at it again I realized that it only supported a list view. So I tried to come up with something that would work for all view types. I don't have a huge library(max of about 800 items in any of my libraries) but it seems to work ok. I think for my size libraries fetching the items up front isn't a big deal and leads to a little better scrolling but I know people with big libraries might benefit from this. I made this the new default but I did include a setting so that someone can go back to the old way if this isn't working for them or they find the older way better.

[pannal]

Can you guide me through what you did and what the concept is? There's a lot of deletion so it's a little hard to navigate.

You're basing the chunking on MOVE_SET? How does it handle navigation via the jumplist?

Edit: I'm a little confused by the chunkStart and lastChunkStart naming.

[bowlingbeeg]

The concept is that we ask up front for the jump list which gives us the total number of media items to display(it does this today so that's not changing) but then instead of sending out request for all of the media we only request the first chunk(240). Then as the user navigates through the list that will trigger an action in the MOVE_SET. During that action we get the selected item and if it's not one we've already fetched we go fetch the chunk that the selected item is in. In order to keep track of which chunks of the media we've already requested I keep the starting pos of that chunk in a set. So the first request will put 0 in the chunk list. Then when the user navigates to an item in the next chunk(aka positions 240-479) then we'll request that chunk and put 240 in the chunk list. The lastChunkStart is kept so that I know when we've hit the end of the list. Jumplist navigation is handled in the keyClicked function after the first movie in that key is selected.

[pannal]

Thank you for the explanation - this solution seems extremely elegant.

My only gripe with it is the naming of chunkStart and how it's used, e.g. here: https://github.com/pannal/plex-for-kodi/pull/84/files#diff-a74435acdce33dd6646a0f25892da1eeb894ccef41062e0b2a46e77f43f3ed86R1381

        chunkStart = (start // CHUNK_SIZE) * CHUNK_SIZE
        if chunkStart > self.lastChunkStart:
            return

Is pretty confusing without any comments explaining what we're doing here.

Could you either add comments or rename the variables to in more graspable way?

Other than that, I think this can be merged.

[bowlingbeeg]

Sure, I can take a look at that this evening. If you have any suggestions on naming let me know.

[pannal]

Not really, as I still don't fully grasp that part. Why would we return when the requested chunkStart is bigger than our old one? It's very counter intuitive, or I'm missing something.

[bowlingbeeg]

It's not last as in the last one we used but the biggest chunkStart we'll ever see. So if we calculate something bigger then we're out of control. For example say there are 300 items then lastChunkStart would be 240(since that chunk would get you items 240-300). And in our chunkList we would only ever have chunkStarts of 0 and 240(aka we'd only need two requests from the Plex server). If we somehow calculated a chunk start of 480 that would be bad so I'm just returning before we make a bogus request.

[pannal]

Ah! OK, then renaming lastChunkStart to endChunk, or lastAvailableChunk or something would make it much clearer.

Alternatively you can just add a comment to that area, which would be fine by me as well.

[pannal]

BTW: This is a great start. What I was trying to do years ago was to actually only display the current chunk, thus trying to not run into memory/performance issues on smaller systems. That's quite a different beast, as it needs dynamic updating and handling of the actual list, cleaning up "invisible" items and handling scrolling up into already cleared areas. That's be an awesome V2.

Edit: It was basically full lazy loading, as with the other horizontal pagination lists.

[bowlingbeeg]

Ah ok, I didn't realize that was the motivation behind the chunk stuff. I guess it makes sense but do systems still have problems if the library is really large or are we beyond those types of issues? Reason I ask is because that sounds like a lot of work and I don't want to do it if no one is really going to benefit.

[bowlingbeeg]

ok, I updated the change with better variable names and some comments. Let me know if these changes work, otherwise I can tweak the names/comments more if they still don't make sense.

[pannal]

Reason I ask is because that sounds like a lot of work and I don't want to do it if no one is really going to benefit.

It is a lot of work, true. Maybe we can find a middle ground somewhere, but first we need to find out if it's still necessary.

Let me know if these changes work, otherwise I can tweak the names/comments more if they still don't make sense.

Thanks! I'd rename endChunkPosition to finalChunkPosition, I think that's even clearer.

BTW, why use the chunkSize of 240? There are smaller common multiples available, such as 60, 120, 180. Is there a reason we want big chunks? (Maybe configurable?)

[bowlingbeeg]

Ok, I'll rename endChunkPosition.

I picked 240 because it was close to the 200 without doing less than we use today for the Plex server calls. I'm not sure if something smaller would be better or worse. It would generate more Plex server calls then we do today if you scrolled through the entire library. It certainly could be a setting if we find certain devices have trouble.

[bowlingbeeg]

Pushed the change to rename endChunkPosition to finalChunkPosition

[pannal]

Found an "issue" with this:

When opening a big-ish library and navigating to a jumplist item that results in a chunk > what we've already retrieved + X, the previous items are "visually there", but not loaded yet (using a chunk size of 120 in this case, which makes this issue more apparrent - it will happen with bigger chunk sizes as well, though): Initial load:

/library/sections/1/all?includeCollections=1&sort=titleSort%3Aasc&X-Plex-Container-Size=120&X-Plex-Container-Start=0&X-Plex-Token=****

After navigation via jumplist:

/library/sections/1/all?includeCollections=1&sort=titleSort%3Aasc&X-Plex-Container-Size=120&X-Plex-Container-Start=600&X-Plex-Token=****

After navigating one item above, the previous chunk loads:

/library/sections/1/all?includeCollections=1&sort=titleSort%3Aasc&X-Plex-Container-Size=120&X-Plex-Container-Start=480&X-Plex-Token=****
Moving task to front: <lib.windows.library.ChunkRequestTask object at 0x000001496F3D9A90>

The behaviour is correct and I don't find it that disturbing, but it'd be nice if we could find out, via aspect ratio and items-per-visible-view, or actual visibility of items (I believe that's possible), whether we need to load -6 to -12 extra items.

Edit: Another option might be to counter the "evenness" by always subtracting 6-12 items from the chunk start

[pannal]

I mean, this is safe as well:

            task = ChunkRequestTask().setup(self.section, startChunkPosition - 12, self.CHUNK_SIZE + 12, self._chunkCallback, filter_=self.getFilterOpts(),
                                            sort=self.getSortOpts(), unwatched=self.filterUnwatched, subDir=self.subDir)

It's not great but it works.

[pannal]

This is a sufficient fix IMHO: https://github.com/pannal/plex-for-kodi/commit/5925438106a35791ce6fd94a0220f0671de7ef36

[bowlingbeeg]

Yeah, I had initially coded this up so that it would select the chunk before and after as well as the current chunk. But I thought that seemed like overkill. Maybe if we selected a smaller chunk size that would be a more reasonable approach. I don't believe your fix works for the small poster size where there can be lots more than just 12 items not showing.

[pannal]

Yeah, I had initially coded this up so that it would select the chunk before and after as well as the current chunk. But I thought that seemed like overkill. Yeah that's a good decision.

I don't think there's any huge issue here, though. +/- 12 or 24 is reasonable and the setting is only an addon setting.

Edit: The threshold can also be dependent on the view mode.

[pannal]

Something like this: pushed again Edit: pushed https://github.com/pannal/plex-for-kodi/commit/c255b3b3dcd8c284085f1765a97dbed28f6c49d6

[bowlingbeeg]

I made a comment in the commit as well, but what happens when we fetch the chunk that contains those extra entries. Does the code just overwrite the data or could that lead to issues?

[pannal]

Should not lead to issues as far as I'm aware. The whole view relies on replacing items on the fly (which is wild already).

[pannal]

The real fix would be to check the position visually, of course.

And you might be right that this could lead to issues. It'd be probably wiser to instead of adding a threshold to the chunks, we could try and scroll the first chunk item to the top of the view to circumvent this. Let me try.

[bowlingbeeg]

we could try and scroll the first chunk item to the top of the view to circumvent this. Let me try.

I like that idea

[pannal]

I like that idea

Just tried a couple of approaches. I haven't been able to get to where I'd like to go. I think the latest approach is "fine". There's definitely more here, but I'm currently not able to get there.

[pannal]

I don't see any issues with the latest cheap approach. Do you?

[bowlingbeeg]

I haven't really had a chance to dive into it but if you're not seeing any errors then it's fine for now. Maybe if I get some time I'll look into the scrolling thing.

[pannal]

I haven't really had a chance to dive into it but if you're not seeing any errors then it's fine for now. Maybe if I get some time I'll look into the scrolling thing.

There is a way to determine the current position and scroll to it, somehow, I was able to, before. I was just half-way there and I've been there years ago. I don't remember whether it was worth the effort to completely rewrite everything or if my approach was wrong.

IIRC what you did was the right thing to do as a "semi quick fix". No guarantee, though.

I think the threshold/overcommit approach is good enough for now.

There is a way to determine the current position and scroll to it, somehow,

Edit: There is. It's what's the "default" right now. I'm talking about a scroll-to-chunk-Y-X-0, which is not what we're doing right now. Edit 2: Might just be the animation offset that does the selection "futher down", but I'm unable to confirm right now.

[bowlingbeeg]

So I looked at this a little bit tonight and as a quick and dirty hack I called the selectItem function twice. The first time I increment the position that I really want by 6 so that it would select something in the row below. Then I called selectItem with the position I really want. This way it was moving "backwards" in the panel. This makes the row of the actual item we want sitting at the top of the panel. But I quickly realized we have the same problem. Now there are cases where that top row is the final row in a chunk and everything after it is blank. But I still like the fact that after you select the letter from the jumplist your selected media items are at the top of the screen so maybe it's worth keeping. My other thought is that we use the CHUNK_OVERCOMMIT value and call the requestChunk() function with the position we want plus the overcommit. This would request the next chunk if it's needed and otherwise do nothing when we're still in the middle of a chunk. I'll upload a new pull request and you can see what you think. And I'll try and explain it a little better in the code.