[Bug 699077] videorate: Add a property to modify clip speed

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=699077
  GStreamer | gst-plugins-base | 1.x

Sebastian Dröge (slomo) <slomo> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #249441|none                        |needs-work
             status|                            |

--- Comment #20 from Sebastian Dröge (slomo) <slomo at circular-chaos.org> 2013-07-18 10:09:13 UTC ---
Review of attachment 249441:
 --> (https://bugzilla.gnome.org/review?bug=699077&attachment=249441)

::: gst/videorate/gstvideorate.c
@@ +747,3 @@
+        segment.stop = (gint64) (segment.stop / videorate->rate);
+      segment.time = (gint64) (segment.time / videorate->rate);
+      segment.applied_rate = segment.applied_rate / videorate->rate;

I think applied rate should be multiplied, not divided. If rate is two, you
want applied rate to be two times higher.

@@ +904,3 @@
+          duration);
+      if (GST_CLOCK_TIME_IS_VALID (duration)) {
+        duration = (gint64) (duration / videorate->rate);

While this is what pitch does, I think what would be better is to consider rate
changes here. So take the upstream length, subtract the last rate change
position, scale by the current rate. Then add that to the last rate change
position.

@@ +1063,3 @@
   intime = in_ts + videorate->segment.base;

+  /* We have a new rate, which means prevbuf and inbuf don't have the same
rate */

I don't understand this code. Can't you just remember the current position
(including the previous offset) as offset, and from that on start calculations
again from zero (and offset them by the current position)? This looks overly
complicated.

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.