Merge lp:~renatofilho/mediaplayer-app/fix-app-crash into lp:mediaplayer-app

[Renato Araujo Oliveira Filho]

Fixed application exit during the thumbnail generation.
Avoid application crashes due a SDK bug.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:80
http://jenkins.qa.ubuntu.com/job/mediaplayer-app-ci/40/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-raring-amd64-ci/11
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-raring-armhf-ci/39
        deb: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-raring-armhf-ci/39/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-raring-i386-ci/39

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mediaplayer-app-ci/40/rebuild

review: Approve (continuous-integration)

[Florian Boucault (fboucault)]

 https://bugs.launchpad.net/ubuntu-ui-toolkit/+bug/1168006 is now fixed

[lp:~renatofilho/mediaplayer-app/fix-app-crash]

81. By Renato Araujo Oliveira Filho on 2013-05-08

Added new package (qt 5.0.2) libqt5multimedia5-plugins as dependency.

82. By Renato Araujo Oliveira Filho on 2013-05-08

Fixed theme loader with a proper solution.

[Gustavo Pichorim Boiko (boiko)]

56 delete m_player;
57 + m_player = 0;

As you have used a mutex in the other played destruction code, shouldn't you add the mutex here too?

review: Needs Information

[Gustavo Pichorim Boiko (boiko)]

76 + // check if the player exists ( the application still running )
77 + if (!m_player) {
78 + return QImage();
79 + }
80 +
81 + m_mutex.lock();

Why there is the need to check for m_player before and after locking the mutex? Wouldn't it be enough to check for the player just after the mutex acquires the lock?

review: Needs Information

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:82
http://jenkins.qa.ubuntu.com/job/mediaplayer-app-ci/41/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-raring-amd64-ci/12
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-raring-armhf-ci/40
        deb: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-raring-armhf-ci/40/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-raring-i386-ci/40

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mediaplayer-app-ci/41/rebuild

review: Approve (continuous-integration)

[lp:~renatofilho/mediaplayer-app/fix-app-crash]

83. By Renato Araujo Oliveira Filho on 2013-05-08

Used QMutexLocker to make the code cleaner.

84. By Renato Araujo Oliveira Filho on 2013-05-08

Added more comments.

85. By Renato Araujo Oliveira Filho on 2013-05-08

Reverted qt 5.0.2 related changes.

[Tiago Salem Herrmann (tiagosh)]

looks good to me.
+1.

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:85
http://jenkins.qa.ubuntu.com/job/mediaplayer-app-ci/42/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-raring-amd64-ci/13
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-raring-armhf-ci/41
        deb: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-raring-armhf-ci/41/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-raring-i386-ci/41

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mediaplayer-app-ci/42/rebuild

review: Approve (continuous-integration)

=== modified file 'src/qml/player/TimeLine.qml'
--- src/qml/player/TimeLine.qml	2013-04-15 12:58:52 +0000
+++ src/qml/player/TimeLine.qml	2013-05-08 21:25:27 +0000
@@ -20,6 +20,7 @@
 import QtQuick 2.0
 import Ubuntu.Components 0.1
 import "../sdk"
+import "../theme"
 
 Item {
     id: _timeLine
@@ -40,14 +41,11 @@
     // "property alias value: _slider.value" does not work
     Binding { target: _slider; property: "value"; value: _timeLine.value }
 
-    Component.onCompleted: {
-        var result = Theme.loadTheme(Qt.resolvedUrl("../theme/theme.qmltheme"))
-    }
-
     Slider {
         id: _slider
 
         objectName: "TimeLine.Slider"
+        ItemStyle.delegate: VideoSlider {property Item item: _slider}
         anchors {
             top: parent.top
             bottom: parent.bottom

=== modified file 'src/thumbnail-provider.cpp'
--- src/thumbnail-provider.cpp	2013-04-04 18:55:30 +0000
+++ src/thumbnail-provider.cpp	2013-05-08 21:25:27 +0000
@@ -20,6 +20,7 @@
 #include <QtCore/QStringList>
 #include <QtCore/QTimer>
 #include <QtCore/QMutex>
+#include <QtCore/QMutexLocker>
 #include <QtCore/QCoreApplication>
 #include <QtMultimedia/QVideoRendererControl>
 #include <QtMultimedia/QMediaService>
@@ -30,14 +31,16 @@
       QQuickImageProvider(QQuickImageProvider::Image),
       m_player(0)
 {
-    connect(qApp, SIGNAL(aboutToQuit()), this, SLOT(applicationAboutToQuit()));
+    connect(qApp, SIGNAL(aboutToQuit()), this, SLOT(applicationAboutToQuit()), Qt::DirectConnection);
     createPlayer();
 }
 
 ThumbnailProvider::~ThumbnailProvider()
 {
     if (m_player) {
+        QMutexLocker locker(&m_mutex);
         delete m_player;
+        m_player = 0;
     }
 }
 
@@ -48,6 +51,7 @@
 
 void ThumbnailProvider::applicationAboutToQuit()
 {
+    QMutexLocker locker(&m_mutex);
     delete m_player;
     m_player = 0;
 }
@@ -79,6 +83,19 @@
         return QImage();        
     }
 
+    // check if the player exists ( the application still running )
+    if (!m_player) {
+        return QImage();
+    }
+
+    QMutexLocker locker(&m_mutex);
+
+    // again check if the player exits after lock the mutex, since this function will run in a separed thread,
+    // the application could be destroyed at this point and the function applicationAboutToQuit was called
+    if (!m_player) {
+        return QImage();
+    }
+
     if (uri != m_player->uri()) {
         m_player->setUri(uri);
         m_cache.clear();
@@ -89,7 +106,9 @@
         img = m_cache[time];
     } else {
         img = m_player->request(time, requestedSize).copy();
-        m_cache.insert(time, img);
+        if (!img.isNull()) {
+            m_cache.insert(time, img);
+        }
     }
 
     *size = img.size();

=== modified file 'src/thumbnail-provider.h'
--- src/thumbnail-provider.h	2013-02-12 14:30:57 +0000
+++ src/thumbnail-provider.h	2013-05-08 21:25:27 +0000
@@ -23,6 +23,7 @@
 #include <QtGui/QImage>
 #include <QtCore/QMap>
 #include <QtCore/QQueue>
+#include <QtCore/QMutex>
 
 class ThumbnailPipeline;
 
@@ -42,6 +43,7 @@
     private:
         ThumbnailPipeline *m_player;
         QMap<qint64, QImage> m_cache;
+        QMutex m_mutex;
 
         void createPlayer();
         QString parseThumbnailName(const QString &id, qint64 *time) const;