Merge lp:~jhodapp/unity-lens-video/remote-videos-scope-trunk into lp:unity-lens-video/remote-videos-scope-trunk

Proposed by Jim Hodapp
Status: Merged
Approved by: Paweł Stołowski
Approved revision: 67
Merged at revision: 57
Proposed branch: lp:~jhodapp/unity-lens-video/remote-videos-scope-trunk
Merge into: lp:unity-lens-video/remote-videos-scope-trunk
Diff against target: 183 lines (+97/-13)
1 file modified
src/unity-scope-video-remote (+97/-13)
To merge this branch: bzr merge lp:~jhodapp/unity-lens-video/remote-videos-scope-trunk
Reviewer Review Type Date Requested Status
Paweł Stołowski (community) Approve
Review via email: mp+121867@code.launchpad.net

This proposal supersedes a proposal from 2012-08-29.

Commit message

Added details to the Amazon video preview

Description of the change

Added details to the Amazon video preview.

To post a comment you must log in.
Revision history for this message
Jim Hodapp (jhodapp) wrote : Posted in a previous version of this proposal

One caveat: the SERVER is currently set to the staging URL. This will need to be changed when the JSON API v0 extended is pushed into production.

Revision history for this message
Jim Hodapp (jhodapp) wrote : Posted in a previous version of this proposal

Change SERVER to videosearch.staging.ubuntu.com to test out the video details for the preview.

Revision history for this message
Paweł Stołowski (stolowski) wrote : Posted in a previous version of this proposal

Looking good, just minor comments:

46 # Clear results details map
47 - self.details_map = {}

The comment above is not needed anymore.

128 + duration = int(details["duration"]) / 60
129 + # TODO: Extend VideoPreview class and set property for showing video length

Extending MoviePreview shouldn't be needed; looking at the mockups, video length should be displayed next to year as preview subtitle; in fact year property is not really used by Unity, so you should populate subtitle with both year and duration; don't forget about units, e.g. "2012, 88 mins".

Revision history for this message
Paweł Stołowski (stolowski) : Posted in a previous version of this proposal
review: Needs Fixing
Revision history for this message
Paweł Stołowski (stolowski) wrote : Posted in a previous version of this proposal

109 + subtitle = release_date.strftime("%Y") + ", " + str(duration) + " mins"

This will crash if release_date or duration are unset (i.e. not in details). Also str(duration) + " mins" should be i18n friendly.

review: Needs Fixing
64. By Jim Hodapp

More flexible duration internationalization. Also allow year or duration to be shown independently of each other being present.

65. By Jim Hodapp

Hide the rating widget by default since the JSON API doesn't yet support rating data in details.

66. By Jim Hodapp

Make the fetch of details data more robust. Signify when the remote server could not be connected to.

Revision history for this message
Paweł Stołowski (stolowski) wrote :

78 uri = rawuri.split('lens-meta://')[0]
...
83 - if self.details_map.has_key (uri):
84 - desc = self.details_map[uri][0]
...
87 + uri = details['browser_url']

Seems we don't need uri anymore?

review: Needs Fixing
67. By Jim Hodapp

No longer need the variable 'uri' in on_preview_uri().

Revision history for this message
Paweł Stołowski (stolowski) wrote :

