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

[Renato Araujo Oliveira Filho]

Avoid black thumbnails on scene selector.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:68
http://jenkins.qa.ubuntu.com/job/mediaplayer-app-ci/18/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-quantal-armhf-ci/18
        deb: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-quantal-armhf-ci/18/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-quantal-i386-ci/18
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-raring-armhf-ci/17
        deb: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-raring-armhf-ci/17/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-raring-i386-ci/17
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/618

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

review: Needs Fixing (continuous-integration)

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

69. By Renato Araujo Oliveira Filho on 2013-04-04

Fixed scene selector for small videos.
Returned a invalid QImage if a good frame was not found, during the thumbnail generation.

70. By Renato Araujo Oliveira Filho on 2013-04-04

Removed invalid comment.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:70
http://jenkins.qa.ubuntu.com/job/mediaplayer-app-ci/19/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-quantal-armhf-ci/19
        deb: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-quantal-armhf-ci/19/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-quantal-i386-ci/19
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-raring-armhf-ci/18
        deb: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-raring-armhf-ci/18/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-raring-i386-ci/18
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/621

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

review: Approve (continuous-integration)

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

71. By Renato Araujo Oliveira Filho on 2013-04-04

uses standard deviation to discovery a good image for thumbnail.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:71
http://jenkins.qa.ubuntu.com/job/mediaplayer-app-ci/20/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-quantal-armhf-ci/20
        deb: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-quantal-armhf-ci/20/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-quantal-i386-ci/20
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-raring-armhf-ci/19
        deb: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-raring-armhf-ci/19/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-raring-i386-ci/19
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/632

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

review: Approve (continuous-integration)

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

72. By Renato Araujo Oliveira Filho on 2013-04-04

Created unit test to check for valid thumbnails.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:72
http://jenkins.qa.ubuntu.com/job/mediaplayer-app-ci/21/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-quantal-armhf-ci/21/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-quantal-i386-ci/21/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-raring-armhf-ci/20/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-raring-i386-ci/20/console

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

review: Needs Fixing (continuous-integration)

[Gustavo Pichorim Boiko (boiko)]

Please use data-driven tests for trying the isMeaningful functions instead of one function per image.

review: Needs Fixing

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

73. By Renato Araujo Oliveira Filho on 2013-04-04

Make sure that image and videos sample data get processed first then unittest project to correct export the variables.

74. By Renato Araujo Oliveira Filho on 2013-04-04

Updated test to use data-driven tests.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:74
http://jenkins.qa.ubuntu.com/job/mediaplayer-app-ci/22/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-quantal-armhf-ci/22
        deb: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-quantal-armhf-ci/22/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-quantal-i386-ci/22
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-raring-armhf-ci/21
        deb: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-raring-armhf-ci/21/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediaplayer-app-raring-i386-ci/21
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/637

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

review: Approve (continuous-integration)

[Gustavo Pichorim Boiko (boiko)]

Looks good now.

review: Approve

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2013-02-21 17:40:40 +0000
+++ CMakeLists.txt	2013-04-04 22:28:27 +0000
@@ -19,6 +19,9 @@
 
 set(MEDIAPLAYER_DIR ${CMAKE_INSTALL_DATADIR}/media-player)
 
+# Tests
+enable_testing()
+
 add_subdirectory(data)
 add_subdirectory(sdk)
 add_subdirectory(src)

=== modified file 'src/qml/player/Controls.qml'
--- src/qml/player/Controls.qml	2013-03-06 15:10:16 +0000
+++ src/qml/player/Controls.qml	2013-04-04 22:28:27 +0000
@@ -276,12 +276,12 @@
             _sceneSelectorModel.clear()
             var frameSize = video.duration / 10;
             for (var i = 0; i < 10; ++i) {
-                // TODO: discuss this with designers
-                // shift 3s to avoid black frame in the position 0
                 var pos = Math.floor(i * frameSize);
-                _sceneSelectorModel.append({"thumbnail": "image://video/" + video.source + "/" + (pos + 3000),
-                                            "start" : pos,
-                                            "duration" : frameSize})
+                if (pos <= video.duration) {
+                    _sceneSelectorModel.append({"thumbnail": "image://video/" + video.source + "/" + pos,
+                                                "start" : pos,
+                                                "duration" : frameSize})
+                }
              }
         }
     }

