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

Provide skins access to the Database/Library ID's for Audio/Video #1017

Merged
merged 1 commit into from Jun 2, 2012

Conversation

Giftie
Copy link
Contributor

@Giftie Giftie commented May 26, 2012

This PR add the following ListItems:

ListItem.DBType <- provides the Database Type for video(availible types - movie, tvshow, episode, musicvideo, set, season)
ListItem.DBID <- provides Database ID of the current selected Audio/Video Library level

@Giftie
Copy link
Contributor Author

Giftie commented May 26, 2012

It might be better to remove the ListItem.ArtistID/AlbumID and add the ListItems.Properties(artist_id/album_id) from PR #963

@jmarshallnz
Copy link
Contributor

For the music artist ID/album ID you need to add SetDatabaseId() in the MusicDatabase::GetArtistsByWhere, GetAlbumsByWhere - I have them in my tree already due to my work on music thumbs, but feel free to add them in here if that's what the problem is.

Not sure how to best handle the other properties (artistid/albumid) - I presume they're useful on songs and albums (artistid) primarily? Also, technically speaking the artistid is not unique for a song or an album, though ofcourse albumID is unique for a song.

@Giftie
Copy link
Contributor Author

Giftie commented May 26, 2012

@jmarshallnz - not sure which branch of yours to look in for the code you think might fix the problem.

@jmarshallnz
Copy link
Contributor

Heh - I hadn't pushed. Turns out that I don't have such a commit.

For albums, I think you need to set it in CMusicInfoTag::SetAlbum() (the one that takes a CAlbum).

Artists should already be OK by the looks (it's set in GetArtistsByWhere at least?)

@Giftie
Copy link
Contributor Author

Giftie commented May 31, 2012

@jmarshallnz - I have tried(and failed) to have the Artist's ID and Album's ID(using ListItem) set when accessing them thru the their Info screens(ie, DialogAlbumInfo.xml) So I've put back ListItem.Property(Album_ID) and ListItem.Property(Artist_ID). These ID's properly match the ID's in the Database albumview. I'm not sure if I can do a better job.

@jmarshallnz
Copy link
Contributor

Have you tried setting the id in CGUIDialogMusicInfo::SetAlbum/SetArtist - by the looks there is no id there (and no retrieval of that info from the db). You should have the id in the CAlbum and CArtist passed in.

And please unify to ListItem.DbId - no need for a difference between music and video for this.

@Giftie
Copy link
Contributor Author

Giftie commented Jun 2, 2012

By using the following in CGUIDialogMusicInfo::SetAlbum

  m_albumItem->GetMusicInfoTag()->SetDatabaseId(m_album.idAlbum);

and in CGUIDialogMusicInfo::SetArtist

  m_albumItem->GetMusicInfoTag()->SetDatabaseId(m_artist.idArtist);

ListItem.DBID reports the ID's for the Artist, Album and Song when on the matching Music Info screen. I'll rebase after I do one final compile(I wish I had a multi-core processor on my XBMC Machine... )

@Giftie
Copy link
Contributor Author

Giftie commented Jun 2, 2012

Ok squashed and rebased against master... I hope this turns out to be the proper way. It works properly.

@ghost ghost assigned jmarshallnz Jun 2, 2012
@jmarshallnz
Copy link
Contributor

Looks good - nice work :)

jmarshallnz added a commit that referenced this pull request Jun 2, 2012
Provide skins access to the Database/Library ID's for Audio/Video
@jmarshallnz jmarshallnz merged commit 1b67386 into xbmc:master Jun 2, 2012
@Giftie
Copy link
Contributor Author

Giftie commented Jun 2, 2012

@jmarshallnz - Thanks for the assist.. I really need to learn C++...

@MartijnKaijser
Copy link
Member

thank you @Giftie :D

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

3 participants