encodebin: Make use of the new preset_name when setting a preset

The behaviour is sensibly changed here. Instead of purely falling when a
preset is set on the #GstEncodingProfile, we now make sure that the
element that is plugged corresponds to the one specified as preset. Then,
if we have a preset_name, we use it, if it fails, we fail (we might rather
just keep working even without setting the element properties?)

 + Add tests that it behave correctly

diff --git a/gst-libs/gst/pbutils/encoding-profile.c b/gst-libs/gst/pbutils/encoding-profile.c
index b2221df..b8e9714 100644 (file)
--- a/gst-libs/gst/pbutils/encoding-profile.c
+++ b/gst-libs/gst/pbutils/encoding-profile.c
@@ -741,7 +741,7 @@ common_creation (GType objtype, GstCaps * format, const gchar * preset,
  * @description: (allow-none): The description of the container profile,
  *     can be %NULL
  * @format: The format to use for this profile
- * @preset: (allow-none): The preset to use for this profile
+ * @preset: (allow-none): The preset to use for this profile.
  *
  * Creates a new #GstEncodingContainerProfile.
  *

diff --git a/gst/encoding/gstencodebin.c b/gst/encoding/gstencodebin.c
index 0bee771..e226f94 100644 (file)
--- a/gst/encoding/gstencodebin.c
+++ b/gst/encoding/gstencodebin.c
@@ -817,18 +817,31 @@ beach:
 
 static GstElement *
 _create_element_and_set_preset (GstElementFactory * factory,
-    const gchar * preset, const gchar * name)
+    const gchar * preset, const gchar * name, const gchar * preset_name)
 {
   GstElement *res = NULL;
 
-  GST_DEBUG ("Creating element from factory %s", GST_OBJECT_NAME (factory));
+  GST_DEBUG ("Creating element from factory %s (preset factory name: %s"
+      " preset name: %s)", GST_OBJECT_NAME (factory), preset, preset_name);
+
   res = gst_element_factory_create (factory, name);
-  if (preset && GST_IS_PRESET (res) &&
-      !gst_preset_load_preset (GST_PRESET (res), preset)) {
-    GST_WARNING ("Couldn't set preset [%s] on element [%s]",
-        preset, GST_OBJECT_NAME (factory));
-    gst_object_unref (res);
-    res = NULL;
+  if (preset) {
+    if (g_strcmp0 (gst_object_get_name (GST_OBJECT (factory)), preset) == 0) {
+      if (preset_name) {
+        if (!gst_preset_load_preset (GST_PRESET (res), preset_name)) {
+          GST_WARNING ("Couldn't set preset [%s] on element [%s]",
+              preset, GST_OBJECT_NAME (factory));
+          gst_object_unref (res);
+          res = NULL;
+        }
+      } else {
+        GST_DEBUG ("Using a preset with no preset name, making use of the"
+            " proper element without setting any property");
+      }
+    } else {
+      gst_object_unref (res);
+      res = NULL;
+    }
   }
 
   return res;
@@ -842,10 +855,11 @@ _get_encoder (GstEncodeBin * ebin, GstEncodingProfile * sprof)
   GstElement *encoder = NULL;
   GstElementFactory *encoderfact = NULL;
   GstCaps *format;
-  const gchar *preset;
+  const gchar *preset, *preset_name;
 
   format = gst_encoding_profile_get_format (sprof);
   preset = gst_encoding_profile_get_preset (sprof);
+  preset_name = gst_encoding_profile_get_preset_name (sprof);
 
   GST_DEBUG ("Getting list of encoders for format %" GST_PTR_FORMAT, format);
 
@@ -867,7 +881,8 @@ _get_encoder (GstEncodeBin * ebin, GstEncodingProfile * sprof)
 
   for (tmp = encoders; tmp; tmp = tmp->next) {
     encoderfact = (GstElementFactory *) tmp->data;
-    if ((encoder = _create_element_and_set_preset (encoderfact, preset, NULL)))
+    if ((encoder = _create_element_and_set_preset (encoderfact, preset,
+                NULL, preset_name)))
       break;
   }
 
@@ -1524,10 +1539,11 @@ _get_formatter (GstEncodeBin * ebin, GstEncodingProfile * sprof)
   GstElement *formatter = NULL;
   GstElementFactory *formatterfact = NULL;
   GstCaps *format;
-  const gchar *preset;
+  const gchar *preset, *preset_name;
 
   format = gst_encoding_profile_get_format (sprof);
   preset = gst_encoding_profile_get_preset (sprof);
+  preset_name = gst_encoding_profile_get_preset_name (sprof);
 
   GST_DEBUG ("Getting list of formatters for format %" GST_PTR_FORMAT, format);
 
@@ -1546,7 +1562,8 @@ _get_formatter (GstEncodeBin * ebin, GstEncodingProfile * sprof)
         GST_OBJECT_NAME (formatterfact));
 
     if ((formatter =
-            _create_element_and_set_preset (formatterfact, preset, NULL)))
+            _create_element_and_set_preset (formatterfact, preset,
+                NULL, preset_name)))
       break;
   }
 
@@ -1591,10 +1608,11 @@ _get_muxer (GstEncodeBin * ebin)
   GstElementFactory *muxerfact = NULL;
   const GList *tmp;
   GstCaps *format;
