Elisa Media Center

Merge lp:~joni-noplu/elisa/teletext_fix into lp:elisa

  1. teletext_fix
  2. Merge into 1.0
Proposed by Jonathan Rauprich
Status: Merged
Merged at revision: 1617
Proposed branch: lp:~joni-noplu/elisa/teletext_fix
Merge into: lp:elisa
Diff against target: 21 lines (+10/-1)
1 file modified
elisa-plugins/elisa/plugins/poblesec/player_video.py (+10/-1)
To merge this branch: bzr merge lp:~joni-noplu/elisa/teletext_fix
Reviewer Review Type Date Requested Status
Olivier Tilloy Approve
Review via email: mp+26430@code.launchpad.net

Description of the change

This fixes Bug #415031
To test it, u can download the attached file in the bug report. Instead of showing a popup to the user when a stream with teletext is played, only a debug message will be shown.

To post a comment you must log in.
Revision history for this message
Olivier Tilloy (osomon) wrote :

Thanks for the patch!
I tested it and it behaves as expected.
I have a couple of minor cosmetic remarks on the patch itself, if you don't object I'll apply the following tweaks and merge:

- Typo: s/Gestreamer/GStreamer/
- Typo: s/poping/popping/
- I'd like the comment to include a link to the bug report (bug #415031)
- The debug string contains extra spaces between "for" and "teletext"

review: Approve
Revision history for this message
Jonathan Rauprich (joni-noplu) wrote :

Thanks for reviewing, feel free to do so.

On Mon, 2010-05-31 at 15:47 +0000, Olivier Tilloy wrote:
> Review: Approve
> Thanks for the patch!
> I tested it and it behaves as expected.
> I have a couple of minor cosmetic remarks on the patch itself, if you don't object I'll apply the following tweaks and merge:
>
> - Typo: s/Gestreamer/GStreamer/
> - Typo: s/poping/popping/
> - I'd like the comment to include a link to the bug report (bug #415031)
> - The debug string contains extra spaces between "for" and "teletext"
>

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

Looks good enough. I would vote for adding a ignored_streams list to the config file, what do you think? You could simply add a list that contains the 'private/teletext' mimetype in its defaults and read it here.

Revision history for this message
Paul van Tilburg (paulvt) wrote :

Typo: s/telext/teletext/

Revision history for this message
Olivier Tilloy (osomon) wrote :

I have already tweaked accordingly and merged Jonathan's branch.

@Michał: yes, I though of that too, but it looked overkill until we have more than one mime type we want to filter out. At that point we'll need to implement your idea or something similar.

@Paul: I had missed this one in my review, but I fixed it before pushing. All good!

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/player_video.py'
2--- elisa-plugins/elisa/plugins/poblesec/player_video.py 2010-01-28 17:18:54 +0000
3+++ elisa-plugins/elisa/plugins/poblesec/player_video.py 2010-05-31 15:11:38 +0000
4@@ -654,7 +654,16 @@
5 details = caps.to_string().split(', ')
6 mimetype = details[0]
7 decoder = pbutils.get_decoder_description(mimetype)
8- self.emit('player-missing-decoder', mimetype, decoder)
9+ # Gestreamer complains that teletext decoder is missing, but there
10+ # is no teletext decoder (in gstreamer) and its not realy a problem
11+ # as long as u don't care for teletext. So this is to prevent the
12+ # warning message poping up everytime a videostream with teletext is
13+ # played.
14+ if mimetype == 'private/teletext':
15+ self.debug('Videostream with telext found, no decoder for \
16+ teletext, dumping it!')
17+ else:
18+ self.emit('player-missing-decoder', mimetype, decoder)
19 elif message.structure.get_name() == 'redirect':
20 new_location = message.structure['new-location']
21 if '://' not in new_location:

Subscribers

People subscribed via source and target branches