compositor: Actually use the output resolution for clamping

The obscured check in compositor was using the dimensions of the pad to clamp
the h/w of the pad instead of the output resolution, and was doing an incorrect
calculation to do so. Fix that by simplifying the whole calculation by using
corner coordinates. Also add a test for this bug which fell through the cracks,
and just skip all the obscured tests if the pad's alpha is 0.0.

https://bugzilla.gnome.org/show_bug.cgi?id=754107

diff --git a/gst/compositor/compositor.c b/gst/compositor/compositor.c
index 2e8ab5c..9aa651f 100644 (file)
--- a/gst/compositor/compositor.c
+++ b/gst/compositor/compositor.c
@@ -350,6 +350,26 @@ is_rectangle_contained (GstVideoRectangle rect1, GstVideoRectangle rect2)
   return FALSE;
 }
 
+static GstVideoRectangle
+clamp_rectangle (guint x, guint y, guint w, guint h, guint outer_width,
+    guint outer_height)
+{
+  guint x2 = x + w;
+  guint y2 = y + h;
+  GstVideoRectangle clamped;
+
+  /* Clamp the x/y coordinates of this frame to the output boundaries to cover
+   * the case where (say, with negative xpos/ypos or w/h greater than the output
+   * size) the non-obscured portion of the frame could be outside the bounds of
+   * the video itself and hence not visible at all */
+  clamped.x = CLAMP (x, 0, outer_width);
+  clamped.y = CLAMP (y, 0, outer_height);
+  clamped.w = CLAMP (x2, 0, outer_width) - clamped.x;
+  clamped.h = CLAMP (y2, 0, outer_height) - clamped.y;
+
+  return clamped;
+}
+
 static gboolean
 gst_compositor_pad_prepare_frame (GstVideoAggregatorPad * pad,
     GstVideoAggregator * vagg)
@@ -451,15 +471,21 @@ gst_compositor_pad_prepare_frame (GstVideoAggregatorPad * pad,
     g_free (wanted_colorimetry);
   }
 
-  /* Clamp the x/y coordinates of this frame to the video boundaries to cover
-   * the case where (say, with negative xpos/ypos) the non-obscured portion of
-   * the frame could be outside the bounds of the video itself and hence not
-   * visible at all */
-  frame_rect.x = CLAMP (cpad->xpos, 0, GST_VIDEO_INFO_WIDTH (&vagg->info));
-  frame_rect.y = CLAMP (cpad->ypos, 0, GST_VIDEO_INFO_HEIGHT (&vagg->info));
-  /* Clamp the width/height to the frame boundaries as well */
-  frame_rect.w = MAX (width - frame_rect.x, 0);
-  frame_rect.h = MAX (height - frame_rect.y, 0);
+  if (cpad->alpha == 0.0) {
+    GST_DEBUG_OBJECT (vagg, "Pad has alpha 0.0, not converting frame");
+    converted_frame = NULL;
+    goto done;
+  }
+
+  frame_rect = clamp_rectangle (cpad->xpos, cpad->ypos, width, height,
+      GST_VIDEO_INFO_WIDTH (&vagg->info), GST_VIDEO_INFO_HEIGHT (&vagg->info));
+
+  if (frame_rect.w == 0 || frame_rect.h == 0) {
+    GST_DEBUG_OBJECT (vagg, "Resulting frame is zero-width or zero-height "
+        "(w: %i, h: %i), skipping", frame_rect.w, frame_rect.h);
+    converted_frame = NULL;
+    goto done;
+  }
 
   GST_OBJECT_LOCK (vagg);
   /* Check if this frame is obscured by a higher-zorder frame
@@ -488,8 +514,12 @@ gst_compositor_pad_prepare_frame (GstVideoAggregatorPad * pad,
         !GST_VIDEO_INFO_HAS_ALPHA (&pad2->info) &&
         is_rectangle_contained (frame_rect, frame2_rect)) {
       frame_obscured = TRUE;
-      GST_DEBUG_OBJECT (pad, "Obscured by %s, skipping frame",
-          GST_PAD_NAME (pad2));
+      GST_DEBUG_OBJECT (pad, "%ix%i@(%i,%i) obscured by %s %ix%i@(%i,%i) "
+          "in output of size %ix%i; skipping frame", frame_rect.w, frame_rect.h,
+          frame_rect.x, frame_rect.y, GST_PAD_NAME (pad2), frame2_rect.w,
+          frame2_rect.h, frame2_rect.x, frame2_rect.y,
+          GST_VIDEO_INFO_WIDTH (&vagg->info),
+          GST_VIDEO_INFO_HEIGHT (&vagg->info));
       break;
     }
   }
@@ -500,12 +530,6 @@ gst_compositor_pad_prepare_frame (GstVideoAggregatorPad * pad,
     goto done;
   }
 
-  if (cpad->alpha == 0.0) {
-    GST_DEBUG_OBJECT (vagg, "Pad has alpha 0.0, not converting frame");
-    converted_frame = NULL;
-    goto done;
-  }
-
   frame = g_slice_new0 (GstVideoFrame);
 
   if (!gst_video_frame_map (frame, &pad->buffer_vinfo, pad->buffer,

diff --git a/tests/check/elements/compositor.c b/tests/check/elements/compositor.c
index a949d38..dc1a58f 100644 (file)
--- a/tests/check/elements/compositor.c
+++ b/tests/check/elements/compositor.c
@@ -1315,6 +1315,17 @@ GST_START_TEST (test_obscured_skipped)
   xpos0 = ypos0 = xpos1 = ypos1 = 0;
   buffer_mapped = FALSE;
 
+  xpos1 = ypos1 = 0;
+  xpos0 = ypos0 = width0 = height0 = width1 = height1 = 10;
+  out_width = out_height = 20;
+  GST_INFO ("testing bug 754107");
+  _test_obscured (caps_str, xpos0, ypos0, width0, height0, alpha0, xpos1, ypos1,
+      width1, height1, alpha1, out_width, out_height);
+  fail_unless (buffer_mapped == TRUE);
+  xpos0 = ypos0 = xpos1 = ypos1 = width0 = height0 = width1 = height1 = 0;
+  out_width = out_height = 0;
+  buffer_mapped = FALSE;
+
   xpos0 = ypos0 = 10000;
   out_width = 320;
   out_height = 240;