framepositioner: Stop lying about the source size
authorThibault Saunier <tsaunier@igalia.com>
Thu, 20 Feb 2020 15:28:59 +0000 (12:28 -0300)
committerThibault Saunier <tsaunier@igalia.com>
Wed, 26 Feb 2020 16:36:30 +0000 (13:36 -0300)
Basically we were advertising that the source size would be the
size of the track if it hadn't been defined by end user, but since
we started to let scaling happen in the compositor, this is not true
as the source size is now the natural size of the underlying video
stream.

Remove the unit test and reimplemented using a validate scenario which
make the test much simpler to read :=)

ges/gstframepositioner.c
tests/check/ges/timelineedition.c
tests/check/meson.build
tests/check/scenarios/check_video_track_restriction_scale.scenario [new file with mode: 0644]
tools/meson.build

index 1c2fbe9..2756d0d 100644 (file)
 
 #include "gstframepositioner.h"
 
+GST_DEBUG_CATEGORY_STATIC (framepositioner);
+#undef GST_CAT_DEFAULT
+#define GST_CAT_DEFAULT framepositioner
+
 /* We  need to define a max number of pixel so we can interpolate them */
 #define MAX_PIXELS 100000
 #define MIN_PIXELS -100000
@@ -422,12 +426,20 @@ gst_frame_positioner_get_property (GObject * object, guint property_id,
       g_value_set_uint (value, pos->zorder);
       break;
     case PROP_WIDTH:
-      real_width = (pos->width > 0) ? pos->width : pos->track_width;
-      g_value_set_int (value, real_width);
+      if (pos->scale_in_compositor) {
+        g_value_set_int (value, pos->width);
+      } else {
+        real_width = pos->width > 0 ? pos->width : pos->track_width;
+        g_value_set_int (value, real_width);
+      }
       break;
     case PROP_HEIGHT:
-      real_height = (pos->height > 0) ? pos->height : pos->track_height;
-      g_value_set_int (value, real_height);
+      if (pos->scale_in_compositor) {
+        g_value_set_int (value, pos->height);
+      } else {
+        real_height = pos->height > 0 ? pos->height : pos->track_height;
+        g_value_set_int (value, real_height);
+      }
       break;
     default:
       G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec);
index 3cab417..5538687 100644 (file)
@@ -1146,257 +1146,6 @@ GST_START_TEST (test_snapping_groups)
 
 GST_END_TEST;
 
-static void
-_set_track_element_width_height (GESTrackElement * trksrc, gint wvalue,
-    gint hvalue)
-{
-  GValue width = { 0 };
-  GValue height = { 0 };
-
-  g_value_init (&width, G_TYPE_INT);
-  g_value_init (&height, G_TYPE_INT);
-  g_value_set_int (&width, wvalue);
-  g_value_set_int (&height, hvalue);
-  if (wvalue >= 0)
-    ges_timeline_element_set_child_property (GES_TIMELINE_ELEMENT (trksrc),
-        "width", &width);
-  if (hvalue >= 0)
-    ges_timeline_element_set_child_property (GES_TIMELINE_ELEMENT (trksrc),
-        "height", &height);
-}
-
-static gboolean
-check_frame_positioner_size (GESClip * clip, gint width, gint height)
-{
-  GESTrackElement *trksrc;
-  GValue val_width = { 0 };
-  GValue val_height = { 0 };
-  gint real_width, real_height;
-
-  trksrc = GES_CONTAINER_CHILDREN (clip)->data;
-  if (!GES_IS_VIDEO_SOURCE (trksrc))
-    return FALSE;
-
-  g_value_init (&val_width, G_TYPE_INT);
-  g_value_init (&val_height, G_TYPE_INT);
-
-  ges_timeline_element_get_child_property (GES_TIMELINE_ELEMENT (trksrc),
-      "width", &val_width);
-  ges_timeline_element_get_child_property (GES_TIMELINE_ELEMENT (trksrc),
-      "height", &val_height);
-
-  real_width = g_value_get_int (&val_width);
-  real_height = g_value_get_int (&val_height);
-
-  assert_equals_int (real_width, width);
-  assert_equals_int (real_height, height);
-
-  return (width == real_width && height == real_height);
-}
-
-GST_START_TEST (test_scaling)
-{
-  GESTimeline *timeline;
-  GESLayer *layer;
-  GESAsset *asset1, *asset2;
-  GESClip *clip;
-  GESTrack *trackv;
-  GstCaps *caps;
-
-  ges_init ();
-
-  trackv = GES_TRACK (ges_video_track_new ());
-  caps =
-      gst_caps_new_simple ("video/x-raw", "width", G_TYPE_INT, 1200, "height",
-      G_TYPE_INT, 1000, NULL);
-
-  timeline = ges_timeline_new ();
-  ges_timeline_add_track (timeline, trackv);
-  layer = ges_layer_new ();
-  fail_unless (ges_timeline_add_layer (timeline, layer));
-
-  g_object_set (layer, "auto-transition", TRUE, NULL);
-
-  asset1 = GES_ASSET (ges_asset_request (GES_TYPE_TEST_CLIP, NULL, NULL));
-  asset2 = GES_ASSET (ges_asset_request (GES_TYPE_TEST_CLIP, NULL, NULL));
-
-  fail_unless (asset1 != NULL && asset2 != NULL);
-
-  GST_DEBUG_BIN_TO_DOT_FILE_WITH_TS (GST_BIN (timeline),
-      GST_DEBUG_GRAPH_SHOW_ALL, "ges-integration-timeline");
-
-  ges_track_set_restriction_caps (trackv, caps);
-  gst_caps_unref (caps);
-
-  GST_DEBUG ("adding clip, should be 1200 x 1000");
-  clip =
-      ges_layer_add_asset (layer, GES_ASSET (asset1), 0 * GST_SECOND,
-      0 * GST_SECOND, 4 * GST_SECOND, GES_TRACK_TYPE_UNKNOWN);
-  gst_object_unref (asset1);
-  gst_object_unref (asset2);
-  g_object_set (clip, "vpattern", (gint) GES_VIDEO_TEST_PATTERN_SMPTE75, NULL);
-
-  /**
-   * Our track: 1200 x 1000
-   *
-   * 0--------------0
-   * | width : 1200 |
-   * | height: 1000 |
-   * 0--------------2
-   */
-
-  /* clip takes the size set on the track as a default */
-  fail_unless (check_frame_positioner_size (clip, 1200, 1000));
-
-  if (GES_IS_VIDEO_SOURCE (GES_CONTAINER_CHILDREN (clip)->data))
-    _set_track_element_width_height (GES_CONTAINER_CHILDREN (clip)->data, 1024,
-        768);
-
-  GST_DEBUG ("Setting clip size, should be 1024 x 768");
-
-  /**
-   * Our timeline : 1200 x 1000
-   *
-   * 0--------------0
-   * | width : 1024 |
-   * | height: 768  |
-   * 0--------------2
-   */
-
-  /* Clip has to comply to direct orders */
-  fail_unless (check_frame_positioner_size (clip, 1024, 768));
-
-  GST_DEBUG ("Changing caps, should still be 1024 x 768");
-
-  caps =
-      gst_caps_new_simple ("video/x-raw", "width", G_TYPE_INT, 1400, "height",
-      G_TYPE_INT, 1200, NULL);
-  ges_track_set_restriction_caps (trackv, caps);
-  gst_caps_unref (caps);
-
-  /**
-   * Our timeline : 1400 x 1200
-   *
-   * 0--------------0
-   * | width : 1024 |
-   * | height: 768  |
-   * 0--------------2
-   */
-
-  /* Clip still has to be the same size */
-  fail_unless (check_frame_positioner_size (clip, 1024, 768));
-
-  GST_DEBUG ("Setting width to 0, should be 1400 x 768");
-
-  /* -1 means don't set it, only valid here */
-  if (GES_IS_VIDEO_SOURCE (GES_CONTAINER_CHILDREN (clip)->data))
-    _set_track_element_width_height (GES_CONTAINER_CHILDREN (clip)->data, 0,
-        -1);
-
-  /**
-   * Our timeline : 1400 x 1200
-   *
-   * 0--------------0
-   * | width : 1400 |
-   * | height: 768  |
-   * 0--------------2
-   */
-
-  /* Clip width was set to 0 so it has to use track width */
-  /* Clip height is still directly set by the user */
-  fail_unless (check_frame_positioner_size (clip, 1400, 768));
-
-  GST_DEBUG ("Setting height to 0, should be 1400 x 1200");
-
-  if (GES_IS_VIDEO_SOURCE (GES_CONTAINER_CHILDREN (clip)->data))
-    _set_track_element_width_height (GES_CONTAINER_CHILDREN (clip)->data, -1,
-        0);
-
-  /**
-   * Our timeline : 1400 x 1200
-   *
-   * 0--------------0
-   * | width : 1400 |
-   * | height: 1200 |
-   * 0--------------2
-   */
-
-  /* Clip width still has to use track width */
-  /* Clip height was set to 0 so it has to use track height */
-  fail_unless (check_frame_positioner_size (clip, 1400, 1200));
-
-  GST_DEBUG ("Removing restriction on track height, should be 1400 x 240");
-
-  caps =
-      gst_caps_new_simple ("video/x-raw", "width", G_TYPE_INT, 1400, "height",
-      G_TYPE_INT, 0, NULL);
-  ges_track_set_restriction_caps (trackv, caps);
-  gst_caps_unref (caps);
-
-  /**
-   * Our timeline : 1400 x no restriction
-   *
-   * 0--------------0
-   * | width : 1400 |
-   * | height: 240  |
-   * 0--------------2
-   */
-
-  /* Clip width still has to use track width */
-  /* Clip height was set to 0 so it has to use natural clip height */
-  fail_unless (check_frame_positioner_size (clip, 1400, 0));
-
-  GST_DEBUG ("Removing restriction on track width, should be 320 x 240");
-
-  caps = gst_caps_new_empty_simple ("video/x-raw");
-  ges_track_set_restriction_caps (trackv, caps);
-  gst_caps_unref (caps);
-
-  /**
-   * Our timeline : no restriction x no restriction
-   *
-   * 0--------------0
-   * | width : 320  |
-   * | height: 240  |
-   * 0--------------2
-   */
-
-  /* Clip width was set to 0 so it has to use natural clip width */
-  /* Clip height was set to 0 so it has to use natural clip height */
-  fail_unless (check_frame_positioner_size (clip, 0, 0));
-
-
-  /**
-   * Our timeline : 320 * 240
-   *
-   * 0--------------0
-   * | width : 320  |
-   * | height: 240  |
-   * 0--------------2
-   */
-
-  /* We set the restriction caps video size to the same as the video source
-   * size. */
-  caps = gst_caps_from_string ("video/x-raw,height=240,width=320");
-  ges_track_set_restriction_caps (trackv, caps);
-  gst_caps_unref (caps);
-  _set_track_element_width_height (GES_CONTAINER_CHILDREN (clip)->data, 320,
-      240);
-
-  /* The video source has the same size as the track restriction caps but we
-   * are changing the aspect ratio, the video should thus not be rescaled. */
-  caps = gst_caps_from_string ("video/x-raw,height=1080,width=1920");
-  ges_track_set_restriction_caps (trackv, caps);
-  gst_caps_unref (caps);
-  fail_unless (check_frame_positioner_size (clip, 320, 240));
-
-  gst_object_unref (timeline);
-
-  ges_deinit ();
-}
-
-GST_END_TEST;
-
 static Suite *
 ges_suite (void)
 {
@@ -1411,7 +1160,6 @@ ges_suite (void)
   tcase_add_test (tc_chain, test_simple_triming);
   tcase_add_test (tc_chain, test_groups);
   tcase_add_test (tc_chain, test_snapping_groups);
-  tcase_add_test (tc_chain, test_scaling);
 
   return s;
 }
