Merge lp:~tdelaet/entertainer/fixes-for-future-branch into lp:entertainer/future

[Thomas Delaet]

[Thomas Delaet (tdelaet)]

This branch fixes two problems I had when trying out the entertainer future branch.

1. The latest version of pyclutter-gst doesn't have the get_playbin() function anymore. This function is renamed to get_pipeline(). (see changelog of pyclutter-gst)

2. When a video is finished playing, my screen turns blue. I fixed this by subscribing to the 'stop' signal of the media player. When the user interface receives this signal, it changes the screen to the previous screen.

See in commit messages for more details.

[Matt Layman (mblayman)]

Thomas, thanks for the branch. I might have a chance to test it out this weekend.

[Matt Layman (mblayman)]

Thomas,

Thanks very much for the branch. Just a few notes:

 1. I guess we don't explicitly state it anywhere, but the pyclutter-gst that we are using is the one that is packaged by Francesco Marella in his PPA (if I could figure out how to copy his packages to the Entertainer PPA, I would do that). That being said, we're a little behind on which version of pyclutter-gst is in use so your branch is a little ahead of the curve. After Ubuntu Lucid Lynx is released, I will probably try to update our pyclutter-gst and gtk, then your code will be very valuable.
 2. I'm about to abandon the future branch. It's an added level of complexity that the Entertainer team thought we would use, but it was a branch that lived on too long to suit the purpose we wanted. Rest assured that trunk is now up-to-date with all of future's code. Therefore, I would request that after you make some changes (which I'm about to mention), please submit for merging into trunk.
 3. The Entertainer developers decided to use copyright assignment so that we could do something in the future like change the license (to something like GPLv3). Because of that, in order for us to accept your code, you'd have to be okay assigning the copyright to Entertainer Developers. To do that, simply add yourself, alphabetically by first name, to the list of contributors at the bottom of the docs/COPYING file.
 4. Finally, a code review comment... You changed the method name to get_pipeline, which is the right thing to do for the API change, but you left the variable as self.playbin. I think this would lead to confusion in the future, and I would request that you change every instance of the self.playbin variable to self.pipeline.

Once these changes are made, I'll be more than happy to merge the branch (though I may not do it until some time after Lucid is released when I actually need the code).

Thanks!
Matt

review: Needs Fixing

[Matt Layman (mblayman)]

I'm going to mark the proposal as rejected, but that's only because I would like to see it merged into trunk instead of future.

Unmerged revisions

387. By Thomas Delaet <thomas@cole> on 2010-02-26

get_playbin does not exist anymore in recent pyclutter-gst bindings. use correct name.

386. By Thomas Delaet <thomas@cole> on 2010-02-26

When a video is finished, the screens turns blue.

The reason for this is that the video_osd screen does not change to another screen and the video texture is removed when playing is done.

This commit fixes the problem: when the user interface detects the stop event from the media player, it switches to the previous screen

=== modified file 'entertainerlib/client/media_player.py'
--- entertainerlib/client/media_player.py	2009-12-21 22:21:55 +0000
+++ entertainerlib/client/media_player.py	2010-02-26 23:18:14 +0000
@@ -77,7 +77,7 @@
         self._internal_callback_timeout_key = None
 
         self.video_texture = cluttergst.VideoTexture()
-        self.playbin = self.video_texture.get_playbin()
+        self.playbin = self.video_texture.get_pipeline()
         self.playbin.set_property("volume", 0.5)
         self._volume = 10
         self.bus = self.playbin.get_bus()

=== modified file 'entertainerlib/gui/user_interface.py'
--- entertainerlib/gui/user_interface.py	2009-12-22 02:44:49 +0000
+++ entertainerlib/gui/user_interface.py	2010-02-26 23:18:14 +0000
@@ -87,6 +87,7 @@
         self.player = MediaPlayer(self.stage,
             self.config.stage_width, self.config.stage_height)
         self.player.connect('volume-changed', self._on_volume_changed)
+        self.player.connect('stop', self.on_stop)
 
         # Initialize menu overlay texture
         self.is_overlay = False
@@ -422,3 +423,7 @@
         self.fade_screen_behaviour.set_bounds(50, 255)
         self.fade_screen_timeline.start()
 
+    def on_stop(self, event):
+        '''Restore previous screen.'''
+        self.move_to_previous_screen()
+