=== modified file 'src/qml/player/SceneFrame.qml'
--- src/qml/player/SceneFrame.qml	2013-03-06 13:33:38 +0000
+++ src/qml/player/SceneFrame.qml	2013-04-04 22:28:27 +0000
@@ -53,13 +53,14 @@
             }
         }
 
-
         image: Image {
             id: _image
 
             fillMode: Image.PreserveAspectCrop
             smooth: true
             asynchronous: true
+            sourceSize.width: _shape.width
+            sourceSize.height: _shape.height
         }
     }
 

=== modified file 'src/thumbnail-pipeline-gst.cpp'
--- src/thumbnail-pipeline-gst.cpp	2013-02-12 14:30:57 +0000
+++ src/thumbnail-pipeline-gst.cpp	2013-04-04 22:28:27 +0000
@@ -16,6 +16,7 @@
 
 #include <QDebug>
 #include <gst/gst.h>
+#include <math.h>
 
 #include "thumbnail-pipeline-gst.h"
 
@@ -152,31 +153,71 @@
     return QImage();
 }
 
-QImage ThumbnailPipeline::parseImage(ThumbnailImageData *buffer)
+QImage ThumbnailPipeline::parseImage(ThumbnailImageData *buffer) const
 {
     return parseImageGst(buffer);
 }
 
-QImage ThumbnailPipeline::request(qint64 time)
+// use standard deviation of the histogram to discovery a good image
+bool ThumbnailPipeline::isMeaningful(QImage img)
+{   
+    const static int threshold = 15;
+    const float average = (img.height() * img.width()) / 256;
+    int histogram[256];
+
+    memset(histogram, 0, sizeof(int) * 256);
+    for(int h=0, hMax = img.height(); h < hMax; h++) {
+        for(int w=0, wMax = img.width(); w < wMax; w++) {
+            histogram[qGray(img.pixel(w, h))]++;
+        }
+    }
+
+    float sum = 0;
+    for(int i=0; i < 256; i++) {
+        sum += pow(average - histogram[i], 2);
+    }
+
+    return ((sqrt(sum / 256) / average) <= threshold);
+}
+
+QImage ThumbnailPipeline::request(qint64 time, QSize size, bool skipBlack)
 {    
     if (m_pipeline == 0) {
         qWarning() << "Pipiline not ready";
         return QImage();
     }
 
-	gst_element_seek (m_pipeline, 1.0,
-                      GST_FORMAT_TIME,  static_cast<GstSeekFlags>(GST_SEEK_FLAG_FLUSH | GST_SEEK_FLAG_KEY_UNIT),
-                      GST_SEEK_TYPE_SET, time * GST_MSECOND,
-                      GST_SEEK_TYPE_NONE, GST_CLOCK_TIME_NONE);
-
-	/* And wait for this seek to complete */
-	gst_element_get_state (m_pipeline, NULL, NULL, GST_CLOCK_TIME_NONE);
-
-    /* get frame */
-    ThumbnailImageData *buf = 0;
-
-    g_signal_emit_by_name (m_pipeline, "convert-frame", m_caps, &buf);
-    QImage img = parseImage (buf);
-
-    return img;
+
+    QImage firstImage;
+    while(time < m_duration) {
+        gst_element_seek (m_pipeline, 1.0,
+                          GST_FORMAT_TIME,  static_cast<GstSeekFlags>(GST_SEEK_FLAG_FLUSH | GST_SEEK_FLAG_KEY_UNIT),
+                          GST_SEEK_TYPE_SET, time * GST_MSECOND,
+                          GST_SEEK_TYPE_NONE, GST_CLOCK_TIME_NONE);
+
+        /* And wait for this seek to complete */
+        gst_element_get_state (m_pipeline, NULL, NULL, GST_CLOCK_TIME_NONE);
+
+        /* get frame */
+        ThumbnailImageData *buf = 0;
+
+        g_signal_emit_by_name (m_pipeline, "convert-frame", m_caps, &buf);
+        QImage img = parseImage (buf);
+        if (size.isValid()) {
+            img = img.scaled(size, Qt::KeepAspectRatio, Qt::SmoothTransformation);
+        }
+
+        if (skipBlack && !isMeaningful (img)) {
+            if (firstImage.isNull()) {
+                firstImage = img.copy();
+            }
+            time += 1000;
+            continue;
+        } else {
+            return img;
+        }
+    }
+
+    // return the original frame if any other was found
+    return firstImage;
 }

=== modified file 'src/thumbnail-pipeline-gst.h'
--- src/thumbnail-pipeline-gst.h	2013-02-12 14:30:57 +0000
+++ src/thumbnail-pipeline-gst.h	2013-04-04 22:28:27 +0000
@@ -30,7 +30,8 @@
     void setUri(const QString &uri);
     QString uri() const;
 
-    QImage request(qint64 time);
+    QImage request(qint64 time, QSize size, bool skipBlack=true);
+    static bool isMeaningful(QImage img);
 
 private:
     GstElement *m_pipeline;
@@ -41,7 +42,9 @@
     void setup();
     bool start();
     void stop();
-    QImage parseImage(ThumbnailImageData *buffer);
+
+    QImage parseImage(ThumbnailImageData *buffer) const;
+
 };
 
 #endif

=== modified file 'src/thumbnail-provider.cpp'
--- src/thumbnail-provider.cpp	2013-02-12 14:30:57 +0000
+++ src/thumbnail-provider.cpp	2013-04-04 22:28:27 +0000
@@ -88,15 +88,10 @@
     if (m_cache.contains(time)) {
         img = m_cache[time];
     } else {
-        img = m_player->request(time).copy();
+        img = m_player->request(time, requestedSize).copy();
         m_cache.insert(time, img);
     }
 
-    if (requestedSize.isValid()) {
-        *size = requestedSize;
-        return img.scaled(requestedSize);
-    } else {
-        *size = img.size();
-        return img;
-    }
+    *size = img.size();
+    return img;
 }

=== modified file 'tests/CMakeLists.txt'
--- tests/CMakeLists.txt	2013-03-04 14:17:19 +0000
+++ tests/CMakeLists.txt	2013-04-04 22:28:27 +0000
@@ -1,2 +1,4 @@
+add_subdirectory(videos)
+add_subdirectory(images)
 add_subdirectory(autopilot)
-add_subdirectory(videos)
+add_subdirectory(unittest)

=== modified file 'tests/autopilot/mediaplayer_app/tests/test_player_with_video.py'
--- tests/autopilot/mediaplayer_app/tests/test_player_with_video.py	2013-04-01 22:17:06 +0000
+++ tests/autopilot/mediaplayer_app/tests/test_player_with_video.py	2013-04-04 22:28:27 +0000
@@ -9,16 +9,11 @@
 
 from __future__ import absolute_import
 
-from testtools.matchers import Equals, NotEquals, GreaterThan
+from testtools.matchers import Equals, GreaterThan
 from autopilot.matchers import Eventually
 
 from mediaplayer_app.tests import MediaplayerAppTestCase
 
-import unittest
-import time
-import os
-from os import path
-
 class TestPlayerWithVideo(MediaplayerAppTestCase):
     """Tests the main media player features while playing a video """
 

=== added directory 'tests/images'
=== added file 'tests/images/CMakeLists.txt'
--- tests/images/CMakeLists.txt	1970-01-01 00:00:00 +0000
+++ tests/images/CMakeLists.txt	2013-04-04 22:28:27 +0000
@@ -0,0 +1,1 @@
+project(sample_images)