Looks good and works after adding geo_store=US to urls when run from Europe. Approving.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/unity-scope-video-remote'
2--- src/unity-scope-video-remote 2012-08-14 12:50:14 +0000
3+++ src/unity-scope-video-remote 2012-08-30 13:20:35 +0000
4@@ -22,6 +22,7 @@
5 import sys
6 from urllib import urlencode
7 import time
8+from datetime import datetime
9 import gettext
10
11 #pylint: disable=E0611
12@@ -49,6 +50,7 @@
13 gettext.bindtextdomain(APP_NAME, LOCAL_PATH)
14 gettext.textdomain(APP_NAME)
15 _ = gettext.gettext
16+n_ = gettext.ngettext
17
18 BUS_NAME = "net.launchpad.scope.RemoteVideos"
19 SERVER = "http://videosearch.ubuntu.com/v0"
20@@ -89,14 +91,14 @@
21 self.query_list_of_sources()
22 self.update_recommendations()
23 self.scope.export()
24- self.details_map = {} #holds details not passed via lens-meta://, needed by preview
25
26 def update_recommendations(self):
27 """Query the server for 'recommendations'.
28
29- In v0, that means simply do an empty search.
30+ In v0 extended, that means simply do an empty search with Amazon
31+ as the source since it's the only paid provider at the moment.
32 """
33- msg = Soup.Message.new("GET", SERVER + "/search")
34+ msg = Soup.Message.new("GET", SERVER + "/search?q=&sources=Amazon")
35 self.session.queue_message(msg, self._recs_cb, None)
36
37 def _recs_cb(self, session, msg, user_data):
38@@ -175,11 +177,10 @@
39 the server, update the model and notify the lens when we are done."""
40 search_string = search.props.search_string.strip()
41 # note search_string is a utf-8 encoded string, now:
42- print "Search changed to %r" % search_string
43+ print "Remote scope search changed to %r" % search_string
44 model = search.props.results_model
45 model.clear()
46 # Clear results details map
47- self.details_map = {}
48 # Create a list of activated sources
49 sources = [source for source in self.sources_list
50 if self.source_activated(source)]
51@@ -207,13 +208,17 @@
52 title = i['title'].encode('utf-8')
53 icon = i['img'].encode('utf-8')
54 comment = i['source'].encode('utf-8')
55- self.details_map[uri] = (comment) #TODO: more to come
56+ details_url = ""
57+ if "details" in i:
58+ details_url = i['details'].encode('utf-8')
59+
60 if uri.startswith('http'):
61 # Instead of an uri, we pass a string of uri + metadata
62- model.append(uri+'lens-meta://'+title+'lens-meta://'+icon,
63+ model.append(uri+'lens-meta://'+title+'lens-meta://'+icon+'lens-meta://'+details_url,
64 icon, 2, "text/html", str(title), str(comment), str(uri))
65 except (KeyError, TypeError, AttributeError):
66 print "Malformed result, skipping."
67+
68 model.flush_revision_queue()
69 if search:
70 search.finished()
71@@ -279,27 +284,106 @@
72
73 def on_preview_uri (self, scope, rawuri):
74 """Preview request handler"""
75- uri = rawuri.split('lens-meta://')[0]
76+ details_url = rawuri.split('lens-meta://')[3]
77+ details = self.get_details(details_url)
78+
79 title = rawuri.split('lens-meta://')[1]
80 thumbnail_uri = rawuri.split('lens-meta://')[2]
81 subtitle = ''
82 desc = ''
83- if self.details_map.has_key (uri):
84- desc = self.details_map[uri][0]
85+ source = ''
86+ if isinstance(details, dict):
87+ title = details['title']
88+ thumbnail_uri = details['image']
89+ desc = details['description']
90+ source = details['source']
91+
92 try:
93 thumbnail_file = Gio.File.new_for_uri (thumbnail_uri)
94 thumbnail_icon = Gio.FileIcon.new(thumbnail_file)
95 except:
96 thumbnail_icon = None
97+
98+ release_date = None
99+ duration = 0
100+
101+ if "release_date" in details:
102+ try:
103+ # v1 spec states release_date will be YYYY-MM-DD
104+ release_date = datetime.strptime(details["release_date"], "%Y-%m-%d")
105+ except ValueError:
106+ print "Failed to parse release_date since it was not in format: YYYY-MM-DD"
107+
108+ if "duration" in details:
109+ # Given in seconds, convert to minutes
110+ duration = int(details["duration"]) / 60
111+
112+ if release_date != None:
113+ subtitle = release_date.strftime("%Y")
114+ if duration > 0:
115+ d = n_("%d min", "%d mins", duration) % duration
116+ if len(subtitle) > 0:
117+ subtitle += ", " + d
118+ else:
119+ subtitle = d
120+
121 preview = Unity.MoviePreview.new(title, subtitle, desc, thumbnail_icon)
122 play_video = Unity.PreviewAction.new("play", _("Play"), None)
123 play_video.connect('activated', self.play_video)
124 preview.add_action(play_video)
125- #TODO: insert proper uploader and upload date when available in videosearch metadata
126- preview.add_info(Unity.InfoHint.new("uploaded-by", _("Uploaded by"), None, ""));
127- preview.add_info(Unity.InfoHint.new("uploaded-on", _("Uploaded on"), None, ""));
128+ # For now, rating == -1 and num_ratings == 0 hides the rating widget from the preview
129+ preview.set_rating(-1, 0)
130+
131+ # TODO: For details of future source types, factor out common detail key/value pairs
132+ if source == "Amazon":
133+ if "directors" in details:
134+ directors = details["directors"]
135+ preview.add_info(Unity.InfoHint.new("directors",
136+ n_("Director", "Directors", len(directors)), None, ', '.join(directors)))
137+ if "starring" in details:
138+ cast = details["starring"]
139+ if cast:
140+ preview.add_info(Unity.InfoHint.new("cast",
141+ n_("Cast", "Cast", len(cast)), None, ', '.join(cast)))
142+ if "genres" in details:
143+ genres = details["genres"]
144+ if genres:
145+ preview.add_info(Unity.InfoHint.new("genres",
146+ n_("Genre", "Genres", len(genres)), None, ', '.join(genres)))
147+ # TODO: Add Vimeo & YouTube details for v1 of JSON API
148+ else:
149+ if "uploaded_by" in details:
150+ preview.add_info(Unity.InfoHint.new("uploaded-by", _("Uploaded by"),
151+ None, details["uploaded_by"]))
152+ if "date_uploaded" in details:
153+ preview.add_info(Unity.InfoHint.new("uploaded-on", _("Uploaded on"),
154+ None, details["date_uploaded"]))
155+
156 return preview
157
158+ def get_details(self, details_url):
159+ """Get online video details for preview"""
160+ details = []
161+ if len(details_url) == 0:
162+ print "Cannot get preview details, no details_url specified."
163+ return details
164+
165+ if self._pending is not None:
166+ self.session.cancel_message(self._pending,
167+ Soup.KnownStatusCode.CANCELLED)
168+
169+ self._pending = Soup.Message.new("GET", details_url)
170+ # Send a synchronous GET request for video metadata details
171+ self.session.send_message(self._pending)
172+ try:
173+ details = json.loads(self._pending.response_body.data)
174+ except ValueError:
175+ print "Error: got garbage json from the server"
176+ except TypeError:
177+ print "Error: failed to connect to videosearch server to retrieve details."
178+
179+ return details
180+
181 def play_video (self, action, uri):
182 """Preview request - Play action handler"""
183 return self.on_activate_uri (action, uri)

Subscribers

People subscribed via source and target branches

to all changes: