Merge lp:~jamesh/camera-app/pre-cache-video-thumbnails into lp:camera-app

Proposed by James Henstridge
Status: Merged
Approved by: Florian Boucault
Approved revision: 582
Merged at revision: 587
Proposed branch: lp:~jamesh/camera-app/pre-cache-video-thumbnails
Merge into: lp:camera-app
Diff against target: 75 lines (+30/-0)
3 files modified
GalleryView.qml (+26/-0)
GalleryViewLoader.qml (+3/-0)
camera-app.qml (+1/-0)
To merge this branch: bzr merge lp:~jamesh/camera-app/pre-cache-video-thumbnails
Reviewer Review Type Date Requested Status
Florian Boucault (community) Approve
PS Jenkins bot continuous-integration Needs Fixing
Review via email: mp+273515@code.launchpad.net

Commit message

After shooting a video, request a thumbnail so that it will be available in the cache when needed (e.g. by the Gallery or Dash).

Description of the change

Due to the bad performance of thumbnailing H.264 video files (on the order of a 1.4 seconds) I started investigating whether we could improve the user experience with a modification to camera-app.

This branch requests a small thumbnail after shooting a video with the camera-app, which will prime the thumbnailer-service's full-size image cache with a screenshot of the video, ready to provide thumbnails for whatever asks for one later (e.g. the "My Videos" scope on the dash, gallery-app, etc).

With this patch applied and an updated camera-app installed on my phone, I can see a thumbnail request in ~/.cache/upstart/dbus.log when I shoot a video. If I switch back to the dash, the video shows up almost instantly rather than the old delay.

It wasn't clear where best to put this code, so if there is a better location I can update the branch.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Florian Boucault (fboucault) wrote :

Functionally, it's great and achieves what the description promises with a very minimal cost. Very nice improvement indeed!
Code wise, only a couple of details:
- it would be nice to unload the Image once the thumbnail has been loaded; although it is a very small save of a 32x32 image
- the Image {}'s id could be more explicit than just 'thumbnail', for example 'preCachingThumbnail' or something
- also a small comment next to the Image explaining its purpose and specifically why a sourceSize of 32x32 suffices to provide all the benefits would be extra useful

Thanks a lot!

582. By James Henstridge

Updates based on Florian's review.

Revision history for this message
James Henstridge (jamesh) wrote :

Thanks for the review.

> Functionally, it's great and achieves what the description promises with a
> very minimal cost. Very nice improvement indeed!
> Code wise, only a couple of details:
> - it would be nice to unload the Image once the thumbnail has been loaded;
> although it is a very small save of a 32x32 image

I've added an onStatusChanged handler to the image that resets the filename to "" whenever the status changes to Image.Ready.

> - the Image {}'s id could be more explicit than just 'thumbnail', for example
> 'preCachingThumbnail' or something

Done.

> - also a small comment next to the Image explaining its purpose and
> specifically why a sourceSize of 32x32 suffices to provide all the benefits
> would be extra useful

Done.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Florian Boucault (fboucault) wrote :

Perfect!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'GalleryView.qml'
2--- GalleryView.qml 2015-02-12 19:25:00 +0000
3+++ GalleryView.qml 2015-10-23 05:21:20 +0000
4@@ -17,6 +17,7 @@
5 import QtQuick 2.2
6 import Ubuntu.Components 1.1
7 import Ubuntu.Content 0.1
8+import Ubuntu.Thumbnailer 0.1
9 import CameraApp 0.1
10 import "MimeTypeMapper.js" as MimeTypeMapper
11
12@@ -51,6 +52,10 @@
13 galleryView.model.prependFile(filePath);
14 }
15
16+ function precacheThumbnail(filePath) {
17+ preCachingThumbnail.filename = filePath;
18+ }
19+
20 function exitUserSelectionMode() {
21 model.clearSelection();
22 model.singleSelectionOnly = true;
23@@ -226,6 +231,27 @@
24 }
25 }
26
27+ // This image component is used to pre-load thumbnails of recently
28+ // created files. This is primarily to hide the delays in
29+ // thumbnailing videos.
30+ Image {
31+ id: preCachingThumbnail
32+ property string filename
33+
34+ visible: false
35+ asynchronous: true
36+ cache: false
37+ sourceSize.width: 32
38+ sourceSize.height: 32
39+ source: filename ? "image://thumbnailer/%1".arg(filename) : ""
40+
41+ onStatusChanged: {
42+ if (status == Image.Ready) {
43+ filename = "";
44+ }
45+ }
46+ }
47+
48 state: galleryView.gridMode ? "GRID" : "SLIDESHOW"
49 states: [
50 State {
51
52=== modified file 'GalleryViewLoader.qml'
53--- GalleryViewLoader.qml 2015-01-24 12:52:12 +0000
54+++ GalleryViewLoader.qml 2015-10-23 05:21:20 +0000
55@@ -31,6 +31,9 @@
56 loader.item.prependMediaToModel(filePath);
57 }
58
59+ function precacheThumbnail(filePath) {
60+ loader.item.precacheThumbnail(filePath);
61+ }
62
63 asynchronous: true
64
65
66=== modified file 'camera-app.qml'
67--- camera-app.qml 2015-07-03 10:32:14 +0000
68+++ camera-app.qml 2015-10-23 05:21:20 +0000
69@@ -245,6 +245,7 @@
70 onVideoShot: {
71 galleryView.prependMediaToModel(filePath);
72 galleryView.showLastPhotoTaken();
73+ galleryView.precacheThumbnail(filePath);
74 }
75 }
76

Subscribers

People subscribed via source and target branches