=== added file 'tests/images/frame0.png'
Binary files tests/images/frame0.png	1970-01-01 00:00:00 +0000 and tests/images/frame0.png	2013-04-04 22:28:27 +0000 differ
=== added file 'tests/images/frame10.png'
Binary files tests/images/frame10.png	1970-01-01 00:00:00 +0000 and tests/images/frame10.png	2013-04-04 22:28:27 +0000 differ
=== added file 'tests/images/frame100.png'
Binary files tests/images/frame100.png	1970-01-01 00:00:00 +0000 and tests/images/frame100.png	2013-04-04 22:28:27 +0000 differ
=== added file 'tests/images/frame20.png'
Binary files tests/images/frame20.png	1970-01-01 00:00:00 +0000 and tests/images/frame20.png	2013-04-04 22:28:27 +0000 differ
=== added directory 'tests/unittest'
=== added file 'tests/unittest/CMakeLists.txt'
--- tests/unittest/CMakeLists.txt	1970-01-01 00:00:00 +0000
+++ tests/unittest/CMakeLists.txt	2013-04-04 22:28:27 +0000
@@ -0,0 +1,30 @@
+add_definitions(-DTEST_SUITE)
+
+if(NOT CTEST_TESTING_TIMEOUT)
+    set(CTEST_TESTING_TIMEOUT 60)
+endif()
+
+include_directories(${CMAKE_BINARY_DIR}
+                    ${mediaplayer_src_SOURCE_DIR}
+                    ${GSTLIB_INCLUDE_DIRS}
+)
+
+configure_file(${CMAKE_CURRENT_SOURCE_DIR}/test-config.h.in
+               ${CMAKE_CURRENT_BINARY_DIR}/test-config.h)
+
+# thumbnail-test ##############################################################
+add_executable(thumbnailtest
+               thumbnailtest.cpp
+               ${mediaplayer_src_SOURCE_DIR}/thumbnail-pipeline-gst.cpp
+)
+
+qt5_use_modules(thumbnailtest Gui Core Test)
+add_test(thumbnailtest thumbnailtest -xunitxml -o test_thumbnailtest.xml)
+set_tests_properties(thumbnailtest PROPERTIES
+                     TIMEOUT ${CTEST_TESTING_TIMEOUT}
+                     ENVIRONMENT "QT_QPA_PLATFORM=minimal")
+target_link_libraries(thumbnailtest 
+                      ${GSTLIB_LDFLAGS})
+###############################################################################
+
+

=== added file 'tests/unittest/test-config.h.in'
--- tests/unittest/test-config.h.in	1970-01-01 00:00:00 +0000
+++ tests/unittest/test-config.h.in	2013-04-04 22:28:27 +0000
@@ -0,0 +1,1 @@
+static const char* SAMPLE_IMAGES_DIR = "@sample_images_SOURCE_DIR@";

=== added file 'tests/unittest/thumbnailtest.cpp'
--- tests/unittest/thumbnailtest.cpp	1970-01-01 00:00:00 +0000
+++ tests/unittest/thumbnailtest.cpp	2013-04-04 22:28:27 +0000
@@ -0,0 +1,57 @@
+/*
+ * Copyright 2013 Canonical Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authors:
+ *      Renato Araujo Oliveira Filho <[e-mail]>
+ */
+
+#include "thumbnail-pipeline-gst.h"
+#include "test-config.h"
+
+#include <QtTest>
+#include <QDebug>
+#include <QImage>
+
+class ThumbnailTest : public QObject
+{
+    Q_OBJECT
+private Q_SLOTS:
+    void testImageIsMeaningful_data()
+    {
+        QTest::addColumn<QString>("path");
+        QTest::addColumn<bool>("expectedResult");
+
+        QTest::newRow("black image") << QString(SAMPLE_IMAGES_DIR) + "/frame0.png" << false;
+        QTest::newRow("non black image") << QString(SAMPLE_IMAGES_DIR) + "/frame10.png" << true;
+        QTest::newRow("collored image") << QString(SAMPLE_IMAGES_DIR) + "/frame20.png" << true;
+        QTest::newRow("almost black image") << QString(SAMPLE_IMAGES_DIR) + "/frame100.png" << false;
+    }
+
+    void testImageIsMeaningful()
+    {
+        QFETCH(QString, path);
+        QFETCH(bool, expectedResult);
+
+        QImage img = QImage(path);
+        QVERIFY(!img.isNull());
+
+        bool result = ThumbnailPipeline::isMeaningful(img);
+        QCOMPARE(result, expectedResult);
+    }
+};
+
+QTEST_MAIN(ThumbnailTest)
+
+#include "thumbnailtest.moc"

=== modified file 'tests/videos/CMakeLists.txt'
--- tests/videos/CMakeLists.txt	2013-03-04 14:17:19 +0000
+++ tests/videos/CMakeLists.txt	2013-04-04 22:28:27 +0000
@@ -1,3 +1,5 @@
+project(sample_videos)
+
 set(MEDIAPLAYER_TEST_VIDEOS small.mp4)
 
 install(FILES ${MEDIAPLAYER_TEST_VIDEOS}