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
This commit is contained in:
Nirbheek Chauhan 2015-08-26 15:40:16 +05:30 committed by Sebastian Dröge
parent 708936de49
commit f12ef34410
2 changed files with 52 additions and 17 deletions

View file

@ -350,6 +350,26 @@ is_rectangle_contained (GstVideoRectangle rect1, GstVideoRectangle rect2)
return FALSE; 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 static gboolean
gst_compositor_pad_prepare_frame (GstVideoAggregatorPad * pad, gst_compositor_pad_prepare_frame (GstVideoAggregatorPad * pad,
GstVideoAggregator * vagg) GstVideoAggregator * vagg)
@ -451,15 +471,21 @@ gst_compositor_pad_prepare_frame (GstVideoAggregatorPad * pad,
g_free (wanted_colorimetry); g_free (wanted_colorimetry);
} }
/* Clamp the x/y coordinates of this frame to the video boundaries to cover if (cpad->alpha == 0.0) {
* the case where (say, with negative xpos/ypos) the non-obscured portion of GST_DEBUG_OBJECT (vagg, "Pad has alpha 0.0, not converting frame");
* the frame could be outside the bounds of the video itself and hence not converted_frame = NULL;
* visible at all */ goto done;
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 = clamp_rectangle (cpad->xpos, cpad->ypos, width, height,
frame_rect.w = MAX (width - frame_rect.x, 0); GST_VIDEO_INFO_WIDTH (&vagg->info), GST_VIDEO_INFO_HEIGHT (&vagg->info));
frame_rect.h = MAX (height - frame_rect.y, 0);
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); GST_OBJECT_LOCK (vagg);
/* Check if this frame is obscured by a higher-zorder frame /* 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) && !GST_VIDEO_INFO_HAS_ALPHA (&pad2->info) &&
is_rectangle_contained (frame_rect, frame2_rect)) { is_rectangle_contained (frame_rect, frame2_rect)) {
frame_obscured = TRUE; frame_obscured = TRUE;
GST_DEBUG_OBJECT (pad, "Obscured by %s, skipping frame", GST_DEBUG_OBJECT (pad, "%ix%i@(%i,%i) obscured by %s %ix%i@(%i,%i) "
GST_PAD_NAME (pad2)); "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; break;
} }
} }
@ -500,12 +530,6 @@ gst_compositor_pad_prepare_frame (GstVideoAggregatorPad * pad,
goto done; 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); frame = g_slice_new0 (GstVideoFrame);
if (!gst_video_frame_map (frame, &pad->buffer_vinfo, pad->buffer, if (!gst_video_frame_map (frame, &pad->buffer_vinfo, pad->buffer,

View file

@ -1315,6 +1315,17 @@ GST_START_TEST (test_obscured_skipped)
xpos0 = ypos0 = xpos1 = ypos1 = 0; xpos0 = ypos0 = xpos1 = ypos1 = 0;
buffer_mapped = FALSE; 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; xpos0 = ypos0 = 10000;
out_width = 320; out_width = 320;
out_height = 240; out_height = 240;