Review Request 123467: Add CoverArt uri to the metadata exposed with MPRIS interfaces

[Sebastian Kügler]

> On April 22, 2015, 10:35 p.m., Sebastian Kügler wrote:
> > libs/mediacenter/mpris2/mpris2.cpp, line 33
> > <https://git.reviewboard.kde.org/r/123467/diff/3/?file=362527#file362527line33>
> >
> >     Actually, this is wrong now. Using the native separator may produce \ (on Windows, for example), which is then concatenated with a /-separated path.
> >     
> >     The old version was correct, though, as it used a path returned from QDir, which is separated by / (always, Qt does that internally), and you concat it with a /-separated path. That's the correct way to do it.
> >     
> >     This is a bit confusing, but a general rule of thumb is to *not* use QDir::separator() or QDir::toNativeSeparator() when you're building paths without user input or anything that's coming from external parts.
> 
> Ashish Madeti wrote:
>     1. I am first concatenating QDir::tempPath() with QLatin1String("/plasma-mediacenter/covers/") and then passing them to toNativeSeparators(). Is it still wrong?
>     2. If I build paths without separator() and toNativeSeparator() will they still work on Windows?

1. Yes, in the code, at this point, you can only get forward slashes, so stick to using them.
2. Yes, that'll work.


- Sebastian


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123467/#review79364
-----------------------------------------------------------


On April 22, 2015, 4:17 p.m., Ashish Madeti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123467/
> -----------------------------------------------------------
> 
> (Updated April 22, 2015, 4:17 p.m.)
> 
> 
> Review request for Plasma, Bhushan Shah, Shantanu Tushar, and Sinny Kumari.
> 
> 
> Repository: plasma-mediacenter
> 
> 
> Description
> -------
> 
> Save Cover art to a temporary folder. Include the path of the cover art in the metadata currently exposed with MPRIS interfaces.
> 
> 
> Diffs
> -----
> 
>   libs/mediacenter/mpris2/mpris2.cpp f03d062 
> 
> Diff: https://git.reviewboard.kde.org/r/123467/diff/
> 
> 
> Testing
> -------
> 
> Correct path is showing up in the metadata when tested with mpristester.
> Correct cover art is showing up in the mpris controller (appearing in system tray) of Plasma 5.
> 
> 
> Thanks,
> 
> Ashish Madeti
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20150423/49d78c8f/attachment.html>