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
=== modified file 'elisa-plugins/elisa/plugins/poblesec/'
--- elisa-plugins/elisa/plugins/poblesec/ 2010-01-08 11:38:15 +0000
+++ elisa-plugins/elisa/plugins/poblesec/ 2010-01-28 17:33:18 +0000
@@ -425,7 +425,7 @@
425 """425 """
426 return self.animated.opacity > 0426 return self.animated.opacity > 0
428class Player(gobject.GObject, Loggable):428class Player(Loggable, gobject.GObject):
430 STOPPED = 0430 STOPPED = 0
431 PLAYING = 1431 PLAYING = 1
@@ -656,16 +656,16 @@
656 decoder = pbutils.get_decoder_description(mimetype)656 decoder = pbutils.get_decoder_description(mimetype)
657 self.emit('player-missing-decoder', mimetype, decoder)657 self.emit('player-missing-decoder', mimetype, decoder)
658 elif message.structure.get_name() == 'redirect':658 elif message.structure.get_name() == 'redirect':
659"redirect")659 new_location = message.structure['new-location']
660 # we got a new location for current uri. Only the filename660 if '://' not in new_location:
661 # of the uri is concerned. So we build a new uri with661 # The new location is not a full URI, it differs only from the
662 # updated filename and reset the pipeline state.662 # original one by the filename. It is not clear whether this is
663 current_uri = MediaUri(self.pipeline.get_property('uri'))663 # valid, but it can be observed for e.g. some Apple trailers.
664 filename = current_uri.filename664 uri = MediaUri(self.pipeline.get_property('uri'))
665 idx = unicode(current_uri).find(filename)665 uri.path = uri.path[:uri.path.rfind('/')+1] + new_location
666 new_filename = message.structure['new-location']666 new_location = unicode(uri)
667 new_uri = current_uri[:idx] + new_filename667'Redirected to %s' % new_location)
668 self.pipeline.set_property('uri', new_uri)668 self.pipeline.set_property('uri', new_location)
669 self.pipeline.set_state(gst.STATE_NULL)669 self.pipeline.set_state(gst.STATE_NULL)
670 self.pipeline.set_state(gst.STATE_PLAYING)670 self.pipeline.set_state(gst.STATE_PLAYING)


People subscribed via source and target branches