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

Add Audio and Subtitle Track Count to Smart Playlist selections #8333

Merged
merged 2 commits into from Dec 5, 2015

Conversation

TonyPh12345
Copy link
Contributor

This adds a new Smart Playlist Rule field named "audiocount" which can be used to locate video files that have a specific number of audio tracks.

I'm a big fan of commentary tracks, and using the existing GUI to locate movies or TV show episodes that have commentaries is almost impossible -- one must be using a skin that supports that info and displays it in navigation. And even then, one must scroll through the library to locate the files.

This addition makes it simple -- just make a playlist for Movies or Episodes that contain more than 1 audio tracks. Such as:

<?xml version="1.0" encoding="UTF-8" standalone="yes" ?>
<smartplaylist type="episodes">
    <name>Test</name>
    <match>all</match>
    <rule field="audiocount" operator="greaterthan">
        <value>1</value>
    </rule>
    <order direction="ascending">tvshowtitle</order>
</smartplaylist>

@TonyPh12345
Copy link
Contributor Author

Hmm. I'm now discovering that the database isn't always pre-populated with the stream details -- I had to browse into SOME of my TV shows using the standard "TV Shows" view, and the log then indicated it was checking stream details. Other shows had been fully populated. Not sure why some where and others weren't...

Is that typical behavior? Or is it because I ran this compiled fork onto an existing Isengard database?

@phil65
Copy link
Contributor

phil65 commented Nov 2, 2015

It is typical behaviour.

@mkortstiege
Copy link
Member

Looks good to me. While at it mind adding the same for subtitle count? Video doesn't make much sense here i guess. Thanks.

@razzeee razzeee added Type: Feature non-breaking change which adds functionality Component: Video Component: Database v17 Krypton labels Nov 2, 2015
@TonyPh12345 TonyPh12345 changed the title Add Audio Track Count to Smart Playlist selections Add Audio and Subtitle Track Count to Smart Playlist selections Nov 2, 2015
@TonyPh12345
Copy link
Contributor Author

@mkortstiege : Done.

I also corrected the word case (I had capitalized the words in the string (Audio Track Count) but the convention appears to be capitalizing only the first word (Audio track count).

All: Is there a CONSTANT that should be used instead of explicitly giving an integer for iStreamType in the database query?

If those values change due to some other PR, this query will break -- I don't know how to look through all the constants to see if there's a C++ constant to put in place there.

In other words, instead of

 EXISTS (SELECT 1 FROM streamdetails WHERE streamdetails.idFile = " + table + ".idFile AND streamdetails.iStreamType=1 GROUP BY streamdetails.idFile HAVING COUNT(streamdetails.iStreamType)

should it be

 EXISTS (SELECT 1 FROM streamdetails WHERE streamdetails.idFile = " + table + ".idFile AND streamdetails.iStreamType=someDefinedConstant GROUP BY streamdetails.idFile HAVING COUNT(streamdetails.iStreamType)

@razzeee
Copy link
Member

razzeee commented Nov 2, 2015

@TonyPh12345
Copy link
Contributor Author

Humm. That's outta this novice's wheelhouse. How does Kodi conventions dictate typecasting that integer into a string?

I'd probably use std::to_string() but I don't see that used anywhere else in the code.

I appreciate y'all's patience and advice.

@TonyPh12345
Copy link
Contributor Author

Will that work? (I mean, it does do what's intended, but is it "legal"?)

@phate89
Copy link
Contributor

phate89 commented Nov 3, 2015

StringUtils::Format() or in this case db.PrepareSQL()
for example:

StringUtils::Format("%i",intNum);

std::to_string() doesn't work in android

…strings

Changed select type to use constants already defined instead of literal integers; also added data validation to limit to positive integers.

More consistent query string formatting
@TonyPh12345
Copy link
Contributor Author

Squashed some and integrated @phate89 suggestion

@TonyPh12345
Copy link
Contributor Author

@phil65 , is there a way to trigger the system to pull the details into the database? This modification I'm proposing loses its appeal if the required data isn't in the database without the user having to trigger the scan for each series...

@razzeee
Copy link
Member

razzeee commented Nov 8, 2015

Not possible right now, as far as I know. Only happens when you browse the media.

@TonyPh12345
Copy link
Contributor Author

Just noticed that some playlist selection criteria that have existed "forever" such as audio language or subtitle language are similarly limited even though the Wiki doesn't mention it so it seems a precedent is already set...

So, I'm hoping this can be merged. I've been compiling this same change into Isengard on Pi and Linux and am quite enjoying it.

@razzeee
Copy link
Member

razzeee commented Nov 11, 2015

We'll merge it when the next merge windows for K****** opens up, but that might take some days.

@MartijnKaijser MartijnKaijser modified the milestone: K***** 17.0-alpha1 Dec 4, 2015
@MartijnKaijser
Copy link
Member

jenkins build and merge

@jenkins4kodi jenkins4kodi merged commit 70cd30f into xbmc:master Dec 5, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants