Kodi Community Forum

Browsing movies by actor is a bit silly if you have a big library of movies. My library has more than 44.000 actors.
What I really want is to browse the actor list and see only actors that make a difference to those movies in my library.

Maybe sorting on the amount of movies they played in instead of alphabetical order.

What do you think? Or don't you 'need' such a feature at all?

I think it would be nice if this was solved in a more general way. For movies for example, the only library node that supports any of the "special" view modes (info views, fanart, etc.) is title - the rest of them (actors, genres, year, etc.) are limited to list and thumbnail and have no sort options. I was thinking that the following would solve the issue and offer some nice additional functionality:

1. Add a label2 value for all of the additional nodes which gives an item count (i.e. number of items that were matches for each returned item).
2. Add the ability to sort on this count.

That would make the solution applicable across all "special" nodes and could be applied to tv shows as well. Then you could just sort by the count and the items would be listed by order of occurrence.

This would likely be a pretty big change though...

You can use ember media manager to weed out 'excess' actors automatically, or you can just manually add only the few that you actors that you consider 'starring' from each movie.

galvanash Wrote:I think it would be nice if this was solved in a more general way. For movies for example, the only library node that supports any of the "special" view modes (info views, fanart, etc.) is title - the rest of them (actors, genres, year, etc.) are limited to list and thumbnail and have no sort options. I was thinking that the following would solve the issue and offer some nice additional functionality:

1. Add a label2 value for all of the additional nodes which gives an item count (i.e. number of items that were matches for each returned item).
2. Add the ability to sort on this count.

That would make the solution applicable across all "special" nodes and could be applied to tv shows as well. Then you could just sort by the count and the items would be listed by order of occurrence.

This would likely be a pretty big change though...

i like it. patch welcome. it won't be much work.

I have the beginnings of a patch done, but I'm very new to this so some advice would be appreciated (I realize I don't really know my way around the code yet, I'm just trying to learn at this point)...

1. My patch is currently hijacking m_iprogramCount to store the counts (it works, but I only did that to get the ball rolling so to speak). What is the right way to store this? Should I actually make a new field on CFileItem? Or is there someway to just set label2 directly and sort on it without involving a specific field (since this is essentially virtual data).

2. Is there a "standard" way to apply a custom format for label2 without adding a new label mask? I would like label2 to be displayed as "x movies" or "x episodes" or whatever, but I wasn't sure if I should be adding additional label masks for something like this...

3. All of the CVideoDatabase::GetWhateverNav() methods have a block in them that conditionally does something different for the following condition:


At this point Im afraid I don't really understand this code... Would my modification need to be applied for those blocks as well? If so I would have to change the sql for them because they currently do not return counts (the normal code path does for everything I have seen so far, so adding this is really easy for those (like a 2 line addition).

4. For CGUIViewState::AddSortMethod(SORT_METHOD sortMethod, int buttonLabel, LABEL_MASKS labelmasks)... Is there a source file that defines the buttonLabel constants? I can't figure out how to add a new label (assuming I would need to).

Thanks in advance.

1&2) yes. use SetLabel2() on the item, then SetLabelPreformatted(). hijacking the programcount is ok, we do such things all over. remember to comment it!

as for the sql, yes you have to modify both blocks. a profile can have locks on its sources. we have to filter out the files from the locked sources in the sql. since this is slower, we have two paths; with and without filtering.

4) that's an entry in language/English/strings.xml

Thanks. I should have it finished up tonight or tomorrow... What should I do with the patch when completely? Should I send to you or just submit it as a patch on bug trac?

patch on trac, not bug on trac

Ok... Before I do any more work on this...

I have found that some fairly major modifications were in order to re-factor the code when trying to incorporate counts. I think I have managed to get my head around the way everything was working before, so I have taken some liberties that I hope are appreciated. But before I go any further I was hoping someone might take a look at this and let me know if I'm going down the right path - or not...

This is a copy of my CVideoDatabase::GetNavCommon(). Im waiting on further guidance before attacking GetPeopleNav, GetYearsNav, etc.

