ges: Properly position video sources in the scene by default

We try to do our best to have the video frames scaled the best way
to fill most space on the final frames, keeping aspect ratio. The user
can later on rescale or move the sources as usual but it makes the
default behaviour a better and more natural especially now that we
set default restriction caps to the video tracks.

And fix the unit test to take that change into account

diff --git a/ges/ges-video-source.c b/ges/ges-video-source.c
index 3ba996c9d5df7d3c5964d7b9c0b10c1569144336..7708a91d8b399f87f7ab1baca950b94fdeef801d 100644 (file)
--- a/ges/ges-video-source.c
+++ b/ges/ges-video-source.c
@@ -27,7 +27,7 @@
  *
  * You can use the following children properties through the
  * #ges_track_element_set_child_property and alike set of methods:
- * 
+ *
  * - #gdouble `alpha`: The desired alpha for the stream.
  * - #gint `posx`: The desired x position for the stream.
  * - #gint `posy`: The desired y position for the stream
@@ -53,6 +53,7 @@
 #include "ges-video-source.h"
 #include "ges-layer.h"
 #include "gstframepositioner.h"
+#include "ges-extractable.h"
 
 #define parent_class ges_video_source_parent_class
 
@@ -62,8 +63,26 @@ struct _GESVideoSourcePrivate
   GstElement *capsfilter;
 };
 
-G_DEFINE_ABSTRACT_TYPE_WITH_PRIVATE (GESVideoSource, ges_video_source,
-    GES_TYPE_SOURCE);
+static void
+ges_video_source_set_asset (GESExtractable * extractable, GESAsset * asset)
+{
+  GESVideoSource *self = GES_VIDEO_SOURCE (extractable);
+
+  ges_video_source_get_natural_size (self,
+      &self->priv->positioner->natural_width,
+      &self->priv->positioner->natural_height);
+}
+
+static void
+ges_extractable_interface_init (GESExtractableInterface * iface)
+{
+  iface->set_asset = ges_video_source_set_asset;
+}
+
+G_DEFINE_ABSTRACT_TYPE_WITH_CODE (GESVideoSource, ges_video_source,
+    GES_TYPE_SOURCE, G_ADD_PRIVATE (GESVideoSource)
+    G_IMPLEMENT_INTERFACE (GES_TYPE_EXTRACTABLE,
+        ges_extractable_interface_init));
 
 /* TrackElement VMethods */
 
@@ -169,6 +188,10 @@ ges_video_source_create_element (GESTrackElement * trksrc)
   self->priv->positioner = GST_FRAME_POSITIONNER (positioner);
   self->priv->positioner->scale_in_compositor =
       !GES_VIDEO_SOURCE_GET_CLASS (self)->ABI.abi.disable_scale_in_compositor;
+  ges_video_source_get_natural_size (self,
+      &self->priv->positioner->natural_width,
+      &self->priv->positioner->natural_height);
+
   self->priv->capsfilter = capsfilter;
 
   return topbin;
@@ -226,7 +249,6 @@ ges_video_source_init (GESVideoSource * self)
   self->priv = ges_video_source_get_instance_private (self);
   self->priv->positioner = NULL;
   self->priv->capsfilter = NULL;
-
 }
 
 /**

diff --git a/ges/ges-video-uri-source.c b/ges/ges-video-uri-source.c
index b10b45814b4941dfc0fca8644b409e25cf96dba8..1d10c2912c2b241e66957cc55e4d6d111f229c58 100644 (file)
--- a/ges/ges-video-uri-source.c
+++ b/ges/ges-video-uri-source.c
@@ -124,7 +124,11 @@ ges_video_uri_source_get_natural_size (GESVideoSource * source, gint * width,
   GstDiscovererStreamInfo *info;
   GESAsset *asset = ges_extractable_get_asset (GES_EXTRACTABLE (source));
 
-  g_assert (GES_IS_URI_SOURCE_ASSET (asset));
+  if (!asset) {
+    GST_DEBUG_OBJECT (source, "No asset set yet");
+    return FALSE;
+  }
+
   info = ges_uri_source_asset_get_stream_info (GES_URI_SOURCE_ASSET (asset));
 
   if (!GST_IS_DISCOVERER_VIDEO_INFO (info)) {
@@ -173,7 +177,7 @@ done:
   return TRUE;
 
 rotate:
-  GST_INFO_OBJECT (self, "Stream is rotated, taking that into account");
+  GST_INFO_OBJECT (source, "Stream is rotated, taking that into account");
   *width =
       gst_discoverer_video_info_get_height (GST_DISCOVERER_VIDEO_INFO (info));
   *height =
@@ -193,6 +197,7 @@ ges_extractable_check_id (GType type, const gchar * id, GError ** error)
 static void
 extractable_set_asset (GESExtractable * extractable, GESAsset * asset)
 {
+  GESExtractableInterface *piface, *iface;
   /* FIXME That should go into #GESTrackElement, but
    * some work is needed to make sure it works properly */
 
@@ -202,6 +207,11 @@ extractable_set_asset (GESExtractable * extractable, GESAsset * asset)
         ges_track_element_asset_get_track_type (GES_TRACK_ELEMENT_ASSET
             (asset)));
   }
+
+  iface = G_TYPE_INSTANCE_GET_INTERFACE (extractable, GES_TYPE_EXTRACTABLE,
+      GESExtractableInterface);
+  piface = g_type_interface_peek_parent (iface);
+  piface->set_asset (extractable, asset);
 }
 
 static void

diff --git a/ges/gstframepositioner.c b/ges/gstframepositioner.c
index 2756d0ddd8fb21e6a5bc725405b8c8fcc0ecee0f..f8533720061976fc799ee953af81d9c5c489a41c 100644 (file)
--- a/ges/gstframepositioner.c
+++ b/ges/gstframepositioner.c
@@ -81,6 +81,56 @@ _weak_notify_cb (GstFramePositioner * pos, GObject * old)
   pos->current_track = NULL;
 }
 
+static gboolean
+auto_position (GstFramePositioner * self)
+{
+  gint scaled_width = -1, scaled_height = -1, x, y;
+
+  if (self->user_positioned) {
+    GST_DEBUG_OBJECT (self, "Was positioned by the user, not touching anymore");
+    return FALSE;
+  }
+
+  if (!self->natural_width || !self->natural_height)
+    return FALSE;
+
+  if (!self->track_width || !self->track_height) {
+    GST_INFO_OBJECT (self, "Track doesn't have a proper size, not "
+        "positioning the source");
+    return FALSE;
+  }
+
+  if (self->track_width == self->natural_width &&
+      self->track_height == self->natural_height)
+    return TRUE;
+
+  scaled_height =
+      gst_util_uint64_scale_int (self->natural_height, self->track_width,
+      self->natural_width);
+  scaled_width = self->track_width;
+  if (scaled_height > self->track_height) {
+    scaled_height = self->track_height;
+    scaled_width =
+        gst_util_uint64_scale_int (self->natural_width, self->track_height,
+        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));
+
+  GST_INFO_OBJECT (self, "Scalling video to match track size from "
+      "%dx%d to %dx%d",
+      self->natural_width, self->natural_height, scaled_width, scaled_height);
+  self->width = scaled_width;
+  self->height = scaled_height;
+  self->posx = x;
+  self->posy = y;
+
+  return TRUE;
+}
+
 static void
 gst_frame_positioner_update_properties (GstFramePositioner * pos,
     gboolean track_mixing, gint old_track_width, gint old_track_height)
@@ -107,18 +157,22 @@ gst_frame_positioner_update_properties (GstFramePositioner * pos,
     gst_caps_set_simple (caps, "pixel-aspect-ratio", GST_TYPE_FRACTION,
         pos->par_n, pos->par_d, NULL);
 
-  if (old_track_width && pos->width == old_track_width &&
-      old_track_height && pos->height == old_track_height &&
-      pos->track_height && pos->track_width &&
-      ((float) old_track_width / (float) old_track_height) ==
-      ((float) pos->track_width / (float) pos->track_height)) {
+  if (!auto_position (pos)) {
 
-    GST_DEBUG_OBJECT (pos, "Following track size width old_track: %d -- pos: %d"
-        " || height, old_track %d -- pos: %d",
-        old_track_width, pos->width, old_track_height, pos->height);
+    if (old_track_width && pos->width == old_track_width &&
+        old_track_height && pos->height == old_track_height &&
+        pos->track_height && pos->track_width &&
+        ((float) old_track_width / (float) old_track_height) ==
+        ((float) pos->track_width / (float) pos->track_height)) {
 
-    pos->width = pos->track_width;
-    pos->height = pos->track_height;
+      GST_DEBUG_OBJECT (pos,
+          "Following track size width old_track: %d -- pos: %d"
+          " || height, old_track %d -- pos: %d", old_track_width, pos->width,
+          old_track_height, pos->height);
+
+      pos->width = pos->track_width;
+      pos->height = pos->track_height;
+    }
   }
 
   GST_DEBUG_OBJECT (caps, "setting caps");
@@ -263,6 +317,9 @@ gst_frame_positioner_class_init (GstFramePositionerClass * klass)
   GstBaseTransformClass *base_transform_class =
       GST_BASE_TRANSFORM_CLASS (klass);
 
+  GST_DEBUG_CATEGORY_INIT (framepositioner, "framepositioner",
+      GST_DEBUG_FG_YELLOW, "ges frame positioner");
+
   gst_element_class_add_static_pad_template (GST_ELEMENT_CLASS (klass),
       &gst_frame_positioner_src_template);
   gst_element_class_add_static_pad_template (GST_ELEMENT_CLASS (klass),
@@ -379,19 +436,23 @@ gst_frame_positioner_set_property (GObject * object, guint property_id,
       break;
     case PROP_POSX:
       framepositioner->posx = g_value_get_int (value);
+      framepositioner->user_positioned = TRUE;
       break;
     case PROP_POSY:
       framepositioner->posy = g_value_get_int (value);
+      framepositioner->user_positioned = TRUE;
       break;
     case PROP_ZORDER:
       framepositioner->zorder = g_value_get_uint (value);
       break;
     case PROP_WIDTH:
+      framepositioner->user_positioned = TRUE;
       framepositioner->width = g_value_get_int (value);
       gst_frame_positioner_update_properties (framepositioner, track_mixing,
           0, 0);
       break;
     case PROP_HEIGHT:
+      framepositioner->user_positioned = TRUE;
       framepositioner->height = g_value_get_int (value);
       gst_frame_positioner_update_properties (framepositioner, track_mixing,
           0, 0);

diff --git a/ges/gstframepositioner.h b/ges/gstframepositioner.h
index 9b1884f73b4aff1034280006c86b8d5b4086a058..9f4683598706578811ac6fe67e1540138bd9c854 100644 (file)
--- a/ges/gstframepositioner.h
+++ b/ges/gstframepositioner.h
@@ -51,7 +51,9 @@ struct _GstFramePositioner
   gint posy;
   guint zorder;
   gint width;
+  gint natural_width;
   gint height;
+  gint natural_height;
   gint track_width;
   gint track_height;
   gint fps_n;
@@ -60,6 +62,8 @@ struct _GstFramePositioner
   gint par_n;
   gint par_d;
 
+  gboolean user_positioned;
+
   /*  This should never be made public, no padding needed */
 };
 

diff --git a/tests/check/scenarios/check_video_track_restriction_scale.scenario b/tests/check/scenarios/check_video_track_restriction_scale.scenario
index 97e40d5ad7d9dd7eb1e81b503a27a79a82dc2707..3ffc1c938c1d9ccc635219b7e5366ceb57c1125e 100644 (file)
--- a/tests/check/scenarios/check_video_track_restriction_scale.scenario
+++ b/tests/check/scenarios/check_video_track_restriction_scale.scenario
@@ -6,7 +6,9 @@ description, handles-states=true,
 
 add-clip, name=clip, asset-id=GESTestClip, layer-priority=0, type=GESTestClip, duration=1.0
 
-check-child-properties, element-name=clip, width=0, height=0
+# VideoTestSource natural size is 1280x720, so keeping aspect ratio, mean
+# that it will be scaled to 1200x675 (placed at x=0, y=163)
+check-child-properties, element-name=clip, width=1200, height=675, posx=0, posy=163
 
 set-child-properties, element-name=clip, width=1024, height=768
 check-child-properties, element-name=clip, width=1024, height=768