[Bug 703093] videomeasure: port to 1.0

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=703093
  GStreamer | gst-plugins-bad | git

Sebastian Dröge <slomo> changed:

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

--- Comment #3 from Sebastian Dröge <slomo at circular-chaos.org> 2013-07-09 12:30:42 UTC ---
Review of attachment 247788:
 --> (https://bugzilla.gnome.org/review?bug=703093&attachment=247788)

Stopped review at some point, but the negotiation here looks rather complicated

::: gst/videomeasure/gstvideomeasure_collector.c
@@ +345,3 @@
+  gst_element_class_set_static_metadata (element_class,
+      "Video rate adjuster", "Filter/Effect/Video",
+      "Drops/duplicates/adjusts timestamps on video frames to make a perfect
stream",

Copy & paste from videorate it seems, can you fix that while you're at it? ;)

::: gst/videomeasure/gstvideomeasure_ssim.c
@@ +227,2 @@
+static gboolean
+gst_ssim_sink_getcaps (GstPad *pad, GstObject *parent, GstQuery *query)

For consistency, maybe make the signature GstCaps * getcaps (GstSSim *ssim,
GstCaps *filter);

@@ +243,3 @@
   GST_OBJECT_UNLOCK (ssim);

+  if (filter && caps) {

caps should always be != NULL here

@@ -431,3 @@
             for (ix = winstart_x; ix <= winend_x; ix++) {
               weight = ssim->weights[weight_offset + ix];
-              mu_o += weight * org[pixel_offset + ix];

Why do you change the algorithm, should be a separate commit

@@ +515,3 @@
               tmp1 = org_with_offset[ix] - mu_o;
               tmp2 = mod_with_offset[ix] - mu_m;
+              weight = weights_with_offset[ix];

Same here

@@ +530,3 @@
       tmp1 = (2 * mu_o * mu_m + ssim->const1) * (2 * sigma_om + ssim->const2)
/
           ((mu_o * mu_o + mu_m * mu_m + ssim->const1) *
+          (sigma_o + sigma_m + ssim->const2)); /* (sigma_o^2 + sigma_m^2 +
ssim->const2) */

And here

@@ +613,2 @@
     GST_DEBUG_OBJECT (ssim, "checking caps on pad %p", otherpad);
     if (direction == GST_PAD_SINK) {

Caps are only set in one direction nowadays: downstream

@@ +626,2 @@
       GST_DEBUG_OBJECT (ssim, "old caps on pad %p,%s were %s", otherpad,
+          padname, capstr);

Just use GST_PTR_FORMAT here to print caps. This will evne work on windows with
1.1

@@ +658,3 @@
+  ssim->height = height;
+  ssim->frame_rate = fps_n;
+  ssim->frame_rate_base = fps_d;

Maybe just store the GstVideoInfo in the instance struct

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