From 53637ad74972f0ca3cee48feeca6003ed49a734c Mon Sep 17 00:00:00 2001 From: Thibault Saunier Date: Wed, 19 Feb 2020 18:09:19 -0300 Subject: [PATCH] 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 --- ges/ges-video-source.c | 30 ++++++- ges/ges-video-uri-source.c | 14 +++- ges/gstframepositioner.c | 81 ++++++++++++++++--- ges/gstframepositioner.h | 4 + ...eck_video_track_restriction_scale.scenario | 4 +- 5 files changed, 116 insertions(+), 17 deletions(-) diff --git a/ges/ges-video-source.c b/ges/ges-video-source.c index 3ba996c9d5..7708a91d8b 100644 --- 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 b10b45814b..1d10c2912c 100644 --- 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 2756d0ddd8..f853372006 100644 --- 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 9b1884f73b..9f46835987 100644 --- 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 97e40d5ad7..3ffc1c938c 100644 --- 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