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.

diff --git a/ges/gstframepositioner.c b/ges/gstframepositioner.c
index 8fd2033..9255c09 100644 (file)
--- 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 9f46835..ff78e3b 100644 (file)
--- 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 d1d4af9..147afef 100644 (file)
--- 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 9be2609..9ec1722 100644 (file)
--- 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