qtubuntu-media

Merge lp://staging/~phablet-team/qtubuntu-media/fix-1449790 into lp://staging/qtubuntu-media

fix-1449790
Merge into trunk

Proposed by Jim Hodapp on 2016-02-11

Status:

Merged

Approved by:

Alfonso Sanchez-Beato on 2016-02-22

Approved revision:

106

Merged at revision:

101

Proposed branch:

lp://staging/~phablet-team/qtubuntu-media/fix-1449790

Merge into:

lp://staging/qtubuntu-media

Diff against target:

2525 lines (+1300/-1040)

14 files modified

README (+7/-0)
src/aal/aalmediaplayerservice.cpp (+10/-2)
src/aal/aalmediaplaylistprovider.cpp (+24/-9)
tests/integration/integration.pro (+6/-23)
tests/integration/playlists/playlists.pro (+25/-0)
tests/integration/playlists/tst_mediaplaylist.cpp (+896/-0)
tests/integration/playlists/tst_mediaplaylist.h (+110/-0)
tests/integration/tst_mediaplaylist.cpp (+0/-896)
tests/integration/tst_mediaplaylist.h (+0/-110)
tests/integration/uris/generate_test_files.sh (+25/-0)
tests/integration/uris/remove_test_files.sh (+7/-0)
tests/integration/uris/tst_mediauris.cpp (+116/-0)
tests/integration/uris/tst_mediauris.h (+51/-0)
tests/integration/uris/uris.pro (+23/-0)

To merge this branch:

bzr merge lp://staging/~phablet-team/qtubuntu-media/fix-1449790

High

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Alfonso Sanchez-Beato		2016-02-11	Approve on 2016-02-22
PS Jenkins bot	continuous-integration		Needs Fixing on 2016-02-22
Review via email: mp+285814@code.staging.launchpad.net

Commit message

URI encode the URI that we call setMedia() on or add to the playlist

Description of the change

URI encode the URI that we call setMedia() on or add to the playlist

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-02-12:

PASSED: Continuous integration, rev:103
http://jenkins.qa.ubuntu.com/job/qtubuntu-media-ci/138/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtubuntu-media-vivid-amd64-ci/63
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtubuntu-media-vivid-armhf-ci/63
        deb: http://jenkins.qa.ubuntu.com/job/qtubuntu-media-vivid-armhf-ci/63/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtubuntu-media-vivid-i386-ci/63

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/qtubuntu-media-ci/138/rebuild

review: Approve (continuous-integration)

lp://staging/~phablet-team/qtubuntu-media/fix-1449790 updated on 2016-02-15

104. By Jim Hodapp on 2016-02-15: Make sure to URI encode the track paths that get added via playlists

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-02-15:

PASSED: Continuous integration, rev:104
http://jenkins.qa.ubuntu.com/job/qtubuntu-media-ci/139/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtubuntu-media-vivid-amd64-ci/64
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtubuntu-media-vivid-armhf-ci/64
        deb: http://jenkins.qa.ubuntu.com/job/qtubuntu-media-vivid-armhf-ci/64/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtubuntu-media-vivid-i386-ci/64

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/qtubuntu-media-ci/139/rebuild

review: Approve (continuous-integration)

Revision history for this message

Alfonso Sanchez-Beato (alfonsosanchezbeato) wrote on 2016-02-16:

My main concern with this change is whether we handle properly UTF-8 or not. The toLocal8Bit() suggests we might be breaking support for non-ascii file names (although I am not sure if we supported that before).

I think it is a good opportunity to add tests for UTF-8 (this MP adds infrastructure for that, like generate_test_files.sh, which is great) and make sure those files get played as expected.

Finally, I have an inline comment.

review: Needs Fixing

lp://staging/~phablet-team/qtubuntu-media/fix-1449790 updated on 2016-02-19

105. By Jim Hodapp on 2016-02-19: Simply pass the URL string through to media-hub, no longer need to do any special handling since media-hub takes care of it all