From 7358cba01709a4b47ab0b167f102a68ac0aee09f Mon Sep 17 00:00:00 2001 From: Thibault Saunier Date: Wed, 5 Dec 2012 15:22:42 -0300 Subject: [PATCH] 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 --- gst-libs/gst/pbutils/encoding-profile.c | 2 +- gst/encoding/gstencodebin.c | 53 +++++++++++++++++-------- tests/check/elements/encodebin.c | 51 ++++++++++++++++++++++++ 3 files changed, 88 insertions(+), 18 deletions(-) diff --git a/gst-libs/gst/pbutils/encoding-profile.c b/gst-libs/gst/pbutils/encoding-profile.c index b2221df4b3..b8e9714294 100644 --- 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 0bee771eb6..e226f944b2 100644 --- 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 a373223c01..3f4ac1720a 100644 --- 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);