From 5720ae4f254d88e520d00a9a22c54c98ad19ba82 Mon Sep 17 00:00:00 2001 From: Thibault Saunier Date: Thu, 16 Apr 2020 20:27:30 -0400 Subject: [PATCH] framepositioner: Fix some source repositionning rounding issues Avoid loosing (too much) precision when rescaling back and forth by storing values in gdoubles. Handle the fact that position values can be negative Also fix debug category static variable as it clashes with the instance variable name in a few methods. --- ges/gstframepositioner.c | 44 +++++++++---------- ges/gstframepositioner.h | 8 ++-- meson.build | 3 +- ...eck_video_track_restriction_scale.scenario | 16 +++++++ 4 files changed, 44 insertions(+), 27 deletions(-) diff --git a/ges/gstframepositioner.c b/ges/gstframepositioner.c index 8fd2033801..9255c09e56 100644 --- a/ges/gstframepositioner.c +++ b/ges/gstframepositioner.c @@ -27,9 +27,9 @@ #include "gstframepositioner.h" -GST_DEBUG_CATEGORY_STATIC (framepositioner); +GST_DEBUG_CATEGORY_STATIC (_framepositioner); #undef GST_CAT_DEFAULT -#define GST_CAT_DEFAULT framepositioner +#define GST_CAT_DEFAULT _framepositioner /* We need to define a max number of pixel so we can interpolate them */ #define MAX_PIXELS 100000 @@ -115,7 +115,7 @@ is_user_positionned (GstFramePositioner * self) static gboolean auto_position (GstFramePositioner * self) { - gint scaled_width = -1, scaled_height = -1, x, y; + gdouble scaled_width = -1, scaled_height = -1, x, y; if (is_user_positionned (self)) { GST_DEBUG_OBJECT (self, "Was positioned by the user, not auto positioning"); @@ -140,13 +140,11 @@ auto_position (GstFramePositioner * self) self->natural_height); } - x = MAX (0, gst_util_uint64_scale_int_round (1, - self->track_width - scaled_width, 2)); - y = MAX (0, gst_util_uint64_scale_int_round (1, - self->track_height - scaled_height, 2)); + x = MAX (0, (self->track_width - scaled_width) / 2.f); + y = MAX (0, (self->track_height - scaled_height) / 2.f); GST_INFO_OBJECT (self, "Scalling video to match track size from " - "%dx%d to %dx%d", + "%dx%d to %fx%f", self->natural_width, self->natural_height, scaled_width, scaled_height); self->width = scaled_width; self->height = scaled_height; @@ -158,7 +156,7 @@ auto_position (GstFramePositioner * self) typedef struct { - gint *value; + gdouble *value; gint old_track_value; gint track_value; GParamSpec *pspec; @@ -186,8 +184,8 @@ reposition_properties (GstFramePositioner * pos, gint old_track_width, GstControlBinding *binding = gst_object_get_control_binding (GST_OBJECT (pos), d.pspec->name); - *(d.value) = gst_util_uint64_scale_int (*(d.value), d.track_value, - d.old_track_value); + *(d.value) = + *(d.value) * (gdouble) d.track_value / (gdouble) d.old_track_value; if (!binding) continue; @@ -432,7 +430,7 @@ gst_frame_positioner_class_init (GstFramePositionerClass * klass) GstBaseTransformClass *base_transform_class = GST_BASE_TRANSFORM_CLASS (klass); - GST_DEBUG_CATEGORY_INIT (framepositioner, "framepositioner", + GST_DEBUG_CATEGORY_INIT (_framepositioner, "framepositioner", GST_DEBUG_FG_YELLOW, "ges frame positioner"); gst_element_class_add_static_pad_template (GST_ELEMENT_CLASS (klass), @@ -591,27 +589,29 @@ gst_frame_positioner_get_property (GObject * object, guint property_id, g_value_set_double (value, pos->alpha); break; case PROP_POSX: - g_value_set_int (value, pos->posx); + g_value_set_int (value, round (pos->posx)); break; case PROP_POSY: - g_value_set_int (value, pos->posy); + g_value_set_int (value, round (pos->posy)); break; case PROP_ZORDER: g_value_set_uint (value, pos->zorder); break; case PROP_WIDTH: if (pos->scale_in_compositor) { - g_value_set_int (value, pos->width); + g_value_set_int (value, round (pos->width)); } else { - real_width = pos->width > 0 ? pos->width : pos->track_width; + real_width = + pos->width > 0 ? round (pos->width) : round (pos->track_width); g_value_set_int (value, real_width); } break; case PROP_HEIGHT: if (pos->scale_in_compositor) { - g_value_set_int (value, pos->height); + g_value_set_int (value, round (pos->height)); } else { - real_height = pos->height > 0 ? pos->height : pos->track_height; + real_height = + pos->height > 0 ? round (pos->height) : round (pos->track_height); g_value_set_int (value, real_height); } break; @@ -707,10 +707,10 @@ gst_frame_positioner_transform_ip (GstBaseTransform * trans, GstBuffer * buf) GST_OBJECT_LOCK (framepositioner); meta->alpha = framepositioner->alpha; - meta->posx = framepositioner->posx; - meta->posy = framepositioner->posy; - meta->width = framepositioner->width; - meta->height = framepositioner->height; + meta->posx = round (framepositioner->posx); + meta->posy = round (framepositioner->posy); + meta->width = round (framepositioner->width); + meta->height = round (framepositioner->height); meta->zorder = framepositioner->zorder; GST_OBJECT_UNLOCK (framepositioner); diff --git a/ges/gstframepositioner.h b/ges/gstframepositioner.h index 9f46835987..ff78e3b5b8 100644 --- a/ges/gstframepositioner.h +++ b/ges/gstframepositioner.h @@ -47,12 +47,12 @@ struct _GstFramePositioner gboolean scale_in_compositor; gdouble alpha; - gint posx; - gint posy; + gdouble posx; + gdouble posy; guint zorder; - gint width; + gdouble width; + gdouble height; gint natural_width; - gint height; gint natural_height; gint track_width; gint track_height; diff --git a/meson.build b/meson.build index d1d4af9bbd..147afef4ea 100644 --- a/meson.build +++ b/meson.build @@ -29,6 +29,7 @@ glib_req = '>= 2.44.0' gst_req = '>= @0@.@1@.0'.format(gst_version_major, gst_version_minor) cc = meson.get_compiler('c') +mathlib = cc.find_library('m', required : false) cdata = configuration_data() @@ -103,7 +104,7 @@ cdata.set('DISABLE_XPTV', not libxml_dep.found()) # gtk_dep = dependency('gtk+-3.0', required : false) libges_deps = [gst_dep, gstbase_dep, gstvideo_dep, gstpbutils_dep, - gstcontroller_dep, gio_dep, libxml_dep] + gstcontroller_dep, gio_dep, libxml_dep, mathlib] if gstvalidate_dep.found() libges_deps = libges_deps + [gstvalidate_dep] diff --git a/tests/check/scenarios/check_video_track_restriction_scale.scenario b/tests/check/scenarios/check_video_track_restriction_scale.scenario index 9be26092c3..9ec1722667 100644 --- a/tests/check/scenarios/check_video_track_restriction_scale.scenario +++ b/tests/check/scenarios/check_video_track_restriction_scale.scenario @@ -39,4 +39,20 @@ check-child-properties, element-name=clip, width=1280, height=720, posx=320, pos set-track-restriction-caps, track-type=video, caps="video/x-raw,width=960,height=540" check-child-properties, element-name=clip, width=640, height=360, posx=160, posy=120 +set-track-restriction-caps, track-type=video, caps="video/x-raw,width=1280,height=720" +set-child-properties, element-name=clip, width=128, height=72, posx=-100, posy=-100 +check-child-properties, element-name=clip, width=128, height=72, posx=-100, posy=-100 + +set-track-restriction-caps, track-type=video, caps="video/x-raw,width=1920,height=1080" +check-child-properties, element-name=clip, width=192, height=108, posx=-150, posy=-150 + +set-track-restriction-caps, track-type=video, caps="video/x-raw,width=192,height=108" +check-child-properties, element-name=clip, width=19, height=11, posx=-15, posy=-15 + +set-child-properties, element-name=clip, posx=10, posy=-10 + +# Make sure we do not lose precision when going back to previous size +set-track-restriction-caps, track-type=video, caps="video/x-raw,width=1920,height=1080" +check-child-properties, element-name=clip, width=192, height=108, posx=100, posy=-100 + stop \ No newline at end of file