-  const gchar *preset;
+  const gchar *preset, *preset_name;
 
   format = gst_encoding_profile_get_format (ebin->profile);
   preset = gst_encoding_profile_get_preset (ebin->profile);
+  preset_name = gst_encoding_profile_get_preset_name (ebin->profile);
 
   GST_DEBUG ("Getting list of muxers for format %" GST_PTR_FORMAT, format);
 
@@ -1645,7 +1663,8 @@ _get_muxer (GstEncodeBin * ebin)
     /* Only use a muxer than can use all streams and than can accept the
      * preset (which may be present or not) */
     if (cansinkstreams && (muxer =
-            _create_element_and_set_preset (muxerfact, preset, "muxer")))
+            _create_element_and_set_preset (muxerfact, preset, "muxer",
+                preset_name)))
       break;
   }
 
@@ -1940,12 +1959,12 @@ gst_encode_bin_setup_profile (GstEncodeBin * ebin, GstEncodingProfile * profile)
 
   g_return_val_if_fail (ebin->profile == NULL, FALSE);
 
-  GST_DEBUG ("Setting up profile %s (type:%s)",
+  GST_DEBUG ("Setting up profile %p:%s (type:%s)", profile,
       gst_encoding_profile_get_name (profile),
       gst_encoding_profile_get_type_nick (profile));
 
   ebin->profile = profile;
-  gst_mini_object_ref ((GstMiniObject *) ebin->profile);
+  gst_object_ref (ebin->profile);
 
   /* Create elements */
   res = create_elements_and_pads (ebin);
@@ -1960,7 +1979,7 @@ gst_encode_bin_set_profile (GstEncodeBin * ebin, GstEncodingProfile * profile)
 {
   g_return_val_if_fail (GST_IS_ENCODING_PROFILE (profile), FALSE);
 
-  GST_DEBUG_OBJECT (ebin, "profile : %s",
+  GST_DEBUG_OBJECT (ebin, "profile (%p) : %s", profile,
       gst_encoding_profile_get_name (profile));
 
   if (G_UNLIKELY (ebin->active)) {

diff --git a/tests/check/elements/encodebin.c b/tests/check/elements/encodebin.c
index a373223..3f4ac17 100644 (file)
--- a/tests/check/elements/encodebin.c
+++ b/tests/check/elements/encodebin.c
@@ -199,6 +199,56 @@ GST_START_TEST (test_encodebin_sink_pads_static)
 
 GST_END_TEST;
 
+GST_START_TEST (test_encodebin_sink_pads_preset_static)
+{
+  GstElement *ebin;
+  guint64 max_delay = 0;
+  GstPreset *oggmuxpreset;
+  GstEncodingProfile *prof;
+
+  /* Create an encodebin with a bogus preset and check it fails switching states */
+
+  ebin = gst_element_factory_make ("encodebin", NULL);
+  oggmuxpreset = GST_PRESET (gst_element_factory_make ("oggmux", NULL));
+
+  /* streamprofile that has a forced presence of 1 */
+  prof = create_ogg_vorbis_profile (1, NULL);
+  /* We also set the name as the load_preset call will reset the element name to
+   * what is described in the preset... which might not be very smart tbh */
+  g_object_set (oggmuxpreset, "max-delay", 12, "name", "testingoggmux", NULL);
+
+  /* Give a name someone should never use outside of that test */
+  gst_preset_save_preset (oggmuxpreset,
+      "test_encodebin_sink_pads_preset_static");
+
+  gst_encoding_profile_set_preset (prof, "oggmux");
+  gst_encoding_profile_set_preset_name (prof,
+      "test_encodebin_sink_pads_preset_static");
+
+  g_object_set (ebin, "profile", prof, NULL);
+
+  gst_encoding_profile_unref (prof);
+
+  /* It will go to READY... */
+  fail_unless_equals_int (gst_element_set_state (ebin, GST_STATE_READY),
+      GST_STATE_CHANGE_SUCCESS);
+  /* ... and to PAUSED */
+  fail_unless (gst_element_set_state (ebin, GST_STATE_PAUSED) !=
+      GST_STATE_CHANGE_FAILURE);
+
+  gst_child_proxy_get (GST_CHILD_PROXY (ebin), "testingoggmux::max-delay",
+      &max_delay, NULL);
+  fail_unless_equals_uint64 (max_delay, 12);
+
+  gst_element_set_state (ebin, GST_STATE_NULL);
+  gst_preset_delete_preset (oggmuxpreset,
+      "test_encodebin_sink_pads_preset_static");
+
+  gst_object_unref (ebin);
+};
+
+GST_END_TEST;
+
 GST_START_TEST (test_encodebin_sink_pads_nopreset_static)
 {
   GstElement *ebin;
@@ -884,6 +934,7 @@ encodebin_suite (void)
   tcase_add_test (tc_chain, test_encodebin_states);
   tcase_add_test (tc_chain, test_encodebin_sink_pads_static);
   tcase_add_test (tc_chain, test_encodebin_sink_pads_nopreset_static);
+  tcase_add_test (tc_chain, test_encodebin_sink_pads_preset_static);
   tcase_add_test (tc_chain, test_encodebin_sink_pads_dynamic);
   tcase_add_test (tc_chain, test_encodebin_sink_pads_multiple_static);
   tcase_add_test (tc_chain, test_encodebin_sink_pads_multiple_dynamic);