http://pastebin.com/ysXCkqAp

The gist of the approach I took:
1. Since getting counts requires doing group bys anyway, there seemed to be little to be gained doing an entirely separate code path for when source lock checks were needed, so the branch for lock checking is happening much further into the routine and is much more granular. The same queries are used regardless of whether or not the checks are needed.
2. The old code essentially returned 1 row per id (id as in the grouping id, genre, studio, etc.) for the normal path and 1 row per item for the lock check path (in order to check the source paths). The new code does 1 row per source path by using a group by on the id and the path.idpath, and handles the duplicate ids in code using the map. (hope that makes sense... The code reads fairly straightforward to me, but I wrote it )
3. I added playcount support for tvshows. It seemed appropriate to do so since the way the code is factored now it will work. It is actually a count of episodes, not shows - but that seems more useful anyway. When you go a level deeper the counts "split" by tv show, so it is obvious what is happening from a user perspective.

I find the new code easier to understand, but there "may" be a performance hit when there is no need for lock checking due to the query now being a bit more complex. It seemed like a good tradeoff to me though.

Also, I did notice something else while working on this. I thought I caused it, but after digging deeper it was already there. Set handling doesn't work quite right (or maybe I should say it doesn't work the way I would have expected).

For example: If you have 3 movies in a set, and one is genre "action" but the other 2 are not. When you go to genres the one movie that matches will be counted. But when you click into the next level of the hierarchy, the entire set is returned. Also, if you have 1 movie in a set (which I admit makes little sense, but I did it just to test), for some reason it doesn't show up in the listing at all... I have not figured out what is causing this - but it isn't my code, it appears it already had this issue.

The more I play with it the more it seems to make sense to just disable displaying sets as a folder underneath the "special" nodes - it never worked right anyway, but with the counts it is painfully obvious now that it doesn't work right...

I did some performance analysis... I don't see much if any slowdown with the new queries. I started doing peopleNav and yearNav. The only node that seems to have a performance issue is the actors node, but it has always been slow it seems. The culprit appears to be in getActorsNav (which wraps getPeopleNav) - it is doing a fileExists on every returned item to see if the thumbnail exists to conditionally load them. That part takes longer than the query and item retrieval combined on most runs.

Average of 3 runs with new code with master lock enabled:

CVideoDatabase::GetPeopleNav - query took 5 ms
CVideoDatabase::GetPeopleNav item retrieval took 16 ms
CVideoDatabase::GetActorsNav - setting actor thumbnails took 20 ms

the original code took 3ms and 17ms respectively (the GetActorsNav part should be the same as I didn't touch that). That was with only about 20 movies and 150 or so actors with a release build with debugging turned on, but it doesn't feel any slower than it was before. I'm going to test on my main install (500+ movies) and report the differences.

btw, with the master lock disabled the item retrieval only took about 5ms either way, it seems my changes have no impact except for the query part (which is good I think).


EDIT:

I timed it on my main install. 550 movies with 20559 actors with masterlock turned off.

new code:

CVideoDatabase::GetPeopleNav - query took 556 ms
CVideoDatabase::GetPeopleNav item retrieval took 458 ms

org code:

CVideoDatabase::GetPeopleNav - query took 241 ms
CVideoDatabase::GetPeopleNav item retrieval took 332 ms

so there is a difference once the resultset gets large enough, but keep in mind the entire operation took over 3 seconds, with the other 2 seconds being the loading of the thumbs, so this represents only about a 16% slowdown overall. With the master lock turned on the speed is nearly identical for item retrieval (the query is still a wee bit slower).

Of course this is all on windows... The thumb loading might not be so slow on other platforms.

I still have sets nav to do and a bit of cleanup and whatnot, but you can use this patch if you want to review.

Notes:
1. I added a strings.xml entry at 167 for the sort label - that isn't included as I wasn't sure whether or not that would be the right thing to do. I labeled it "count".

http://pastebin.com/EJkHJEHT

I am up against some fairly big obstacles that make getting this to work pretty challenging at this point...

1. Sets don't behave properly. If I put counts on a movie node (i.e. movie genres), the existence of sets breaks behavior badly. If there is one matching movie for a genre, but it is in a set, when you click into the folder no items are displayed (because for some reason a set with only one movie is being filtered out somewhere else in the code). If there are say 2 movies in the set that match - the entire set is displayed. The set support just isn't factored in well enough - and I'm afraid that adding counts will just make it that much more obvious that it is broken... The best short-term fix that I can think of is to simply not display sets except on the title node - but I don't know if that would fly with the dev team...

2. For some reason under movies the "special" nodes allow setting watched/unwatched status, but under tvshows they do not. CGUIWindowVideoNav::GetContextButtons seems to block setting up the buttons for setting watched/unwatched under tvshows - but Im not sure why and I don't want to break anything out of ignorance. It may be better to simply not support watched status on the special nodes - my patch includes it and it seems to work properly, but since the user can't edit it under tvhsows it seems pointless.

3. The modifications I did to the queries, particularly for the actors node, do slow things down a bit for those who do not use master locking. I don't know if it is worth doing 2 code paths here since the query is necessarily more complex than it was before due to have to get the counts...

Any advice/feedback/anything would be appreciated at this point. I'm hesitant to submit what I have as a patch since a) it isn't completely working due to the sets issues and b) I don't want to get yelled at for making heavy modifications for what might be considered a non-critical feature.

