Review Request: Rework MediaSource(const QString &filename) constructor in phonon five.

[David Faure]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/106974/#review21340
-----------------------------------------------------------

Ship it!


Looks good to me now.

Extra nitpicking would be "get rid of the code duplication for the case of qrc urls", but if the first one is deprecated, it's probably not worth it.

I'm also surprised that local files are "Url, using QUrl", not "Stream, using QFile" -- but that's unrelated to your change and I don't know phonon internals anyway.

- David Faure


On Nov. 1, 2012, 1:48 p.m., Jon Severinsson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106974/
> -----------------------------------------------------------
> 
> (Updated Nov. 1, 2012, 1:48 p.m.)
> 
> 
> Review request for KDE Frameworks and Phonon.
> 
> 
> Description
> -------
> 
> QFSFileEngine has been removed from Qt5, so the MediaSource(const QString &filename) constructor has to be reworked to not need it.  The only behaviourial changes from the original code are that non-local absolute file paths (eg smb share file paths such as "//hostname/share/path") and "qrc:///" URLs actually works.
> 
> I have also added qrc support to the QUrl constructor, and deprecated the QString constructor in favour of it.
> 
> 
> Diffs
> -----
> 
>   phonon/mediasource.h 5374f00 
>   phonon/mediasource.cpp 9e35094 
> 
> Diff: http://git.reviewboard.kde.org/r/106974/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jon Severinsson
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20121102/8d97c329/attachment.html>