Fix Missing Art in GetItem when Played from Music Addon. Fixes ticket #15416 #8465
Conversation
…k in CPlayerOperations::GetItem. This now similar to how video handled.
DaveTBlake
added
Type: Fix
|
Obviously +1 from me. Need a Windows build to test if you need confirmations :( Does Kodi compile with VS studio 2015 ? |
|
@Tolriq win32 build started. Will end up on the mirror in ~2 hours with branchname in the filename |
|
@Tolriq Will check this PR soon. |
|
A much better approach than the current implementation or #8440 so +1 from me. |
| SetLabel(music.GetTitle()); | ||
| if (!music.GetURL().empty()) | ||
| m_strPath = music.GetURL(); | ||
| m_bIsFolder = URIUtils::HasSlashAtEnd(m_strPath); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
Different TimeZone will test now :) @Montellese glad to see you back, @DaveTBlake wanted to make some research about the fact that stuff played from GUI have more metadata than stuff played from JSON (specially addons) maybe you have some insight about where he should start looking ? :) |
|
Code looks good, but can't test it right now. Ran into another error when compiling, but it's not related to your change. But it's something I want to look into. |
|
@MartijnKaijser not 100% related but a fresh install of this did directly trigger the message about TADB no more supported for MusicVideo. Seems despite the removal of the support in Jarvis the addon is still shipped / enable for music video by default (I did a fresh install of this test build) |
|
IIRC the main issue is that there are 3-4 different code paths in our code to start playback. One goes through builtins into a playlist, one goes through builtins and playfile, one goes through playlist and one goes directly through playfile. Depending on which path is taken different metadata is available and/or loaded if necessary. Unfortunately not all paths can be taken from all code places (depending on the thread etc) which leads to this mess. |
|
@Tolriq that's because this PR was branched before merging the removal. Next rebase or merge that's fixed. |
|
@MartijnKaijser Another slight problem but long press backspace on keyboard in dialogs does no more repeat the backspace so, it's now horrible to delete long text when you have made a mistake. I did remember seeing a PR for adding some longpress support in keymaps, maybe they should not apply in dialogs ? Same for arrows, dialogs are now quite not good. |
|
can we keep on topic to this PR please? |
|
Well since we are in final betas I find blocking things, forum and trac are not really followed and there's no way to open important issues here that will be followed, at least it's reported do what you want with the information. (I mean look at the trac issue from @da-anda 15 month for something that could have been quickly fixed since ages and he is on the Team) About the PR, I had one of the habitual random Windows crash but I did press debug so it opened VS but without .pdb nothing usable and pressing continue it did continue to work correctly. And no crash dump / log generated. Apart that I have not found any regressions and it does fix the Trac issue mentioned. Sounds good to me. Let's hope @DaveTBlake have time to fix the other JSON metadata problem and it will be perfect for 17. |
|
Forum and Trac ARE followed. Please do not start a discussion about this here. |
|
Then http://forum.kodi.tv/showthread.php?tid=250445 for the issue and remembering of blocking track issue 4 month old without action ;) |
|
@Montellese remembered right regarding the differences in metatdata availability between play from GUI rather than JSON. One play path would be best, so something for K***. Anyone know if there is a ticket for that issue, I find trac hard (and slow) to navigate or search. Meanwhile back with this PR, going to merge before we go off topic any more! |
Fix Missing Art in GetItem when Played from Music Addon. Fixes ticket #15416
|
There is no ticket for that yet. If it qualifies as a bug feel free to create one. |
|
just testend and seemed to work. Thanks for the fix |
This fixes http://trac.kodi.tv/ticket/15416 the specific case that having initiated playing music from an addon
CPlayerOperations::GetItemdoes not return fan art or thumbnail (cover art) when it is available.It does not address the longstanding issue that when play is started from JSON GetItem does not return art work.
This is an alternative fix to that tried in #8440, so see discussion on that thread. Art work was being lost because CFileItem was being created rather than populated from CMusicInfoTag. This fix only impacts music, and makes no changes to video that does not have this bug. It also makes music code more consistent with video for future maintenance. In the longer term a refactoring of fileitem processing would be good, but given uncertainties about how some parts of it work this solution is both sufficient and robust for Jarvis.
This has been tested for playing songs that are both in the library and not, and both from GUI and JSON.
@razzeee, @Montellese (back yet?), @da-anda @Tolriq would like to get this into Jarvis if you agree