[Bug 734424] videorate produces bogus output when framerate=0/1

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=734424
  GStreamer | gst-plugins-base | 1.4.0

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

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

--- Comment #18 from Sebastian Dröge (slomo) <slomo at coaxion.net> 2014-12-14 10:56:34 UTC ---
Review of attachment 292612:
 --> (https://bugzilla.gnome.org/review?bug=734424&attachment=292612)

::: gst/videorate/gstvideorate.c
@@ +922,3 @@

+static void
+gst_video_rate_check_variable_rate (GstVideoRate * videorate,

Please add some comments here what this function tries to do :)

@@ +932,3 @@
+
+  g_object_get (GST_BASE_TRANSFORM_SRC_PAD (videorate), "caps", &srcpadcaps,
+      NULL);

Why not just gst_pad_get_current_caps()?

@@ +933,3 @@
+  g_object_get (GST_BASE_TRANSFORM_SRC_PAD (videorate), "caps", &srcpadcaps,
+      NULL);
+  gst_video_guess_framerate (GST_BUFFER_PTS (buffer), &fps_n, &fps_d);

This function expects a buffer duration, not a pts. But even if you use a
duration here, what will happen if that duration was inaccurate or the average
duration of frames changes over time?

@@ +941,3 @@
+  pad = gst_pad_get_peer (GST_BASE_TRANSFORM_SRC_PAD (videorate));
+  if (pad == NULL)
+    pad = gst_object_ref (GST_BASE_TRANSFORM_SRC_PAD (videorate));

If there is no peer, why not just go out of here immediately? Checking the
videorate srcpad if it accepts the caps seems useless.

@@ +958,3 @@
+
+  gst_pad_push_event (GST_BASE_TRANSFORM_SRC_PAD (videorate),
+      gst_event_new_caps (tmpcaps));

This seems potentially problematic as it bypasses the basetransform caps
handling. Maybe should be gst_base_transform_reconfigure_src()?

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