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 a method to retrieve video tags #7369

Merged
merged 1 commit into from
Jun 2, 2016
Merged

Conversation

phate89
Copy link
Contributor

@phate89 phate89 commented Jun 30, 2015

Right now you can retrieve all tags from a media type using a "trick" (Files.GetDirectory to videodb://movies/tags/ for example) but imho a specific method is better.
Exactly like "GetGenres" is more obvious to try to use a "GetTags" instead of using files.getdirectory

"transport": "Response",
"permission": "ReadData",
"params": [
{ "name": "type", "type": "string", "required": true, "enum": [ "movie", "tvshow", "musicvideo"] },

This comment was marked as spam.

@Montellese Montellese added Type: Feature non-breaking change which adds functionality API change labels Jun 30, 2015
@Montellese Montellese self-assigned this Jun 30, 2015
@Montellese
Copy link
Member

Looks ok apart from the minors.

@phate89
Copy link
Contributor Author

phate89 commented Jun 30, 2015

It should be fixed now. I don't know why I thought tags had thumbnails..

return InternalError;

/* need to set strTitle in each item*/
for (unsigned int i = 0; i < static_cast<unsigned int>(items.Size()); i++)

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@Tolriq
Copy link
Contributor

Tolriq commented Jul 3, 2015

Since this is related to the GetGenres too, is it possible to complete the PR to also return the TAG ids when querying for the movies ?

Because as with genre this will lead to a list of movie with tags as strings and a getTags that return the tags and ids but no way to link them in offline cache (for remotes for example) except by recreating fake ids or tons of string searches, leading to this function a little less useful.

(Of course same thing for genre would be perfect).

@razzeee
Copy link
Member

razzeee commented Dec 16, 2015

@phate89 how ready is this to go in?

@phate89
Copy link
Contributor Author

phate89 commented Dec 16, 2015

I still need to look If I can do the changes requested by tolriq...

@Tolriq
Copy link
Contributor

Tolriq commented Dec 17, 2015

If the method is needed for other things, do not block for the ids.

Ids would be cool for remotes for genre and tags, but there's workaround, if this open things for Kodi internals or addons then let's use the workaround for remotes until maybe ids are added later.

@phate89
Copy link
Contributor Author

phate89 commented Apr 5, 2016

rebased
@Tolriq adding ids to getdetails is not that easy because they're not parsed at all (they're stored as a list of strings right now both genres and tags)
So for now (finally) I want to get this pr in..
Do you have new comments/do you want to test it?

@Montellese
Copy link
Member

Please fix the issue brought up by @Jalle19. Apart from that it looks good to me.

The only thing I've been wondering is if there's any sorting for tags that makes sense at all.

@Tolriq
Copy link
Contributor

Tolriq commented Apr 5, 2016

@phate89 nothing more from me if Id can not be returned from getMovies / Details:)

I can test if you want but for Yatse I won't use as it's faster to reconstruct the TAG data from getMovies than do another query that returns id that I can't use :(

But need a test build as there's the VS 2015 migration occurring and I'm still at 2013 until all is stable :)

@phate89
Copy link
Contributor Author

phate89 commented Apr 5, 2016

@Montellese I left it like getgenres and the case is the same.. Should I drop sorting?

@Tolriq http://mirrors.kodi.tv/test-builds/win32/KodiSetup-20160405-08edc16-expose_tag_json.exe for testing

@Tolriq
Copy link
Contributor

Tolriq commented Apr 5, 2016

I know not the correct place bla bla :)

But addons\skin.re-touched is shipped with _screenshots folder + some git files + build.bat

  • build is missing python27.dll

Except that, all works at JSON level.

But as a side note it's incredibly complicated now to find the file entry for video to create a musicvideo source. Specially with the file manager that is easy to access from main screen and will lead users to add sources there :(

@phate89
Copy link
Contributor Author

phate89 commented Apr 18, 2016

rebased
@Montellese do I have to drop sorting field? I don't think there are options to sort tags but I'm not sure

@Montellese
Copy link
Member

We can keep it for consistency but it's not much use.

@phate89
Copy link
Contributor Author

phate89 commented Jun 2, 2016

jenkins build this please

@Montellese
Copy link
Member

No objections from me.

@phate89 phate89 merged commit 91a1cd5 into xbmc:master Jun 2, 2016
@phate89 phate89 deleted the expose_tag_json branch June 18, 2016 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API change: JSON-RPC Type: Feature non-breaking change which adds functionality v17 Krypton Wiki: Done
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants