Merge lp:~jamesh/thumbnailer/vs-thumb-async into lp:thumbnailer/devel

[James Henstridge]

Convert the ThumbnailExtractor class to run the extraction within the event loop.

Convert the ThumbnailExtractor class to run the extraction within the event loop. This will allow us to convert vs-thumb to a D-Bus service that can manage multiple extractions simultaneously.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:258
http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-ci/405/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-wily-amd64-ci/215/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-wily-armhf-ci/220/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-wily-i386-ci/214/console

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

review: Needs Fixing (continuous-integration)

[Michi Henning (michihenning)]

The thumbnailer test gets one failure here (deterministically):

[ RUN ] ThumbnailerTest.empty_file
Changing to state PAUSED
State changed to 2
/home/michi/src/thumbnailer/vs-thumb-async/tests/thumbnailer/thumbnailer_test.cpp:646: Failure
Value of: spy.wait(5000)
  Actual: false
Expected: true
[ FAILED ] ThumbnailerTest.empty_file (6119 ms)

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:258
http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-ci/406/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-wily-amd64-ci/216/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-wily-armhf-ci/221/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-wily-i386-ci/215/console

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

review: Needs Fixing (continuous-integration)

[lp:~jamesh/thumbnailer/vs-thumb-async]

259. By James Henstridge on 2015-08-17

Bail out on errors from any element rather than just the playbin.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:259
http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-ci/407/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-wily-amd64-ci/217/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-wily-armhf-ci/222/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-wily-i386-ci/216/console

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

review: Needs Fixing (continuous-integration)

[lp:~jamesh/thumbnailer/vs-thumb-async]

260. By James Henstridge on 2015-08-18

Fix test for error message on empty files.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:260
http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-ci/409/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-wily-amd64-ci/219
    SUCCESS: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-wily-armhf-ci/224
        deb: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-wily-armhf-ci/224/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-wily-i386-ci/218

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

review: Approve (continuous-integration)

[Michi Henning (michihenning)]

I'm getting this when building with vivid:

[ 18%] Building CXX object src/vs-thumb/CMakeFiles/vs-thumb.dir/thumbnailextractor.cpp.o
/home/michi/src/thumbnailer/vs-thumb-async/src/vs-thumb/thumbnailextractor.cpp: In destructor ‘unity::thumbnailer::internal::ThumbnailExtractor::~ThumbnailExtractor()’:
/home/michi/src/thumbnailer/vs-thumb-async/src/vs-thumb/thumbnailextractor.cpp:165:40: error: ‘gst_bus_remove_watch’ was not declared in this scope
         gst_bus_remove_watch(bus_.get());
                                        ^
src/vs-thumb/CMakeFiles/vs-thumb.dir/build.make:77: recipe for target 'src/vs-thumb/CMakeFiles/vs-thumb.dir/thumbnailextractor.cpp.o' failed

Some change in GStreamer version?

[lp:~jamesh/thumbnailer/vs-thumb-async]

261. By James Henstridge on 2015-08-19

gst_bus_remove_watch() doesn't exist in GStreamer 1.4, so use
g_source_remove instead.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:261
http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-ci/410/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-wily-amd64-ci/220
    SUCCESS: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-wily-armhf-ci/225
        deb: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-wily-armhf-ci/225/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-wily-i386-ci/219

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

review: Approve (continuous-integration)

[Michi Henning (michihenning)]

Looks good to me, thanks! Eventually, we'll have to get the test coverage up. But, seeing that we'll probably make a few more changes to vs-thumb, it's probably premature to do this now.

review: Approve

=== modified file 'src/vs-thumb/thumbnailextractor.cpp'
--- src/vs-thumb/thumbnailextractor.cpp	2015-06-22 23:50:46 +0000
+++ src/vs-thumb/thumbnailextractor.cpp	2015-08-19 03:01:01 +0000
@@ -28,8 +28,6 @@
 #pragma GCC diagnostic ignored "-Wcast-qual"
 #pragma GCC diagnostic ignored "-Wcast-align"
 
-#include <gdk-pixbuf/gdk-pixbuf.h>
-#include <gst/gst.h>
 #include <gst/tag/tag.h>
 
 #include <internal/gobj_memory.h>
@@ -60,59 +58,6 @@
     throw std::runtime_error(message);
 }
 
-void change_state(GstElement* element, GstState state)
-{
-    GstStateChangeReturn ret = gst_element_set_state(element, state);
-    switch (ret)
-    {
-        case GST_STATE_CHANGE_NO_PREROLL:
-        case GST_STATE_CHANGE_SUCCESS:
-            return;
-        case GST_STATE_CHANGE_ASYNC:
-            // The change is happening in a background thread, which we
-            // will wait on below.
-            break;
-        case GST_STATE_CHANGE_FAILURE:
-        default:
-            throw_error("change_state(): Could not change element state");
-    }
-
-    // We're in the async case here, so pop messages off the bus until
-    // it is done.
-    gobj_ptr<GstBus> bus(gst_element_get_bus(element));
-    while (true)
-    {
-        std::unique_ptr<GstMessage, decltype(&gst_message_unref)> message(
-            gst_bus_timed_pop_filtered(bus.get(), GST_CLOCK_TIME_NONE,
-                                       static_cast<GstMessageType>(GST_MESSAGE_ASYNC_DONE | GST_MESSAGE_ERROR)),
-            gst_message_unref);
-        if (!message)
-        {
-            break;
-        }
-
-        switch (GST_MESSAGE_TYPE(message.get()))
-        {
-            case GST_MESSAGE_ASYNC_DONE:
-                if (GST_MESSAGE_SRC(message.get()) == GST_OBJECT(element))
-                {
-                    return;
-                }
-                break;
-            case GST_MESSAGE_ERROR:
-            {
-                GError* error = nullptr;
-                gst_message_parse_error(message.get(), &error, nullptr);
-                throw_error("change_state(): reading async messages", error);
-                break;
-            }
-            default:
-                /* ignore other message types */
-                ;
-        }
-    }
-}
-
 class BufferMap final
 {
 public:
@@ -161,16 +106,6 @@
 
 }  // namespace
 
-struct ThumbnailExtractor::Private
-{
-    gobj_ptr<GstElement> playbin;
-    gint64 duration = -1;
-
-    std::unique_ptr<GstSample, decltype(&gst_sample_unref)> sample{nullptr, gst_sample_unref};
-    GdkPixbufRotation sample_rotation = GDK_PIXBUF_ROTATE_NONE;
-    bool sample_raw = true;
-};
-
 /* GstPlayFlags flags from playbin.
  *
  * GStreamer does not install headers for the enums of individual
@@ -192,14 +127,13 @@
 } GstPlayFlags;
 
 ThumbnailExtractor::ThumbnailExtractor()
-    : p(new Private)
 {
     GstElement* pb = gst_element_factory_make("playbin", "playbin");
     if (!pb)
     {
         throw_error("ThumbnailExtractor(): Could not create playbin");
     }
-    p->playbin.reset(static_cast<GstElement*>(g_object_ref_sink(pb)));
+    playbin_.reset(static_cast<GstElement*>(g_object_ref_sink(pb)));
 
     GstElement* audio_sink = gst_element_factory_make("fakesink", "audio-fake-sink");
     if (!audio_sink)
@@ -214,57 +148,170 @@
     }
 
     g_object_set(video_sink, "sync", TRUE, nullptr);
-    g_object_set(p->playbin.get(), "audio-sink", audio_sink, "video-sink", video_sink, "flags",
-                 GST_PLAY_FLAG_VIDEO | GST_PLAY_FLAG_AUDIO, nullptr);
+    g_object_set(playbin_.get(),
+                 "audio-sink", audio_sink,
+                 "video-sink", video_sink,
+                 "flags", GST_PLAY_FLAG_VIDEO | GST_PLAY_FLAG_AUDIO,
+                 nullptr);
+
+    bus_.reset(gst_element_get_bus(pb));
+    bus_watch_id_ = gst_bus_add_watch(
+        bus_.get(), &ThumbnailExtractor::on_new_message, this);
 }
 
 ThumbnailExtractor::~ThumbnailExtractor()
 {
+    if (bus_watch_id_ != 0)
+    {
+        g_source_remove(bus_watch_id_);
+    }
+    if (playbin_)
+    {
+        reset();
+    }
+}
+
+void ThumbnailExtractor::extract(std::string const& uri, std::function<void(GdkPixbuf* const)> callback)
+{
+    callback_ = callback;
+    set_uri(uri);
+}
+
+void ThumbnailExtractor::finished(GdkPixbuf* const thumbnail)
+{
+    if (callback_)
+    {
+        callback_(thumbnail);
+        callback_ = nullptr;
+    }
     reset();
 }
 
 void ThumbnailExtractor::reset()
 {
-    change_state(p->playbin.get(), GST_STATE_NULL);
-    p->sample.reset();
-    p->sample_rotation = GDK_PIXBUF_ROTATE_NONE;
-    p->sample_raw = true;
+    gst_element_set_state(playbin_.get(), GST_STATE_NULL);
+    is_seeking_ = false;
+}
+
+gboolean ThumbnailExtractor::on_new_message(GstBus */*bus*/, GstMessage *message, void *user_data)
+{
+    auto extractor = reinterpret_cast<ThumbnailExtractor*>(user_data);
+
+    switch (GST_MESSAGE_TYPE(message))
+    {
+    case GST_MESSAGE_STATE_CHANGED:
+        if (GST_MESSAGE_SRC(message) != GST_OBJECT(extractor->playbin_.get()))
+        {
+            break;
+        }
+        GstState newstate;
+        gst_message_parse_state_changed(message, nullptr, &newstate, nullptr);
+        extractor->state_changed(newstate);
+        break;
+    case GST_MESSAGE_ASYNC_DONE:
+        if (GST_MESSAGE_SRC(message) != GST_OBJECT(extractor->playbin_.get()))
+        {
+            break;
+        }
+        fprintf(stderr, "Got Async done message, is_seeking == %d\n", extractor->is_seeking_);
+        if (extractor->is_seeking_)
+        {
+            // Check to make sure the state change after the seek has
+            // truely completed.
+            if (gst_element_get_state(
+                    extractor->playbin_.get(),
+                    nullptr, nullptr, 0) != GST_STATE_CHANGE_ASYNC)
+            {
+                extractor->is_seeking_ = false;
+                extractor->seek_done();
+            }
+        }
+        break;
+    case GST_MESSAGE_ERROR:
+    {
+        GError* error = nullptr;
+        gst_message_parse_error(message, &error, nullptr);
+        extractor->bus_error(error);
+        g_error_free(error);
+        break;
+    }
+    default:
+        /* ignore other message types */
+        ;
+    }
+
+    return G_SOURCE_CONTINUE;
 }
 
 void ThumbnailExtractor::set_uri(const std::string& uri)
 {
     reset();
-    g_object_set(p->playbin.get(), "uri", uri.c_str(), nullptr);
+    g_object_set(playbin_.get(), "uri", uri.c_str(), nullptr);
+
     fprintf(stderr, "Changing to state PAUSED\n");
-    change_state(p->playbin.get(), GST_STATE_PAUSED);
-
-    if (!gst_element_query_duration(p->playbin.get(), GST_FORMAT_TIME, &p->duration))
+    auto ret = gst_element_set_state(playbin_.get(), GST_STATE_PAUSED);
+    if (ret == GST_STATE_CHANGE_FAILURE)
     {
-        p->duration = -1;
-    }
+        throw_error("set_uri(): Could not change pipeline state to paused.");
+    }
+}
+
+void ThumbnailExtractor::state_changed(GstState state)
+{
+    fprintf(stderr, "State changed to %d\n", (int)state);
+    switch (state) {
+    case GST_STATE_PAUSED:
+        if (has_video())
+        {
+            if (!is_seeking_)
+            {
+                // Seek somewhere that will hopefully give a relevant image.
+                int64_t duration, seek_point;
+                if (!gst_element_query_duration(playbin_.get(), GST_FORMAT_TIME, &duration))
+                {
+                    seek_point = 2 * duration / 7;
+                }
+                else
+                {
+                    seek_point = 10 * GST_SECOND;
+                }
+                fprintf(stderr, "Seeking to position %d\n", (int)(seek_point / GST_SECOND));
+                is_seeking_ = true;
+                gst_element_seek_simple(playbin_.get(), GST_FORMAT_TIME,
+                                        static_cast<GstSeekFlags>(GST_SEEK_FLAG_FLUSH | GST_SEEK_FLAG_KEY_UNIT), seek_point);
+            }
+        }
+        else
+        {
+            extract_audio_cover_art();
+        }
+        break;
+    default:
+        break;
+    }
+}
+
+void ThumbnailExtractor::seek_done()
+{
+    extract_video_frame();
+}
+
+void ThumbnailExtractor::bus_error(GError *error)
+{
+    fprintf(stderr, "Error: %s\n", error->message);
+    finished(nullptr);
 }
 
 bool ThumbnailExtractor::has_video()
 {
     int n_video = 0;
-    g_object_get(p->playbin.get(), "n-video", &n_video, nullptr);
+    g_object_get(playbin_.get(), "n-video", &n_video, nullptr);
+    fprintf(stderr, "Number of video streams == %d\n", n_video);
     return n_video > 0;
 }
 
-bool ThumbnailExtractor::extract_video_frame()
+void ThumbnailExtractor::extract_video_frame()
 {
-    gint64 seek_point = 10 * GST_SECOND;
-    if (p->duration >= 0)
-    {
-        seek_point = 2 * p->duration / 7;
-    }
-    fprintf(stderr, "Seeking to position %d\n", (int)(seek_point / GST_SECOND));
-    gst_element_seek_simple(p->playbin.get(), GST_FORMAT_TIME,
-                            static_cast<GstSeekFlags>(GST_SEEK_FLAG_FLUSH | GST_SEEK_FLAG_KEY_UNIT), seek_point);
-    fprintf(stderr, "Waiting for seek to complete\n");
-    gst_element_get_state(p->playbin.get(), nullptr, nullptr, GST_CLOCK_TIME_NONE);
-    fprintf(stderr, "Done.\n");
-
     // Retrieve sample from the playbin
     fprintf(stderr, "Requesting sample frame.\n");
     std::unique_ptr<GstCaps, decltype(&gst_caps_unref)> desired_caps(
@@ -272,18 +319,20 @@
                             1, nullptr),
         gst_caps_unref);
     GstSample* s;
-    g_signal_emit_by_name(p->playbin.get(), "convert-sample", desired_caps.get(), &s);
+    g_signal_emit_by_name(playbin_.get(), "convert-sample", desired_caps.get(), &s);
     if (!s)
     {
-        return false;
+        fprintf(stderr, "Could not extract sample\n");
+        finished(nullptr);
+        return;
     }
-    p->sample.reset(s);
-    p->sample_raw = true;
+    std::unique_ptr<GstSample, decltype(&gst_sample_unref)> sample(
+        s, gst_sample_unref);
 
     // Does the sample need to be rotated?
-    p->sample_rotation = GDK_PIXBUF_ROTATE_NONE;
+    GdkPixbufRotation rotation = GDK_PIXBUF_ROTATE_NONE;
     GstTagList* tags = nullptr;
-    g_signal_emit_by_name(p->playbin.get(), "get-video-tags", 0, &tags);
+    g_signal_emit_by_name(playbin_.get(), "get-video-tags", 0, &tags);
     if (tags)
     {
         char* orientation = nullptr;
@@ -291,80 +340,90 @@
         {
             if (!strcmp(orientation, "rotate-90"))
             {
-                p->sample_rotation = GDK_PIXBUF_ROTATE_CLOCKWISE;
+                rotation = GDK_PIXBUF_ROTATE_CLOCKWISE;
             }
             else if (!strcmp(orientation, "rotate-180"))
             {
-                p->sample_rotation = GDK_PIXBUF_ROTATE_UPSIDEDOWN;
+                rotation = GDK_PIXBUF_ROTATE_UPSIDEDOWN;
             }
             else if (!strcmp(orientation, "rotate-270"))
             {
-                p->sample_rotation = GDK_PIXBUF_ROTATE_COUNTERCLOCKWISE;
+                rotation = GDK_PIXBUF_ROTATE_COUNTERCLOCKWISE;
             }
         }
         gst_tag_list_unref(tags);
     }
-    return true;
+    save_screenshot(sample.get(), rotation, true);
 }
 
-bool ThumbnailExtractor::extract_audio_cover_art()
+void ThumbnailExtractor::extract_audio_cover_art()
 {
+    fprintf(stderr, "Extracting cover art\n");
     GstTagList* tags = nullptr;
-    g_signal_emit_by_name(p->playbin.get(), "get-audio-tags", 0, &tags);
+    g_signal_emit_by_name(playbin_.get(), "get-audio-tags", 0, &tags);
     if (!tags)
     {
-        return false;
+        finished(nullptr);
+        return;
     }
-    p->sample.reset();
-    p->sample_rotation = GDK_PIXBUF_ROTATE_NONE;
-    p->sample_raw = false;
+    std::unique_ptr<GstSample, decltype(&gst_sample_unref)> sample(
+        nullptr, gst_sample_unref);
     bool found_cover = false;
     for (int i = 0; !found_cover; i++)
     {
-        GstSample* sample;
-        if (!gst_tag_list_get_sample_index(tags, GST_TAG_IMAGE, i, &sample))
+        GstSample* s;
+        if (!gst_tag_list_get_sample_index(tags, GST_TAG_IMAGE, i, &s))
         {
             break;
         }
         // Check the type of this image
-        auto caps = gst_sample_get_caps(sample);
+        auto caps = gst_sample_get_caps(s);
         auto structure = gst_caps_get_structure(caps, 0);
         int type = GST_TAG_IMAGE_TYPE_UNDEFINED;
         gst_structure_get_enum(structure, "image-type", GST_TYPE_TAG_IMAGE_TYPE, &type);
         if (type == GST_TAG_IMAGE_TYPE_FRONT_COVER)
         {
-            p->sample.reset(gst_sample_ref(sample));
+            sample.reset(gst_sample_ref(s));
             found_cover = true;
         }
-        else if (type == GST_TAG_IMAGE_TYPE_UNDEFINED && !p->sample)
+        else if (type == GST_TAG_IMAGE_TYPE_UNDEFINED && !sample)
         {
             // Save the first unknown image tag, but continue scanning
             // in case there is one marked as the cover.
-            p->sample.reset(gst_sample_ref(sample));
+            sample.reset(gst_sample_ref(s));
         }
-        gst_sample_unref(sample);
+        gst_sample_unref(s);
     }
     gst_tag_list_unref(tags);
-    return bool(p->sample);
+    if (!sample)
+    {
+        finished(nullptr);
+        return;
+    }
+    save_screenshot(sample.get(), GDK_PIXBUF_ROTATE_NONE, false);
 }
 
-void ThumbnailExtractor::save_screenshot(const std::string& filename)
+void ThumbnailExtractor::save_screenshot(GstSample* sample, GdkPixbufRotation rotation, bool raw)
 {
-    if (!p->sample)
+    if (!sample)
     {
-        throw_error("save_screenshot(): Could not retrieve screenshot");
+        fprintf(stderr, "save_screenshot(): Could not retrieve screenshot\n");
+        finished(nullptr);
+        return;
     }
 
     // Construct a pixbuf from the sample
     fprintf(stderr, "Creating pixbuf from sample\n");
     BufferMap buffermap;
     gobj_ptr<GdkPixbuf> image;
-    if (p->sample_raw)
+    if (raw)
     {
-        GstCaps* sample_caps = gst_sample_get_caps(p->sample.get());
+        GstCaps* sample_caps = gst_sample_get_caps(sample);
         if (!sample_caps)
         {
-            throw_error("save_screenshot(): Could not retrieve caps for sample buffer");
+            fprintf(stderr, "save_screenshot(): Could not retrieve caps for sample buffer\n");
+            finished(nullptr);
+            return;
         }
         GstStructure* sample_struct = gst_caps_get_structure(sample_caps, 0);
         int width = 0, height = 0;
@@ -372,10 +431,12 @@
         gst_structure_get_int(sample_struct, "height", &height);
         if (width <= 0 || height <= 0)
         {
-            throw_error("save_screenshot(): Could not retrieve image dimensions");
+            fprintf(stderr, "save_screenshot(): Could not retrieve image dimensions\n");
+            finished(nullptr);
+            return;
         }
 
-        buffermap.map(gst_sample_get_buffer(p->sample.get()));
+        buffermap.map(gst_sample_get_buffer(sample));
         image.reset(gdk_pixbuf_new_from_data(buffermap.data(), GDK_COLORSPACE_RGB, FALSE, 8, width, height,
                                              GST_ROUND_UP_4(width * 3), nullptr, nullptr));
     }
@@ -383,7 +444,7 @@
     {
         gobj_ptr<GdkPixbufLoader> loader(gdk_pixbuf_loader_new());
 
-        buffermap.map(gst_sample_get_buffer(p->sample.get()));
+        buffermap.map(gst_sample_get_buffer(sample));
         GError* error = nullptr;
         if (gdk_pixbuf_loader_write(loader.get(), buffermap.data(), buffermap.size(), &error) &&
             gdk_pixbuf_loader_close(loader.get(), &error))
@@ -396,29 +457,23 @@
         }
         else
         {
-            throw_error("save_screenshot(): decoding image", error);
+            fprintf(stderr, "save_screenshot(): decoding image: %s\n", error->message);
+            g_error_free(error);
+            finished(nullptr);
+            return;
         }
     }
 
-    if (p->sample_rotation != GDK_PIXBUF_ROTATE_NONE)
+    if (rotation != GDK_PIXBUF_ROTATE_NONE)
     {
-        GdkPixbuf* rotated = gdk_pixbuf_rotate_simple(image.get(), p->sample_rotation);
+        GdkPixbuf* rotated = gdk_pixbuf_rotate_simple(image.get(), rotation);
         if (rotated)
         {
             image.reset(rotated);
         }
     }
 
-    // Saving as TIFF with no compression here to avoid artefacts due to converting to jpg twice.
-    // (The main thumbnailer saves as jpg.) By staying lossless here, we
-    // keep all the policy decisions about image quality in the main thumbnailer.
-    fprintf(stderr, "Saving pixbuf to tiff\n");
-    GError* error = nullptr;
-    if (!gdk_pixbuf_save(image.get(), filename.c_str(), "tiff", &error, "compression", "1", nullptr))
-    {
-        throw_error("save_screenshot(): saving image", error);
-    }
-    fprintf(stderr, "Done.\n");
+    finished(image.get());
 }
 
 }  // namespace internal

=== modified file 'src/vs-thumb/thumbnailextractor.h'
--- src/vs-thumb/thumbnailextractor.h	2015-05-26 23:55:06 +0000
+++ src/vs-thumb/thumbnailextractor.h	2015-08-19 03:01:01 +0000
@@ -18,6 +18,17 @@
 
 #pragma once
 
+#include <internal/gobj_memory.h>
+
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wold-style-cast"
+#pragma GCC diagnostic ignored "-Wcast-qual"
+#pragma GCC diagnostic ignored "-Wcast-align"
+#include <gdk-pixbuf/gdk-pixbuf.h>
+#include <gst/gst.h>
+#pragma GCC diagnostic pop
+
+#include <functional>
 #include <memory>
 #include <string>
 
@@ -32,21 +43,32 @@
 
 class ThumbnailExtractor final
 {
-    struct Private;
-
 public:
     ThumbnailExtractor();
     ~ThumbnailExtractor();
 
+    void extract(std::string const& uri, std::function<void(GdkPixbuf* const)> callback);
     void reset();
+
+private:
+    gobj_ptr<GstElement> playbin_;
+    gobj_ptr<GstBus> bus_;
+    unsigned int bus_watch_id_ = 0;
+    bool is_seeking_ = false;
+
+    std::function<void(GdkPixbuf* const)> callback_;
+
+    void finished(GdkPixbuf* const thumbnail);
     void set_uri(const std::string& uri);
     bool has_video();
-    bool extract_video_frame();
-    bool extract_audio_cover_art();
-    void save_screenshot(const std::string& filename);
+    void extract_video_frame();
+    void extract_audio_cover_art();
+    void save_screenshot(GstSample* sample, GdkPixbufRotation rotation, bool raw);
 
-private:
-    std::unique_ptr<Private> p;
+    static gboolean on_new_message(GstBus *bus, GstMessage *message, void *user_data);
+    void state_changed(GstState state);
+    void bus_error(GError *error);
+    void seek_done();
 };
 
 }  // namespace internal

=== modified file 'src/vs-thumb/vs-thumb.cpp'
--- src/vs-thumb/vs-thumb.cpp	2015-05-28 22:16:02 +0000
+++ src/vs-thumb/vs-thumb.cpp	2015-08-19 03:01:01 +0000
@@ -54,28 +54,6 @@
     return uri;
 }
 
-bool extract_thumbnail(string const& uri, string const& ofname)
-{
-    ThumbnailExtractor extractor;
-
-    extractor.set_uri(uri);
-    if (extractor.has_video())
-    {
-        if (!extractor.extract_video_frame())
-        {
-            return false;
-        }
-    }
-    else
-    {
-        if (!extractor.extract_audio_cover_art())
-        {
-            return false;
-        }
-    }
-    extractor.save_screenshot(ofname);
-    return true;
-}
 }
 
 int main(int argc, char** argv)
@@ -88,7 +66,6 @@
     }
     string uri;
     string outfile(argv[2]);
-    bool success = false;
 
     try
     {
@@ -100,15 +77,39 @@
         return 1;
     }
 
+    std::unique_ptr<GMainLoop, decltype(&g_main_loop_unref)> main_loop(
+        g_main_loop_new(nullptr, false), g_main_loop_unref);
+    bool success = false;
+    auto callback = [&](GdkPixbuf* const thumbnail) {
+        if (thumbnail)
+        {
+            // Saving as TIFF with no compression here to avoid
+            // artefacts due to converting to jpg twice.
+            // (The main thumbnailer saves as jpg.) By staying
+            // lossless here, we keep all the policy decisions about
+            // image quality in the main thumbnailer.
+            fprintf(stderr, "Saving pixbuf to tiff\n");
+            GError* error = nullptr;
+            success = gdk_pixbuf_save(thumbnail, outfile.c_str(), "tiff", &error, "compression", "1", nullptr);
+            if (!success)
+            {
+                fprintf(stderr, "save_screenshot(): saving image: %s\n", error->message);
+                g_error_free(error);
+            }
+        }
+        g_main_loop_quit(main_loop.get());
+    };
+    ThumbnailExtractor extractor;
     try
     {
-        success = extract_thumbnail(uri, outfile);
+        extractor.extract(uri, callback);
     }
     catch (exception const& e)
     {
-        fprintf(stderr, "Error creating thumbnail: %s\n", e.what());
-        return 2;
+        fprintf(stderr, "Error starting extraction \"%s\": %s\n", argv[1], e.what());
+        return 1;
     }
+    g_main_loop_run(main_loop.get());
 
     return success ? 0 : 1;
 }

=== modified file 'tests/thumbnailer/thumbnailer_test.cpp'
--- tests/thumbnailer/thumbnailer_test.cpp	2015-07-30 04:28:48 +0000
+++ tests/thumbnailer/thumbnailer_test.cpp	2015-08-19 03:01:01 +0000
@@ -653,7 +653,7 @@
     catch (unity::ResourceException const& e)
     {
         string msg = e.what();
-        EXPECT_NE(string::npos, msg.find("extractor pipeline failed")) << msg;
+        EXPECT_NE(string::npos, msg.find("could not extract screenshot")) << msg;
     }
 }
 

=== modified file 'tests/vs-thumb/vs-thumb_test.cpp'
--- tests/vs-thumb/vs-thumb_test.cpp	2015-06-01 06:19:23 +0000
+++ tests/vs-thumb/vs-thumb_test.cpp	2015-08-19 03:01:01 +0000
@@ -42,37 +42,13 @@
 const char MP4_LANDSCAPE_TEST_FILE[] = TESTDATADIR "/gegl-landscape.mp4";
 const char MP4_PORTRAIT_TEST_FILE[] = TESTDATADIR "/gegl-portrait.mp4";
 const char VORBIS_TEST_FILE[] = TESTDATADIR "/testsong.ogg";
+const char EMPTY_TEST_FILE[] = TESTDATADIR "/empty";
 
 class ExtractorTest : public ::testing::Test
 {
 protected:
-    ExtractorTest()
-    {
-    }
-    virtual ~ExtractorTest()
-    {
-    }
-
-    virtual void SetUp() override
-    {
-        tempdir = "./vsthumb-test.XXXXXX";
-        if (mkdtemp(const_cast<char*>(tempdir.data())) == nullptr)
-        {
-            tempdir = "";
-            throw std::runtime_error("Could not create temporary directory");
-        }
-    }
-
-    virtual void TearDown() override
-    {
-        if (!tempdir.empty())
-        {
-            std::string cmd = "rm -rf \"" + tempdir + "\"";
-            ASSERT_EQ(system(cmd.c_str()), 0);
-        }
-    }
-
-    std::string tempdir;
+    ExtractorTest() = default;
+    virtual ~ExtractorTest() = default;
 };
 
 std::string filename_to_uri(const std::string& filename)
@@ -86,17 +62,20 @@
     return uri.get();
 }
 
-gobj_ptr<GdkPixbuf> load_image(const std::string& filename)
+gobj_ptr<GdkPixbuf> extract(std::string const& filename)
 {
-    GError* error = nullptr;
-    gobj_ptr<GdkPixbuf> image(gdk_pixbuf_new_from_file(filename.c_str(), &error));
-    if (error)
-    {
-        std::string message(error->message);
-        g_error_free(error);
-        throw std::runtime_error(message);
-    }
-    return std::move(image);
+    gobj_ptr<GdkPixbuf> image;
+    std::unique_ptr<GMainLoop, decltype(&g_main_loop_unref)> main_loop(
+        g_main_loop_new(nullptr, false), g_main_loop_unref);
+    auto callback = [&](GdkPixbuf* const thumbnail) {
+        // We copy the image here, because it may be backed by mmaped memory
+        image.reset(thumbnail ? gdk_pixbuf_copy(thumbnail) : nullptr);
+        g_main_loop_quit(main_loop.get());
+    };
+    ThumbnailExtractor extractor;
+    extractor.extract(filename_to_uri(filename), callback);
+    g_main_loop_run(main_loop.get());
+    return image;
 }
 
 bool supports_decoder(const std::string& format)
@@ -143,14 +122,8 @@
         return;
     }
 
-    ThumbnailExtractor extractor;
-    std::string outfile = tempdir + "/out.jpg";
-    extractor.set_uri(filename_to_uri(THEORA_TEST_FILE));
-    ASSERT_TRUE(extractor.has_video());
-    ASSERT_TRUE(extractor.extract_video_frame());
-    extractor.save_screenshot(outfile);
-
-    auto image = load_image(outfile);
+    auto image = extract(THEORA_TEST_FILE);
+    ASSERT_NE(nullptr, image.get());
     EXPECT_EQ(gdk_pixbuf_get_width(image.get()), 1920);
     EXPECT_EQ(gdk_pixbuf_get_height(image.get()), 1080);
 }
@@ -163,14 +136,8 @@
         return;
     }
 
-    ThumbnailExtractor extractor;
-    std::string outfile = tempdir + "/out.jpg";
-    extractor.set_uri(filename_to_uri(MP4_LANDSCAPE_TEST_FILE));
-    ASSERT_TRUE(extractor.has_video());
-    ASSERT_TRUE(extractor.extract_video_frame());
-    extractor.save_screenshot(outfile);
-
-    auto image = load_image(outfile);
+    auto image = extract(MP4_LANDSCAPE_TEST_FILE);
+    ASSERT_NE(nullptr, image.get());
     EXPECT_EQ(gdk_pixbuf_get_width(image.get()), 1920);
     EXPECT_EQ(gdk_pixbuf_get_height(image.get()), 1080);
 }
@@ -183,36 +150,29 @@
         return;
     }
 
-    ThumbnailExtractor extractor;
-    std::string outfile = tempdir + "/out.jpg";
-    extractor.set_uri(filename_to_uri(MP4_PORTRAIT_TEST_FILE));
-    ASSERT_TRUE(extractor.extract_video_frame());
-    extractor.save_screenshot(outfile);
-
-    auto image = load_image(outfile);
+    auto image = extract(MP4_PORTRAIT_TEST_FILE);
+    ASSERT_NE(nullptr, image.get());
     EXPECT_EQ(gdk_pixbuf_get_width(image.get()), 720);
     EXPECT_EQ(gdk_pixbuf_get_height(image.get()), 1280);
 }
 
 TEST_F(ExtractorTest, extract_vorbis_cover_art)
 {
-    ThumbnailExtractor extractor;
-
-    std::string outfile = tempdir + "/out.jpg";
-    extractor.set_uri(filename_to_uri(VORBIS_TEST_FILE));
-    ASSERT_FALSE(extractor.has_video());
-    ASSERT_TRUE(extractor.extract_audio_cover_art());
-    extractor.save_screenshot(outfile);
-
-    auto image = load_image(outfile);
+    auto image = extract(VORBIS_TEST_FILE);
+    ASSERT_NE(nullptr, image.get());
     EXPECT_EQ(gdk_pixbuf_get_width(image.get()), 200);
     EXPECT_EQ(gdk_pixbuf_get_height(image.get()), 200);
 }
 
+TEST_F(ExtractorTest, extract_empty)
+{
+    auto image = extract(EMPTY_TEST_FILE);
+    ASSERT_EQ(nullptr, image.get());
+}
+
 TEST_F(ExtractorTest, file_not_found)
 {
-    ThumbnailExtractor extractor;
-    EXPECT_THROW(extractor.set_uri(filename_to_uri(TESTDATADIR "/no-such-file.ogv")), std::runtime_error);
+    EXPECT_THROW(extract(TESTDATADIR "/no-such-file.ogv"), std::runtime_error);
 }
 
 int main(int argc, char** argv)