Merge lp:~artem-anufrij/audience/Bugfix-1359688 into lp:~audience-members/audience/trunk

[Artem Anufrij]

Reset last played videos and show welcome after playback ends

[Cody Garver (codygarver)]

Shows the welcome (with continued background audio) at the end of playback of each playlist item, instead of only the final playlist item

review: Needs Fixing

[lp:~artem-anufrij/audience/Bugfix-1359688]

399. By artem-anufrij on 2014-09-19

The fixes have been corrected. Please, check it.

[Artem Anufrij (artem-anufrij)]

Please, check my fixes...

Thx

review: Approve

[Fabio Zaramella (fabiozaramella)]

it looks good to me

review: Approve

[Cody Garver (codygarver)]

I'm gonna merge this tomorrow with some minor cosmetic changes

On Sun, Sep 21, 2014 at 3:45 PM, Fabio Zaramella <email address hidden>
wrote:

> Review: Approve
>
> it looks good to me
> --
>
> https://code.launchpad.net/~artem-anufrij/audience/Bugfix-1359688/+merge/235220
> You are reviewing the proposed merge of
> lp:~artem-anufrij/audience/Bugfix-1359688 into lp:audience.
>

--
Cody Garver

[Viko Adi Rahmawan (vikoadi)]

I think we want a "Replay playlist" in the newly made welcome screen is it?
And video doesnt get resumed after closing and reopening audience.

review: Needs Fixing

[Viko Adi Rahmawan (vikoadi)]

Ah, i just see that the Bug #1363391 and resuming video imediately is a design decission (which is debatable by the way) so changing this behaviour need UX brainstorming. And current resuming behaviour is respecting resume-videos key in gsettings, so if its to be changed you only need to change default value for resume-videos key.
thanks

[lp:~artem-anufrij/audience/Bugfix-1359688]

400. By artem-anufrij on 2014-09-25

Replay button on welcome screen at the end of playback.

[Artem Anufrij (artem-anufrij)]

> I think we want a "Replay playlist" in the newly made welcome screen is it?
> And video doesnt get resumed after closing and reopening audience.

Here is a example for a replay button on welcome screen.

What do you think?

[Artem Anufrij (artem-anufrij)]

https://bugs.launchpad.net/audience/+bug/1359688/+attachment/4215095/+files/Audience%20-%20replay%20video%20or%20playlist.png

[Viko Adi Rahmawan (vikoadi)]

* put replay button as number two button, under the open file button so that it looks like changing resume button
* use last video title as replay button description text just like resume button
* dont clear video setting, instead call playlist.save_playlist_config (); set playlist first video as setting.current_video and set setting.last_stopped to 0.0 so that if you close audience resume video will pickup your playlist
* as i type in previous comment, dont remove the ability to resume video on audience opening

I cant reproduce Bug #1360667 so have nothing to say about it.

review: Needs Fixing

[lp:~artem-anufrij/audience/Bugfix-1359688]

401. By artem-anufrij on 2014-09-25

replay button was put as number two button; video setting won't be cleaned; ability to resume video on audience opening

[Artem Anufrij (artem-anufrij)]

> * put replay button as number two button, under the open file button so that
> it looks like changing resume button

OK

> * use last video title as replay button description text just like resume
> button

Where can I do that after append the Button?

> * dont clear video setting, instead call playlist.save_playlist_config (); set
> playlist first video as setting.current_video and set setting.last_stopped to
> 0.0 so that if you close audience resume video will pickup your playlist

OK

> * as i type in previous comment, dont remove the ability to resume video on
> audience opening

OK

[Viko Adi Rahmawan (vikoadi)]

Yeah, i think we can't change it, just ignore my comment

try this one:
- play video till it show the welcome screen
- close audience
- open audience, there is resume video, close audience
- open audience, but now there isnt resume video entries.

review: Needs Fixing

[Artem Anufrij (artem-anufrij)]

> Yeah, i think we can't change it, just ignore my comment
>
> try this one:
> - play video till it show the welcome screen
> - close audience
> - open audience, there is resume video, close audience
> - open audience, but now there isnt resume video entries.

And if I would like to see again and again the same video to be played at the start?

I think automaticly resume is not a good feature. A resume button in the welcome screen is nice. Because if i start audience i don't wont automaticly play the last video, but with the resume button in the welcome screen i have the option to do that.

[Viko Adi Rahmawan (vikoadi)]

I dont think you understand my last comment. Im fine with the welcome screen but not with clear_video_setting.

I mean the problem is with clear_video_settings(); in on_destroy ().
remove those clear_video_settings () and my complain about there is no resume video entries will be fixed.

Thanks

review: Needs Fixing

[Artem Anufrij (artem-anufrij)]

> I mean the problem is with clear_video_settings(); in on_destroy ().
> remove those clear_video_settings () and my complain about there is no resume
> video entries will be fixed.

Current clear_video_setting () will be called after a DVD was playing and not everytime. I'll create a flowchart.

[Viko Adi Rahmawan (vikoadi)]

Follow my steps above, setting will get cleared.

review: Needs Fixing

[Artem Anufrij (artem-anufrij)]

> Follow my steps above, setting will get cleared.

Yes! I think I have understood what you mean.

Please check my flowchart:
https://bugs.launchpad.net/audience/+bug/1359688/+attachment/4215892/+files/Audience.jpg

Is it correct?

[Viko Adi Rahmawan (vikoadi)]

As I dont have DVD i'll let you decide whether we need DVD checking in bottom left. You dont need to remove it if you think thats necessary.
The problem with current code is when on_destroy being called from welcomescreen in topleft, clear_setting got called which is unecessary

i think this one should work CMIIW

        private void on_destroy () {
            if ( video_player.uri.has_prefix ("dvd://")) {
                clear_video_settings();
                return;
            }

            if (video_player.uri == null || video_player.uri == "" )
                return;

            save_last_played_videos ();
        }

And dont forget about inline comment i type before about some little code format changes.
Thanks Artem.

review: Needs Fixing

[Viko Adi Rahmawan (vikoadi)]

Sorry I meant

As I dont have DVD i'll let you decide whether we need DVD checking in bottom *right

review: Needs Fixing

[lp:~artem-anufrij/audience/Bugfix-1359688]

402. By artem-anufrij on 2014-09-26

Implemented as agreed (flowchart).

[Artem Anufrij (artem-anufrij)]

Hey guys, please check my changes.

[Viko Adi Rahmawan (vikoadi)]

Great work Artem,
As I can't reproduce Bug #1360667 I'll just give it a go.
Thanks

review: Approve

[Cody Garver (codygarver)]

review: Approve

=== modified file 'src/Audience.vala'
--- src/Audience.vala	2014-09-03 00:07:10 +0000
+++ src/Audience.vala	2014-09-26 20:46:20 +0000
@@ -198,7 +198,12 @@
             //end
             video_player.ended.connect (() => {
                 Idle.add (() => {
-                    playlist.next ();
+                    if (!playlist.next ()) {
+                        welcome.set_item_visible (1, false);
+                        welcome.set_item_visible (2, true);
+                        welcome.show_all ();
+                        clutter.hide ();
+                    }
                     return false;
                 });
             });
@@ -328,25 +333,30 @@
             }
 
             welcome.set_item_visible (1, show_last_file);
-
+            
+            welcome.append ("media-playlist-repeat", _("Replay"), _("Replay last video, or playlist"));
+            welcome.set_item_visible (2, false);
+            
             welcome.append ("media-cdrom", _("Play from Disc"), _("Watch a DVD or open a file from disc"));
-            welcome.set_item_visible (2, false);
+            welcome.set_item_visible (3, false);
 
             //look for dvd
             var disk_manager = DiskManager.get_default ();
             foreach (var volume in disk_manager.get_volumes ()) {
-                welcome.set_item_visible (2, true);
+                welcome.set_item_visible (3, true);
             }
 
             disk_manager.volume_found.connect ((vol) => {
-                welcome.set_item_visible (2, true);
+                welcome.set_item_visible (3, true);
             });
 
             disk_manager.volume_removed.connect ((vol) => {
                 if (disk_manager.get_volumes ().length () <= 0)
-                    welcome.set_item_visible (2, false);
+                    welcome.set_item_visible (3, false);
             });
 
+            
+            
             //handle welcome
             welcome.activated.connect ((index) => {
                 switch (index) {
@@ -361,8 +371,16 @@
                         video_player.playing = false;
                         Idle.add (() => {video_player.progress = settings.last_stopped; return false;});
                         video_player.playing = true;
+                        break;                   
+                    case 2:                        
+                        welcome.hide ();
+                        clutter.show_all ();
+                        open_file (playlist.get_first_item ().get_path ());
+                        video_player.playing = false;
+                        Idle.add (() => {video_player.progress = 0; return false;});
+                        video_player.playing = true;
                         break;
-                    case 2:
+                    case 3:
                         run_open_dvd ();
                         break;
                     default:
@@ -522,11 +540,15 @@
         }
 
         private void on_destroy () {
-            if (video_player.uri == null || video_player.uri == "" || video_player.uri.has_prefix ("dvd://"))
+            if (video_player.uri.has_prefix ("dvd://")) {
+                clear_video_settings ();
                 return;
-            if (!video_player.at_end) {
-                save_last_played_videos ();
             }
+
+            if (video_player.uri == null || video_player.uri == "")
+                return;                
+                
+            save_last_played_videos ();
         }
 
         private int old_h = - 1;
@@ -563,11 +585,22 @@
         private inline void save_last_played_videos () {
             playlist.save_playlist_config ();
 
-            if (settings.current_video != "")
+            debug ("saving settings for: %s", playlist.get_first_item ().get_uri ());
+
+            if (settings.current_video != "" && !video_player.at_end)
                 settings.last_stopped = video_player.progress;
-            else
+            else if (settings.current_video != "" && video_player.at_end) {
+                settings.current_video = playlist.get_first_item ().get_uri ();
                 settings.last_stopped = 0;
-        }
+            }
+        }
+        
+        private inline void clear_video_settings () {
+            settings.last_stopped = 0;
+            settings.last_played_videos = null;
+            settings.current_video = "";
+        }
+        
 
         private void restore_playlist () {
             foreach (var filename in settings.last_played_videos) {
@@ -593,13 +626,20 @@
 
             file.set_current_folder (settings.last_folder);
             if (file.run () == Gtk.ResponseType.ACCEPT) {
-                welcome.hide ();
-                clutter.show_all ();
-
-                playlist.add_item (file.get_file ());
-                if (video_player.uri == null)
+                if (welcome.is_visible ()) {
+                    playlist.clear_items ();
+                }
+                
+                foreach (File item in file.get_files ()) {
+                    playlist.add_item (item);
+                }
+                
+                if (video_player.uri == null || welcome.is_visible ())
                     open_file (file.get_uri ());
-
+                    
+                welcome.hide ();
+                clutter.show_all ();
+                
                 settings.last_folder = file.get_current_folder ();
             }
 
@@ -724,13 +764,16 @@
                 && settings.last_played_videos.length > 0 
                 && settings.current_video != ""
                 && file_exists (settings.current_video)) {
-                welcome.hide ();
-                clutter.show_all ();
                 restore_playlist ();
-                open_file (settings.current_video);
-                video_player.playing = false;
-                Idle.add (() => {video_player.progress = settings.last_stopped; return false;});
-                video_player.playing = true;
+                
+                if (settings.last_stopped > 0) {
+                    welcome.hide ();
+                    clutter.show_all ();
+                    open_file (settings.current_video);
+                    video_player.playing = false;
+                    Idle.add (() => {video_player.progress = settings.last_stopped; return false;});
+                    video_player.playing = true;
+                }
             }
         }
 

=== modified file 'src/Widgets/Playlist.vala'
--- src/Widgets/Playlist.vala	2014-08-09 01:13:20 +0000
+++ src/Widgets/Playlist.vala	2014-09-26 20:46:20 +0000
@@ -67,14 +67,17 @@
             });
         }
 
-        public void next () {
+        public bool next () {
             Gtk.TreeIter iter;
             if (playlist.get_iter_from_string (out iter, (this.current + 1).to_string ())){
                 string filename;
                 playlist.get (iter, Columns.FILENAME, out filename);
                 current++;
                 play (File.new_for_commandline_arg (filename));
+                return true;
             }
+            current = 0;
+            return false;
         }
 
         public void previous () {
@@ -131,6 +134,11 @@
                 return false;
             });
         }
+        
+        public void clear_items () {
+            current = 0;
+            playlist.clear ();
+        }
 
         public File? get_first_item () {
             Gtk.TreeIter iter;
@@ -165,7 +173,7 @@
             playlist.set (new_iter, Columns.PLAYING, new ThemedIcon ("media-playback-start-symbolic"));
 
             this.current = current_played;
-            
+
         }
 
         public List<string> get_all_items () {

=== modified file 'src/Widgets/VideoPlayer.vala'
--- src/Widgets/VideoPlayer.vala	2014-09-01 10:41:18 +0000
+++ src/Widgets/VideoPlayer.vala	2014-09-26 20:46:20 +0000
@@ -199,8 +199,10 @@
             });
 
             playbin.about_to_finish.connect (() => {
-                at_end = true;
-                ended ();
+                if (!at_end) {
+                    at_end = true;
+                    ended ();
+                }
             });
 
             playbin.text_tags_changed.connect ((el) => {
@@ -442,8 +444,7 @@
             }
         }
 
-        public void seek_jump_seconds (int seconds)
-        {
+        public void seek_jump_seconds (int seconds) {
             int64 position;
             playbin.query_position (Gst.Format.TIME, out position);
 
@@ -458,4 +459,4 @@
             playbin.seek_simple (Gst.Format.TIME, Gst.SeekFlags.FLUSH | Gst.SeekFlags.ACCURATE, new_position);
         }
     }
-}
+}
\ No newline at end of file