This is the current state of the patch

hi, sorry for the lack of feedback, fell under my radar for some reason.

1) yes, we remove single-movie sets. that means the query instead returns the movie - so that shouldn't cause an issue? in either case, if we need to refactor, so be it. we're not afraid of doing that.

2)
afaict we don't block shows unless the content is NOT tvshows. in either case, it shouldn't break - all we do is list the dir and call recursively on each item.

3) slowdown does not sound good. i think we may want some hackery here. separate path and a refetch if sort by count is selected. alternatively we could perhaps use the background loader to split in fetch and count, i.e., do the counting in the background.

in general; it sounds like you are addressing several issues. open a trac ticket and split the patch issue by issue. it's much easier for all parts to work over there - for instance things don't go as easily under my/others radar

spiff Wrote:hi, sorry for the lack of feedback, fell under my radar for some reason.

No problem at all.

spiff Wrote:1) yes, we remove single-movie sets. that means the query instead returns the movie - so that shouldn't cause an issue? in either case, if we need to refactor, so be it. we're not afraid of doing that.

Ok, I see. I haven't nailed it quite down yet, but it appears if the single-movie set is the only item in the list then the movie isn't being returned at all - nothing is returned. I need step through it some more to be certain though.

spiff Wrote:2)afaict we don't block shows unless the content is NOT tvshows. in either case, it shouldn't break - all we do is list the dir and call recursively on each item.

Well this one was just from observation. If you navigate to say movies->genres->some genre in the library and pull up a context menu, set watched/unwatched is available as an option. However if you do the same under tvshows it isn't... regardless of whether my patch is used or not. The code works too (under movies) - if you set watched for say "action" movies it does mark all matching movies as watched - but the node doesn't reflect that the watched status was changed - I was just adding that while I was in there since it seemed to make sense.

spiff Wrote:3) slowdown does not sound good. i think we may want some hackery here. separate path and a refetch if sort by count is selected. alternatively we could perhaps use the background loader to split in fetch and count, i.e., do the counting in the background.

I see. yes, I think I could do that (refetch only for the sort by count case). Ill take a wack at it.

spiff Wrote:in general; it sounds like you are addressing several issues. open a trac ticket and split the patch issue by issue. it's much easier for all parts to work over there - for instance things don't go as easily under my/others radar

Will do. Ill do one patch for the counts and a separate one for the watched/unwatched status.

As the OP I've been following this thread with interest.
I would like to follow the developments more in detail so I looked here: http://trac.xbmc.org/timeline and searched for galvanash, but couldn't find any entries.

I'm sure I completely misunderstand the whole development process, but if you feel like explaining I would be much obliged, unless it will take valuable time from getting this to work, of course ;-)