index 907fe89..20248f3 100644 (file)
@@ -71,6 +71,15 @@ foreach t : ges_tests
   endif
 endforeach
 
+if gstvalidate_dep.found()
+  scenarios = ['check_video_track_restriction_scale']
+
+  foreach scenario: scenarios
+    scenario_file = join_paths(meson.current_source_dir(), 'scenarios', scenario + '.scenario')
+    test(scenario, ges_launch, env: env, args: ['--set-scenario', scenario_file])
+  endforeach
+endif
+
 if build_gir
   # Make sure to use the subproject gst-validate-launcher if avalaible.
   if gstvalidate_dep.found() and gstvalidate_dep.type_name() == 'internal'
diff --git a/tests/check/scenarios/check_video_track_restriction_scale.scenario b/tests/check/scenarios/check_video_track_restriction_scale.scenario
new file mode 100644 (file)
index 0000000..97e40d5
--- /dev/null
@@ -0,0 +1,35 @@
+description, handles-states=true,
+    ges-options={\
+        --track-types, video,
+        --video-caps, "video/x-raw,width=1200,height=1000" \
+    }
+
+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
+
+set-child-properties, element-name=clip, width=1024, height=768
+check-child-properties, element-name=clip, width=1024, height=768
+
+set-track-restriction-caps, track-type=video, caps="video/x-raw,width=1400,height=1200"
+check-child-properties, element-name=clip, width=1024, height=768
+
+set-child-properties, element-name=clip, width=0
+check-child-properties, element-name=clip, width=0, height=768
+
+set-child-properties, element-name=clip, height=0
+check-child-properties, element-name=clip, width=0, height=0
+
+set-child-properties, element-name=clip, width=1400, height=1200
+check-child-properties, element-name=clip, width=1400, height=1200
+
+# Changing track size, keeping aspect ratio should scale the video source
+set-track-restriction-caps, track-type=video, caps="video/x-raw,width=700,height=600"
+check-child-properties, element-name=clip, width=700, height=600
+
+# The video source has the same size as the track restriction caps but we
+# are changing the aspect ratio, the video should thus not be rescaled. */
+set-track-restriction-caps, track-type=video, caps="video/x-raw,width=1920,height=1080"
+check-child-properties, element-name=clip, width=700, height=600
+
+stop
\ No newline at end of file
index 60fe5ed..4629165 100644 (file)
@@ -6,7 +6,7 @@ if gstvalidate_dep.found()
   ges_tool_args += ['-DGST_USE_UNSTABLE_API']
 endif
 
-executable('ges-launch-@0@'.format(apiversion),
+ges_launch = executable('ges-launch-@0@'.format(apiversion),
     'ges-validate.c', 'ges-launch.c', 'ges-launcher.c', 'utils.c',
     c_args : [ges_tool_args],
     dependencies : deps,