Merge lp:~osomon/moovida/element_redirections into lp:moovida

Proposed by Olivier Tilloy
Status: Merged
Merged at revision: not available
Proposed branch: lp:~osomon/moovida/element_redirections
Merge into: lp:moovida
Diff against target: 38 lines (+11/-11)
1 file modified
elisa-plugins/elisa/plugins/poblesec/ (+11/-11)
To merge this branch: bzr merge lp:~osomon/moovida/element_redirections
Reviewer Review Type Date Requested Status
Michał Sawicz (community) code functional Approve
Review via email:
To post a comment you must log in.
Revision history for this message
Olivier Tilloy (osomon) wrote :

This branch fixes wrong handling of stream redirections in the general case where the "new-location" attribute of the message contains a full URI rather than just a filename.

Both cases are now handled gracefully, although it's unclear whether the latter is valid (it can be observed for Apple trailers so we need to support it anyway).

As a side effect, the fix improves the situation regarding bug #494713: for some mms:// streams Moovida previously crashed after entering an infinite loop of redirections, it now fails gracefully (reporting an error message to the user). It can be verified with e.g. mms://

I also fixed this annoying issue that the Player was not logging anything.

Revision history for this message
Michał Sawicz (saviq) wrote :

This merge indeed fixes this issue. I couldn't find any regressions and everything behaved as expected.

review: Approve (code functional)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'elisa-plugins/elisa/plugins/poblesec/'
2--- elisa-plugins/elisa/plugins/poblesec/ 2010-01-08 11:38:15 +0000
3+++ elisa-plugins/elisa/plugins/poblesec/ 2010-01-28 17:33:18 +0000
4@@ -425,7 +425,7 @@
5 """
6 return self.animated.opacity > 0
8-class Player(gobject.GObject, Loggable):
9+class Player(Loggable, gobject.GObject):
11 STOPPED = 0
12 PLAYING = 1
13@@ -656,16 +656,16 @@
14 decoder = pbutils.get_decoder_description(mimetype)
15 self.emit('player-missing-decoder', mimetype, decoder)
16 elif message.structure.get_name() == 'redirect':
18- # we got a new location for current uri. Only the filename
19- # of the uri is concerned. So we build a new uri with
20- # updated filename and reset the pipeline state.
21- current_uri = MediaUri(self.pipeline.get_property('uri'))
22- filename = current_uri.filename
23- idx = unicode(current_uri).find(filename)
24- new_filename = message.structure['new-location']
25- new_uri = current_uri[:idx] + new_filename
26- self.pipeline.set_property('uri', new_uri)
27+ new_location = message.structure['new-location']
28+ if '://' not in new_location:
29+ # The new location is not a full URI, it differs only from the
30+ # original one by the filename. It is not clear whether this is
31+ # valid, but it can be observed for e.g. some Apple trailers.
32+ uri = MediaUri(self.pipeline.get_property('uri'))
33+ uri.path = uri.path[:uri.path.rfind('/')+1] + new_location
34+ new_location = unicode(uri)
35+'Redirected to %s' % new_location)
36+ self.pipeline.set_property('uri', new_location)
37 self.pipeline.set_state(gst.STATE_NULL)
38 self.pipeline.set_state(gst.STATE_PLAYING)


People subscribed via source and target branches