Merge lp:~shockone89/audience/rewinding into lp:~audience-members/audience/trunk

Proposed by Volodymyr Shatsky on 2014-05-04
Status: Rejected
Rejected by: Cody Garver on 2014-08-30
Proposed branch: lp:~shockone89/audience/rewinding
Merge into: lp:~audience-members/audience/trunk
Diff against target: 79 lines (+34/-7)
3 files modified
src/Audience.vala (+15/-5)
src/Consts.vala (+2/-1)
src/Widgets/VideoPlayer.vala (+17/-1)
To merge this branch: bzr merge lp:~shockone89/audience/rewinding
Reviewer Review Type Date Requested Status
Cody Garver Disapprove on 2014-08-30
Robert Roth (community) 2014-05-04 Needs Fixing on 2014-06-23
Review via email: mp+218217@code.launchpad.net
To post a comment you must log in.
Robert Roth (evfool) wrote :

The changes look fine, some comments though
* with these changes, the jump intervals are hardcoded to 5 seconds and 60 seconds, instead of 5% of the total length. Is this something we want? In a 3-hour movie, 60 seconds jumps are rather short. Maybe short jump could work with jump 5-10 seconds, long jump could jump 5-10% of the total length.

review: Needs Fixing
Robert Roth (evfool) wrote :

Additionally, see my inline comments on the diff for what could be fixed.

review: Needs Fixing
Cody Garver (codygarver) wrote :

I just did a clean merge of the branch this code came from

review: Disapprove

Unmerged revisions

337. By Volodymyr Shatsky on 2014-05-04

Short and long rewinding.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/Audience.vala'
2--- src/Audience.vala 2013-11-24 08:10:51 +0000
3+++ src/Audience.vala 2014-05-04 14:17:09 +0000
4@@ -243,13 +243,18 @@
5 mainwindow.destroy ();
6 break;
7 case Gdk.Key.Left:
8- if ((video_player.progress - 0.05) < 0)
9- video_player.progress = 0.0;
10- else
11- video_player.progress -= 0.05;
12+ if (modifier_is_pressed (e, Gdk.ModifierType.SHIFT_MASK)) {
13+ video_player.rewind_seconds (-REWIND_LONG);
14+ } else {
15+ video_player.rewind_seconds (-REWIND_SHORT);
16+ }
17 break;
18 case Gdk.Key.Right:
19- video_player.progress += 0.05;
20+ if (modifier_is_pressed (e, Gdk.ModifierType.SHIFT_MASK)) {
21+ video_player.rewind_seconds (REWIND_LONG);
22+ } else {
23+ video_player.rewind_seconds (REWIND_SHORT);
24+ }
25 break;
26 case Gdk.Key.a:
27 next_audio ();
28@@ -597,6 +602,11 @@
29 }
30 }
31
32+ private bool modifier_is_pressed (Gdk.EventKey event, Gdk.ModifierType modifier)
33+ {
34+ return (event.state & modifier) == modifier;
35+ }
36+
37 internal void open_file (string filename, bool dont_modify=false)
38 {
39 var file = File.new_for_commandline_arg (filename);
40
41=== modified file 'src/Consts.vala'
42--- src/Consts.vala 2013-03-31 21:46:37 +0000
43+++ src/Consts.vala 2014-05-04 14:17:09 +0000
44@@ -2,5 +2,6 @@
45 namespace Audience {
46
47 public const int CONTROLS_HEIGHT = 32;
48-
49+ public const int REWIND_SHORT = 5;
50+ public const int REWIND_LONG = 60;
51 }
52
53=== modified file 'src/Widgets/VideoPlayer.vala'
54--- src/Widgets/VideoPlayer.vala 2014-04-28 12:58:09 +0000
55+++ src/Widgets/VideoPlayer.vala 2014-05-04 14:17:09 +0000
56@@ -592,5 +592,21 @@
57 }
58 } catch (Error e) { warning (e.message); }
59 }
60+
61+ public void rewind_seconds (int seconds)
62+ {
63+ int64 position;
64+ playbin.query_position (Gst.Format.TIME, out position);
65+
66+ var gst_seconds = 1000000000 * (int64)seconds;
67+ var new_position = position + gst_seconds;
68+
69+ if (new_position < 0) {
70+ playbin.seek_simple (Gst.Format.TIME, Gst.SeekFlags.FLUSH | Gst.SeekFlags.ACCURATE, 1);
71+ return;
72+ }
73+
74+ playbin.seek_simple (Gst.Format.TIME, Gst.SeekFlags.FLUSH | Gst.SeekFlags.ACCURATE, new_position);
75+ }
76 }
77-}
78\ No newline at end of file
79+}

Subscribers

People subscribed via source and target branches