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

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

Please, check my fixes...

Thx

it looks good to me

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

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.

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

> 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?

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

* 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.

> * 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

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.

> 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.

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

> 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.

Follow my steps above, setting will get cleared.

> 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?

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.

Sorry I meant

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

Hey guys, please check my changes.

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