[Music]Completing the move to using Artist Credits rather than Artist Vector
#1
Music 
Keeping design discussion off Github...

I feel that we need to make some changes to the multiple ways we hold artist names, and inconsistent way they are passed to different processes. I suspect that is giving us so problems with JSON output and NFO application, but if not it would still be good to make the design more robust.

Back in Gotham the concept of artist credits was introduced, this holds both artist name and MusicBrainz ID. The artists for songs and albums are now held in a vector of these objects. Previously the artist names were just held in a string vector, and this has been continued. We also store the concatonated names as a string, but this has a reasonable use.

I think that rather than try and maintain the same data in several forms we should stop using the string vector and in situations that a string vector is required use a method that extracts that information from the artist credits. An initial step in this direction has been made by PR8079, that uses a method to get artist names from Song and Album into CMusicInfoTag, but does not remove the string vector.

Because this change effect a number of files I would like some indication for dev team members if this is OK, rather than waste my time on editing code only to be rejected. It is really just good housekeeping rather than design change.

Anyone?
Reply
#2
We can't really say until we see the code, but if you think its a good idea then go for it and get a PR up.

As you know, the team believes the music handling needs improvement.

The best way to know is get some test builds up and test it with users, if it improves scanning and makes it more consistent then there wont be too many objections I would imagine Wink
Reply
#3
Or do some basic proof of concept push to your Github account then feel free to ping people on the commits within your account so any discussion happens there.
Reply
#4
Thanks for input Zag and JJD. I'm away from my dev environment (at a family funeral) so can't do much but read.

Apparently I have missed the merge window anyway, what is that exactly? Are there set dates I could know about so I can plan my work? [Edit: Martijn just explained last 10 days of month used for fixes]

Seems any of my work, even a one line change, needs a sign off from at least 2 team members with a confidence to approve changes to the music area. That's understandable, but it seems Kodi is short of suitable members. Who can I ping?
Reply
#5
(2015-09-21, 17:27)jjd-uk Wrote: Or do some basic proof of concept push to your Github account then feel free to ping people on the commits within your account so any discussion happens there.

OK, got WIP on my repo https://github.com/DaveTBlake/xbmc/tree/...tor_method. Not was widely impacting as I first thought, and not ready to go yet either but just in case someone facies a look.

It has revealed some design weaknesses/inconsistencies in JSON interface and loading AlbumInfo which I will look at separately. Means it was a worthwhile tidy up.

Not sure how to ping others from commits on my repo, so you can all live in peace for now. Smile
Reply
#6
Montellese does the JSON stuff so always ping him when you throw up the PR

Had a quick scan through the code, and looks like its moving things in the right direction for getting the Artist names
Reply
#7
(2015-09-28, 20:48)DaveBlake Wrote:
(2015-09-21, 17:27)jjd-uk Wrote: Or do some basic proof of concept push to your Github account then feel free to ping people on the commits within your account so any discussion happens there.

OK, got WIP on my repo https://github.com/DaveTBlake/xbmc/tree/...tor_method. Not was widely impacting as I first thought, and not ready to go yet either but just in case someone facies a look.

It has revealed some design weaknesses/inconsistencies in JSON interface and loading AlbumInfo which I will look at separately. Means it was a worthwhile tidy up.

Not sure how to ping others from commits on my repo, so you can all live in peace for now. Smile

Once you're happy your series of commits which shows your intentions then go the the latest one i.e. https://github.com/DaveTBlake/xbmc/commi...f49929cc24

Then in the comment box at the bottom use @git_username e.g. @jjd-uk to ping people and explain what you need feedback on.

This way you can get early feedback before it's ready to PR, or even if already PR'ed you can do it this way to get code comments without generating notifications to the whole Kodi community. For example, you have an idea for how to do something but there might be several possible paths in doing this, so choose what you think is best so you can do a rough proof of concept to establish the intention, then you can ping people if you want to be sure that the chosen path is the right one to follow before you get too far down the road.
Reply
#8
Other option is to submit a [WIP] PR that is clearly labelled Work In Progress. Then people don't review it completely and are open to major changes.
Reply
#9
Gone for a PR 8158 (PR). Ready for review, but the weak areas it revealed will need yet another PR to fix. Trying the tiny step thing Smile

I suspect the code syntax may get a good check, but I would rather get some data through it - scan tags, import/export, scrape and NFO, even some JSON access.

Boring housekeeping, before the good stuff!!
Reply

Logout Mark Read Team Forum Stats Members Help
[Music]Completing the move to using Artist Credits rather than Artist Vector0