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 support for music video collections #12

Merged
merged 3 commits into from
May 10, 2020
Merged

Add support for music video collections #12

merged 3 commits into from
May 10, 2020

Conversation

Maxr1998
Copy link
Contributor

Quickly threw this together, so I'm not 100% sure if it's complete, but in my tests everything worked fine, including the filters.

Before this commit, music video collections would be shown, but they contained no items, because of the includeitemtypes query having the wrong value. I'm not sure if the values of the new constants are correct, or whether I should have "shifted" the other ones to get them into ascending order again, so please let me know what'd be the correct thing here.

@Aanok
Copy link
Owner

Aanok commented May 10, 2020

Oh wow. I'm impressed somebody was brave enough to wade the spaghetti :D

The note about includeitemtypes being the problem is incorrect. Right now jftui will use the generic JF_ITEM_TYPE_COLLECTION for musicvideos CollectionTypes, which makes it treat them like Folders, which is fine. The problem is that the query returns a bunch of MusicVideo items and jftui silently discards them.

The implicit question I have is: what exactly is a MusicVideo collection? I've never used one and had kind of postponed dealing with them when writing the initial core of jftui and then inevitably forgot all about them. The Jellyfin docs have no trace of them, while the Emby wiki just tersely says to use the same naming convention as movies. From a very cursory look, I see they can contain at least Folders, MusicVideos and Movies.

To me this means the most straightforward thing to do is to recognize MusicVideo items and collections as their own entity like you're suggesting, but then treating navigation like you were going by generic Folders. This is what the web client does as well, without recursive and without specifying includeitemtypes in the first place.

How is your MusicVideo collection organized?

By the way, the value of the constants is fine. You can adjust it progressively if you feel like it, or I'll just do it next time I commit something. It's dumb, I do it by hand.

src/menu.c Outdated Show resolved Hide resolved
@Maxr1998
Copy link
Contributor Author

Maxr1998 commented May 10, 2020

Oh wow. I'm impressed somebody was brave enough to wade the spaghetti :D

Haha, nice. I mean, it was bugging me that my music videos didn't show, so I dived straight into it. And honestly, it was pretty easy. 😄

The note about includeitemtypes being the problem is incorrect. Right now jftui will use the generic JF_ITEM_TYPE_COLLECTION for musicvideos CollectionTypes, which makes it treat them like Folders, which is fine. The problem is that the query returns a bunch of MusicVideo items and jftui silently discards them.

Are you sure? When I set includeitemtypes to 'Movie' like it does here in cURL, Jellyfin itself already returns an empty response.

The implicit question I have is: what exactly is a MusicVideo collection? I've never used one and had kind of postponed dealing with them when writing the initial core of jftui and then inevitably forgot all about them. The Jellyfin docs have no trace of them, while the Emby wiki just tersely says to use the same naming convention as movies. From a very cursory look, I see they can contain at least Folders, MusicVideos and Movies.

As I understand it, it's just a custom type of content when creating a new library. At least, it doesn't have to have a structure like music does.

To me this means the most straightforward thing to do is to recognize MusicVideo items and collections as their own entity like you're suggesting, but then treating navigation like you were going by generic Folders. This is what the web client does as well, without recursive and without specifying includeitemtypes in the first place.

I see, so we could actually remove the includeitemttypes and recursive from the URL then? Or simply set the collection type to JF_ITEM_TYPE_FOLDER? Nvm, I think I understand what you mean now. Gimme a sec.

How is your MusicVideo collection organized?

It's actually just a folder with all the music videos places directly into it, no other structure whatsoever. Thus, it shows all files on the same level in jftui.

By the way, the value of the constants is fine. You can adjust it progressively if you feel like it, or I'll just do it next time I commit something. It's dumb, I do it by hand.

Ah, I see, then I'll update the values.

@Aanok
Copy link
Owner

Aanok commented May 10, 2020

Are you sure? When I set includeitemtypes to 'Movie' like it does here in cURL, Jellyfin itself already returns an empty response.

Positive, I ran it on my instance. The reason is probably that your collection doesn't have any Movies: your video files on disk are getting scanned and attached metadata from their file tags, then classified as MusicVideos. To get bare Movies you'd have to disable the metadata providers for the library or have files without tags, I imagine.

Anyways, everything looks good to me now. Thanks for the contribution!

@Aanok Aanok merged commit b9f8911 into Aanok:master May 10, 2020
@Maxr1998
Copy link
Contributor Author

Maxr1998 commented May 10, 2020

The reason is probably that your collection doesn't have any Movies

Yup, I have a separate collection for movies, to keep everything nice & tidy :P

your video files on disk are getting scanned and attached metadata from their file tags, then classified as MusicVideos. To get bare Movies you'd have to disable the metadata providers for the library or have files without tags, I imagine.

Yeah, that's probably what's happening. Anyway, now the 'MusicVideo' type is supported as well 😁

Anyways, everything looks good to me now. Thanks for the contribution!

Thanks for the quick review and merge!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants