Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

smartplaylist: use sorttitle in ORDER BY clause to sort movies by title #876

Merged
merged 2 commits into from May 2, 2012

Conversation

Montellese
Copy link
Member

The title says it all. This adjusts the sorting behaviour for movie-based smartplaylists to behave the same way sorting movies by title works in the library views. Instead of using the "title" property (c00) it uses the "sorttitle" property (c10). This has been requested here http://forum.xbmc.org/showthread.php?tid=127793 and IMO makes sense as SmartPlaylists should give (almost) the same experience as browsing the standard libraries.

@Memphiz
Copy link
Member

Memphiz commented Apr 12, 2012

Yep - the title says it all. But the varname "whereClause" doesn't. Maybe you find a better fit? Beside that i think a bool is not that nice to understand from the callerside. Can't you pass in the ID itself here and default to VIDEODB_ID_TITLE ?

@Montellese
Copy link
Member Author

The only more fitting one I could think of is "inWhereClause" but for me that doesn't make it any easier to understand. I don't understand what you mean by "pass in the ID itself"? I only need to know whether the actualy db-field name we retrieve is for use in a WHERE or in a ORDER so not sure what ID/whatever you are referring to.

@Memphiz
Copy link
Member

Memphiz commented Apr 12, 2012

Not at a rig here so i can't see how these are defined but:

if (field == FIELD_TITLE) result.Format("c%02d", whereClause ? VIDEODB_ID_TITLE : VIDEODB_ID_SORTTITLE);

looks like the bool only decides if the result is ordered by VIDEODB_ID_TITLE or by VIDEODB_ID_SORTTITLE

Though i might just read the code wrong and will better shut up ;)

@Memphiz
Copy link
Member

Memphiz commented Apr 12, 2012

Mhh rereading your answer i think i got it now. Yeah my first suggestion is not applying here of course ;). So that function just gives the C00 or C01 or whatever back. I see.

How about: "forSorting" - at least the prototype would read like CStdString CSmartPlaylistRule::GetDatabaseField(DATABASE_FIELD field, const CStdString& type, bool forSorting /* = false */) - which is some sort of human readable imho.

But thats only picking on nothing, because i didn't read the code change in the right way at the first time.

Beside that i would say that this change is sane of course :)

@jmarshallnz
Copy link
Contributor

sorttitle is not set by default in the database unless the user has .nfo files.

You'd need something like CASE WHEN c10='' THEN c01 ELSE c10 END to do this.

@Montellese
Copy link
Member Author

Ah damn I didn't know/consider that. All my movies have a sorttitle so it worked perfectly fine for me when testing ;-)

Question is whether both SQLite and MySQL support the same CASE syntax. According to http://www.sqlite.org/lang_expr.html and http://dev.mysql.com/doc/refman/5.0/en/control-flow-functions.html#operator_case the SQL syntax is the same so I'll give it a try and see if SQLite supports CASE in ORDER BY clauses (I'm pretty sure that MySQL does if SQLite does).

@Montellese
Copy link
Member Author

OK I added a commit which uses

CASE WHEN length(c10) > 0 THEN c10 ELSE c00 END

for correct sorting. The only difference I can see now in sorting smartplaylists for videos is that there's no ignore article functionality.

@jmarshallnz
Copy link
Contributor

Yeah, doing that with the db is probably not easy at all. There's a reason all our sorting stuff is outside the db.

All we need now is confirmation that it works in mysql.

@xbmcfanboy
Copy link
Contributor

I am a Win7 + smb/mysql user and am willing to test. I have some movies with nfo and some without nfo. Let me know if I can be of assistance.

@jmarshallnz
Copy link
Contributor

@xbmcfanboy: please do test this out, yes. Essentially we want to test the query above in af15d26.

@ghost ghost assigned Montellese Apr 16, 2012
@xbmcfanboy
Copy link
Contributor

Sorry took me so long to reply. Been busy rendering videos for a deadline. Let me know if I misunderstood the testing needed to help with this PR. I created a new smart playlist called "montellese" of type "Movies" and orderBy "Title".

Looks like the first query grabs any movie sets.

11 items returned
SELECT sets.idSet,sets.strSet,COUNT(1) AS c,count(files.playCount) FROM sets JOIN setlinkmovie ON sets.idSet=setlinkmovie.idSet JOIN movie ON setlinkmovie.idMovie=movie.idMovie JOIN files ON files.idFile=movie.idFile  where movie.idMovie in (select movieview.idMovie from movieview WHERE ('1')) GROUP BY sets.idSet HAVING c>1

Second query gets the movies

263 items returned
select * from movieview WHERE ('1') and movieview.idMovie NOT IN (SELECT idMovie FROM setlinkmovie s1 JOIN(SELECT idSet, COUNT(1) AS c FROM setlinkmovie GROUP BY idSet HAVING c>1) s2 ON s2.idSet=s1.idSet) ORDER BY CASE WHEN length(c10) > 0 THEN c10 ELSE c00 END

The playlist displays in xbmc with the 11 movie sets first, followed by the 263 movies not in movie sets.

Below examples are in CSV format. Notice that "Three Kings" doesn't have a value in column c10.

"idSet";"strSet";"c";"count(files.playCount)"
"1";"Alien Collection";"3";"0"
"2";"Are We There Yet";"2";"0"
"3";"Austin Powers";"2";"1"
"4";"Barbershop";"2";"0"
"6";"Children of Dune (2003)";"2";"0"
"7";"Darkman Trilogy";"3";"0"
"9";"The Street Fighter";"2";"0"
"10";"Scream";"2";"0"
"11";"Star Wars";"5";"0"
"13";"Tremors";"4";"0"
"14";"X-Men";"4";"0"
"idMovie";"c00";"c10"
"261";"The Tourist";"The Tourist"
"262";"The Transporter";"The Transporter"
"263";"The Usual Suspects";"The Usual Suspects"
"159";"The Warrior";"The Warrior"
"295";"Three Kings";
"266";"Titan A.E.";"Titan A.E."
"267";"Top Secret!";"Top Secret!"
"268";"Total Recall";"Total Recall"

The original query looked like this:

select * from movieview WHERE ('1') and movieview.idMovie NOT IN (SELECT idMovie FROM setlinkmovie s1 JOIN(SELECT idSet, COUNT(1) AS c FROM setlinkmovie GROUP BY idSet HAVING c>1) s2 ON s2.idSet=s1.idSet) ORDER BY c00

and here is snippet of the results:

"idMovie";"c00";"c10"
"261";"The Tourist";"The Tourist"
"262";"The Transporter";"The Transporter"
"263";"The Usual Suspects";"The Usual Suspects"
"159";"The Warrior";"The Warrior"
"295";"Three Kings";
"266";"Titan A.E.";"Titan A.E."
"267";"Top Secret!";"Top Secret!"
"268";"Total Recall";"Total Recall"

@Montellese
Copy link
Member Author

@xbmcfanboy Thanks a lot for this. I'm glad that the SQL query works for MySQL as well. Could you do one more test for me? Could you e.g. change the sorttitle value (c10) from "The Tourist" to "The Transporter 2" and from "The Transporter" to "The Transporter 1" and then open the playlist/run the query again? It should then list "The Tourist" after "The Transporter". With that we will know for sure that it works.

Thanks for your help.

@xbmcfanboy
Copy link
Contributor

Changed c10 per your suggestion:

UPDATE xbmc_video61.movie SET c10 = 'The Transporter 2' WHERE movie.idMovie =261;
UPDATE xbmc_video61.movie SET c10 = 'The Transporter 1' WHERE movie.idMovie =262;

Within xbmc, "The Transporter" appears before "The Tourist" in the "montellese" playlist.

Results of SQL (in CSV format):

"idMovie";"c00";"c10"
"262";"The Transporter";"The Transporter 1"
"261";"The Tourist";"The Transporter 2"
"263";"The Usual Suspects";"The Usual Suspects"
"159";"The Warrior";"The Warrior"
"295";"Three Kings";
"266";"Titan A.E.";"Titan A.E."
"267";"Top Secret!";"Top Secret!"
"268";"Total Recall";"Total Recall"

@Montellese
Copy link
Member Author

Thanks a lot xbmcfanboy looks like it works as it is meant to. So this can be merged in the May merge window.

@xbmcfanboy
Copy link
Contributor

Glad I could help. I did one more test to make sure sorting works in both directions.

UPDATE xbmc_video61.movie SET c10 = 'The Xyz 2' WHERE movie.idMovie =261;
UPDATE xbmc_video61.movie SET c10 = 'The Xyz 1' WHERE movie.idMovie =262;
"idMovie";"c00";"c10"
"263";"The Usual Suspects";"The Usual Suspects"
"159";"The Warrior";"The Warrior"
"262";"The Transporter";"The Xyz 1"
"261";"The Tourist";"The Xyz 2"
"295";"Three Kings";
"266";"Titan A.E.";"Titan A.E."
"267";"Top Secret!";"Top Secret!"
"268";"Total Recall";"Total Recall"

Montellese added a commit that referenced this pull request May 2, 2012
smartplaylist: use sorttitle in ORDER BY clause to sort movies by title
@Montellese Montellese merged commit 99d7682 into xbmc:master May 2, 2012
@eghetto
Copy link

eghetto commented May 3, 2012

Ahh, I see, thanks! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Fix non-breaking change which fixes an issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants