Merge lp:~jpakkane/thumbnailer/embedded_out_of_process into lp:thumbnailer

[Jussi Pakkanen]

Move embedded thumbnail extraction to a helper binary to avoid linking main library with gstreamer.

Move embedded thumbnail extraction to a helper binary to avoid linking main library with gstreamer.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:98
http://jenkins.qa.ubuntu.com/job/thumbnailer-ci/82/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/thumbnailer-utopic-amd64-ci/30
    FAILURE: http://jenkins.qa.ubuntu.com/job/thumbnailer-utopic-armhf-ci/24/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/thumbnailer-utopic-i386-ci/27

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/thumbnailer-ci/82/rebuild

review: Needs Fixing (continuous-integration)

[James Henstridge (jamesh)]

Looks good. One optional optimisation mentioned in inline comments, but overall this looks good.

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:98
http://jenkins.qa.ubuntu.com/job/thumbnailer-ci/83/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/thumbnailer-utopic-amd64-ci/31/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/thumbnailer-utopic-armhf-ci/25
        deb: http://jenkins.qa.ubuntu.com/job/thumbnailer-utopic-armhf-ci/25/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/thumbnailer-utopic-i386-ci/28

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/thumbnailer-ci/83/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:98
http://jenkins.qa.ubuntu.com/job/thumbnailer-ci/84/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/thumbnailer-utopic-amd64-ci/32/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/thumbnailer-utopic-armhf-ci/26/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/thumbnailer-utopic-i386-ci/29

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/thumbnailer-ci/84/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:98
http://jenkins.qa.ubuntu.com/job/thumbnailer-ci/85/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/thumbnailer-utopic-amd64-ci/33
    FAILURE: http://jenkins.qa.ubuntu.com/job/thumbnailer-utopic-armhf-ci/27/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/thumbnailer-utopic-i386-ci/30

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/thumbnailer-ci/85/rebuild

review: Needs Fixing (continuous-integration)

[lp:~jpakkane/thumbnailer/embedded_out_of_process]

99. By Jussi Pakkanen on 2014-09-03

Bring in changelog entries from manual revert uploads.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:99
http://jenkins.qa.ubuntu.com/job/thumbnailer-ci/86/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/thumbnailer-utopic-amd64-ci/34
    FAILURE: http://jenkins.qa.ubuntu.com/job/thumbnailer-utopic-armhf-ci/28/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/thumbnailer-utopic-i386-ci/31

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/thumbnailer-ci/86/rebuild

review: Needs Fixing (continuous-integration)

=== modified file 'debian/changelog'
--- debian/changelog	2014-09-01 07:32:23 +0000
+++ debian/changelog	2014-09-03 11:24:10 +0000
@@ -1,3 +1,18 @@
+thumbnailer (1.2+14.10.20140901.is.1.2+14.10.20140814-0ubuntu2) utopic; urgency=medium
+
+  * Reintroduce the pthread linkage fix from 1.2+14.10.20140827.1-0ubuntu1,
+    required for the package to build.
+
+ -- Steve Langasek <[e-mail]>  Tue, 02 Sep 2014 13:26:28 -0700
+
+thumbnailer (1.2+14.10.20140901.is.1.2+14.10.20140814-0ubuntu1) utopic; urgency=medium
+
+  * Revert the last two landings. The 1.2+14.10.20140827.1 version
+    introduced a serious regression breaking many projects, indirectly
+    caused by thumbnailer using gst.  LP: #1363314.
+
+ -- Łukasz 'sil2100' Zemczak <[e-mail]>  Tue, 02 Sep 2014 21:50:08 +0200
+
 thumbnailer (1.2+14.10.20140901-0ubuntu1) utopic; urgency=medium
 
   [ Pawel Stolowski ]

=== modified file 'src/CMakeLists.txt'
--- src/CMakeLists.txt	2014-08-18 14:46:38 +0000
+++ src/CMakeLists.txt	2014-09-03 11:24:10 +0000
@@ -13,7 +13,6 @@
 set(symbol_map "${CMAKE_CURRENT_SOURCE_DIR}/libthumbnailer.map")
 
 target_link_libraries(thumbnailer
- ${GST_DEPS_LDFLAGS}
  ${GLIB_DEPS_LDFLAGS}
  ${GIO_DEPS_LDFLAGS}
  ${IMG_DEPS_LDFLAGS}
@@ -32,7 +31,7 @@
 )
 
 add_executable(vs-thumb vs-thumb.cpp)
-target_link_libraries(vs-thumb ${GST_DEPS_LDFLAGS} ${IMG_DEPS_LDFLAGS})
+target_link_libraries(vs-thumb ${GST_DEPS_LDFLAGS} ${IMG_DEPS_LDFLAGS} ${GIO_DEPS_LDFLAGS})
 install(
   TARGETS vs-thumb
   ARCHIVE DESTINATION ${SHARE_PRIV_DIR}

=== modified file 'src/audioimageextractor.cpp'
--- src/audioimageextractor.cpp	2013-10-08 08:13:11 +0000
+++ src/audioimageextractor.cpp	2014-09-03 11:24:10 +0000
@@ -21,39 +21,30 @@
 #include<unistd.h>
 #include<stdexcept>
 #include<memory>
-#include <gst/pbutils/pbutils.h>
+#include"internal/videoscreenshotter.h"
 
 using namespace std;
 
-class GstInitializer {
+class AudioImageExtractorPrivate {
+
 public:
-    GstInitializer() { gst_init(nullptr, nullptr); };
-};
-
-class AudioImageExtractorPrivate {
-};
-
-static bool has_tag(const GstTagList *tlist, const gchar *tagname) {
-  const GValue *val = gst_tag_list_get_value_index(tlist, tagname, 0);
-  return val;
-}
-
-static void extract_image(const GstTagList *tlist, FILE *outf) {
-  const GValue *val = gst_tag_list_get_value_index(tlist, GST_TAG_IMAGE, 0);
-  GstSample *sample = (GstSample*)g_value_get_boxed(val);
-  GstBuffer *buf = gst_sample_get_buffer(sample);
-
-  for(guint i=0; i<gst_buffer_n_memory(buf); i++) {
-    GstMemory *mem = gst_buffer_peek_memory(buf, i);
-    GstMapInfo mi;
-    gst_memory_map(mem, &mi, GST_MAP_READ);
-    fwrite(mi.data, 1, mi.size, outf);
-    gst_memory_unmap(mem, &mi);
-  }
+
+    bool extract(const string &ifname, const string &ofname);
+
+private:
+    // Use the external binary to extract the embedded image.
+    // We could use the vs object in the Thumbnailer class and remove
+    // AudioImageExtractor completely but it is possible that
+    // this is refactored to use a dbus service for efficiency
+    // so having this class around will make that refactoring easier.
+    VideoScreenshotter vs;
+};
+
+bool AudioImageExtractorPrivate::extract(const string &ifname, const string &ofname) {
+    return vs.extract(ifname, ofname);
 }
 
 AudioImageExtractor::AudioImageExtractor() {
-    static GstInitializer i; // C++ standard guarantees this to be lazy and thread safe.
     p = new AudioImageExtractorPrivate();
 }
 
@@ -63,38 +54,5 @@
 
 
 bool AudioImageExtractor::extract(const string &ifname, const string &ofname) {
-    string uri("file://");
-    if(ifname[0] != '/') {
-        uri += getcwd(nullptr, 0);
-        uri += '/';
-    }
-    uri += ifname;
-    GError *err = nullptr;
-    unique_ptr<GstDiscoverer, void(*)(GstDiscoverer*)> dsc(
-            gst_discoverer_new(GST_SECOND, &err),
-            [](GstDiscoverer *t) {g_object_unref(G_OBJECT(t));});
-    if(err) {
-        string msg(err->message);
-        g_error_free(err);
-        throw runtime_error(msg);
-    }
-    unique_ptr<GstDiscovererInfo, void(*)(GstDiscovererInfo*)> info(
-            gst_discoverer_discover_uri(dsc.get(), uri.c_str(), &err),
-            [](GstDiscovererInfo *t) {g_object_unref(G_OBJECT(t));});
-    if(err) {
-        string msg(err->message);
-        g_error_free(err);
-        throw runtime_error(msg);
-    }
-    const GstTagList *tlist = gst_discoverer_info_get_tags(info.get());
-    if(!tlist) {
-        return false;
-    }
-    if(!has_tag(tlist, GST_TAG_IMAGE)) {
-        return false;
-    }
-    FILE *outfile = fopen(ofname.c_str(), "wb");
-    extract_image(tlist, outfile);
-    fclose(outfile);
-    return true;
+    return p->extract(ifname, ofname);
 }

=== modified file 'src/vs-thumb.cpp'
--- src/vs-thumb.cpp	2014-08-14 11:26:33 +0000
+++ src/vs-thumb.cpp	2014-09-03 11:24:10 +0000
@@ -23,10 +23,15 @@
 #include<gst/gst.h>
 #include<gst/app/gstappsink.h>
 #include<gdk-pixbuf/gdk-pixbuf.h>
+#include<gio/gio.h>
+#include<glib.h>
+#include<gst/pbutils/pbutils.h>
 
 using namespace std;
 
-bool extract(const std::string &ifname, const std::string &ofname) {
+namespace {
+
+bool extract_video(const std::string &ifname, const std::string &ofname) {
     string caps_string = "video/x-raw,format=RGB,pixel-aspect-ratio=1/1";
     GstElement *sink;
     gint64 duration, seek_point;
@@ -106,6 +111,64 @@
     return true;
 }
 
+bool has_tag(const GstTagList *tlist, const gchar *tagname) {
+  const GValue *val = gst_tag_list_get_value_index(tlist, tagname, 0);
+  return val;
+}
+
+void extract_embedded_image(const GstTagList *tlist, FILE *outf) {
+  const GValue *val = gst_tag_list_get_value_index(tlist, GST_TAG_IMAGE, 0);
+  GstSample *sample = (GstSample*)g_value_get_boxed(val);
+  GstBuffer *buf = gst_sample_get_buffer(sample);
+
+  for(guint i=0; i<gst_buffer_n_memory(buf); i++) {
+    GstMemory *mem = gst_buffer_peek_memory(buf, i);
+    GstMapInfo mi;
+    gst_memory_map(mem, &mi, GST_MAP_READ);
+    fwrite(mi.data, 1, mi.size, outf);
+    gst_memory_unmap(mem, &mi);
+  }
+}
+
+bool extract_audio(const std::string &ifname, const std::string &ofname) {
+    string uri("file://");
+    if(ifname[0] != '/') {
+        uri += getcwd(nullptr, 0);
+        uri += '/';
+    }
+    uri += ifname;
+    GError *err = nullptr;
+    unique_ptr<GstDiscoverer, void(*)(GstDiscoverer*)> dsc(
+            gst_discoverer_new(GST_SECOND, &err),
+            [](GstDiscoverer *t) {g_object_unref(G_OBJECT(t));});
+    if(err) {
+        string msg(err->message);
+        g_error_free(err);
+        throw runtime_error(msg);
+    }
+    unique_ptr<GstDiscovererInfo, void(*)(GstDiscovererInfo*)> info(
+            gst_discoverer_discover_uri(dsc.get(), uri.c_str(), &err),
+            [](GstDiscovererInfo *t) {g_object_unref(G_OBJECT(t));});
+    if(err) {
+        string msg(err->message);
+        g_error_free(err);
+        throw runtime_error(msg);
+    }
+    const GstTagList *tlist = gst_discoverer_info_get_tags(info.get());
+    if(!tlist) {
+        return false;
+    }
+    if(!has_tag(tlist, GST_TAG_IMAGE)) {
+        return false;
+    }
+    FILE *outfile = fopen(ofname.c_str(), "wb");
+    extract_embedded_image(tlist, outfile);
+    fclose(outfile);
+    return true;
+}
+
+}
+
 int main(int argc, char **argv) {
     gst_init(&argc, &argv);
     if(argc != 3) {
@@ -114,13 +177,43 @@
     }
     string infile(argv[1]);
     string outfile(argv[2]);
-    bool success;
-    try {
-        success = extract(infile, outfile);
-    } catch(runtime_error &e) {
-        printf("Error creating thumbnail: %s\n", e.what());
-        return 2;
-    }
+    bool success = false;
+
+    // Determine file type.
+    std::unique_ptr<GFile, void(*)(void *)> file(
+            g_file_new_for_path(infile.c_str()), g_object_unref);
+    if(!file) {
+        return 1;
+    }
+
+    std::unique_ptr<GFileInfo, void(*)(void *)> info(
+            g_file_query_info(file.get(), G_FILE_ATTRIBUTE_STANDARD_CONTENT_TYPE,
+                G_FILE_QUERY_INFO_NONE, /* cancellable */ NULL, /* error */NULL),
+            g_object_unref);
+    if(!info) {
+        return 1;
+    }
+
+    std::string content_type(g_file_info_get_content_type(info.get()));
+
+    if (content_type.find("audio/") == 0) {
+        try {
+            success = extract_audio(infile, outfile);
+        } catch(runtime_error &e) {
+            printf("Error creating thumbnail: %s\n", e.what());
+            return 2;
+        }
+    }
+
+    if (content_type.find("video/") == 0) {
+        try {
+            success = extract_video(infile, outfile);
+        } catch(runtime_error &e) {
+            printf("Error creating thumbnail: %s\n", e.what());
+            return 2;
+        }
+    }
+
     if(success) {
         return 0;
     }