Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
structure: Add support for brackets as nested structures/caps specifiers
This introduces a more human friendly syntax to specify nested
structures It does so by using 2 different markers for opening and
closing them instead of abusing quotes which lead to requiring an insane
amount of escaping to match nesting levels.

The brackets (`[` and `]`) have been chosen as they avoid complex
constructions with curly brackets (or lower/higher than signs) where you
could have structures embedded inside arrays (which also use curly
brackets), ie. `s, array=(structure){{struct}}` should be parsed as an
array of structures, but the cast seems to imply something different. We
do not have this issue with brackets as they are currently used for
ranges, which can only be casted to numeric types.

This commit does not make use of that new syntax for serialization as
that would break backward compatibility, so it is basically a 'sugar'
syntax for humans. A notice has been explicitly made in the
documentation to let the user know about it.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/532>
  • Loading branch information
thiblahute committed Dec 4, 2020
1 parent 87ffe28 commit 322caf8
Show file tree
Hide file tree
Showing 5 changed files with 195 additions and 12 deletions.
2 changes: 1 addition & 1 deletion gst/gst_private.h
Expand Up @@ -193,7 +193,7 @@ G_GNUC_INTERNAL
void priv_gst_caps_features_append_to_gstring (const GstCapsFeatures * features, GString *s);

G_GNUC_INTERNAL
gboolean priv_gst_structure_parse_name (gchar * str, gchar **start, gchar ** end, gchar ** next);
gboolean priv_gst_structure_parse_name (gchar * str, gchar **start, gchar ** end, gchar ** next, gboolean check_valid);
G_GNUC_INTERNAL
gboolean priv_gst_structure_parse_fields (gchar *str, gchar ** end, GstStructure *structure);

Expand Down
2 changes: 1 addition & 1 deletion gst/gstcaps.c
Expand Up @@ -2400,7 +2400,7 @@ gst_caps_from_string_inplace (GstCaps * caps, const gchar * string)
break;
}

if (!priv_gst_structure_parse_name (s, &s, &end, &next)) {
if (!priv_gst_structure_parse_name (s, &s, &end, &next, FALSE)) {
g_free (copy);
return FALSE;
}
Expand Down
31 changes: 22 additions & 9 deletions gst/gststructure.c
Expand Up @@ -119,11 +119,15 @@
* a-struct, nested=(GstStructure)"nested-struct, nested=true"
* ```
*
* > *Note*: Be aware that the current #GstCaps / #GstStructure serialization
* > into string has limited support for nested #GstCaps / #GstStructure fields.
* > It can only support one level of nesting. Using more levels will lead to
* > unexpected behavior when using serialization features, such as
* > gst_caps_to_string() or gst_value_serialize() and their counterparts.
* Since 1.20, nested structures and caps can be specified using brackets
* (`[` and `]`), for example:
*
* ```
* a-struct, nested=[nested-struct, nested=true]
* ```
*
* > *note*: For backward compatility reason, the serialization functions won't
* > use that synthax.
*/

#ifdef HAVE_CONFIG_H
Expand Down Expand Up @@ -307,7 +311,6 @@ gst_structure_new_id_empty (GQuark quark)
return gst_structure_new_id_empty_with_size (quark, 0);
}

#ifndef G_DISABLE_CHECKS
static gboolean
gst_structure_validate_name (const gchar * name)
{
Expand Down Expand Up @@ -341,7 +344,6 @@ gst_structure_validate_name (const gchar * name)

return TRUE;
}
#endif

/**
* gst_structure_new_empty:
Expand Down Expand Up @@ -2215,7 +2217,7 @@ gst_structure_parse_field (gchar * str,

gboolean
priv_gst_structure_parse_name (gchar * str, gchar ** start, gchar ** end,
gchar ** next)
gchar ** next, gboolean check_valid)
{
char *w;
char *r;
Expand All @@ -2234,6 +2236,17 @@ priv_gst_structure_parse_name (gchar * str, gchar ** start, gchar ** end,
return FALSE;
}

if (check_valid) {
gchar save = *w;

*w = '\0';
if (!gst_structure_validate_name (*start)) {
*w = save;
return FALSE;
}
*w = save;
}

*end = w;
*next = r;

Expand Down Expand Up @@ -2339,7 +2352,7 @@ gst_structure_from_string (const gchar * string, gchar ** end)
copy = g_strdup (string);
r = copy;

if (!priv_gst_structure_parse_name (r, &name, &w, &r))
if (!priv_gst_structure_parse_name (r, &name, &w, &r, FALSE))
goto error;

save = *w;
Expand Down
83 changes: 82 additions & 1 deletion gst/gstvalue.c
Expand Up @@ -2644,6 +2644,87 @@ _priv_gst_value_parse_simple_string (gchar * str, gchar ** end)
return (s != str);
}

static gboolean
_priv_gst_value_parse_struct_or_caps (gchar * str, gchar ** after, GType type,
GValue * value)
{
gint openers = 1;
gboolean ret = FALSE;
gchar *s = str, t, *start, *end, *next;

if (*s != '[')
return FALSE;

s++;
str = s;
for (; *s; s++) {
if (*s == ']')
openers--;
else if (*s == '[')
openers++;

if (openers == 0) {
*after = s + 1;
break;
}
}

if (*after == NULL)
return FALSE;

t = *s;
*s = '\0';
g_value_init (value, type);
if (priv_gst_structure_parse_name (str, &start, &end, &next, TRUE))
ret = gst_value_deserialize (value, str);
if (G_UNLIKELY (!ret)) {
*s = t;
g_value_unset (value);
}

return ret;
}

static gboolean
_priv_gst_value_parse_range_struct_caps (gchar * s, gchar ** after,
GValue * value, GType type)
{
gint i;
gchar *tmp = s;
gboolean ret = FALSE;
GType try_types[] = {
GST_TYPE_STRUCTURE,
GST_TYPE_CAPS,
};

if (type == GST_TYPE_CAPS || type == GST_TYPE_STRUCTURE)
ret = _priv_gst_value_parse_struct_or_caps (tmp, &tmp, type, value);

if (ret)
goto ok;

tmp = s;
ret = _priv_gst_value_parse_range (tmp, &tmp, value, type);
if (ret)
goto ok;

if (type != G_TYPE_INVALID)
return ret;

for (i = 0; i < G_N_ELEMENTS (try_types); i++) {
tmp = s;
ret = _priv_gst_value_parse_struct_or_caps (tmp, &tmp, try_types[i], value);
if (ret)
goto ok;
}

return ret;

ok:
*after = tmp;
return ret;
}

gboolean
_priv_gst_value_parse_value (gchar * str,
gchar ** after, GValue * value, GType default_type, GParamSpec * pspec)
Expand Down Expand Up @@ -2697,7 +2778,7 @@ _priv_gst_value_parse_value (gchar * str,
while (g_ascii_isspace (*s))
s++;
if (*s == '[') {
ret = _priv_gst_value_parse_range (s, &s, value, type);
ret = _priv_gst_value_parse_range_struct_caps (s, &s, value, type);
} else if (*s == '{') {
g_value_init (value, GST_TYPE_LIST);
ret = _priv_gst_value_parse_list (s, &s, value, type, pspec);
Expand Down
89 changes: 89 additions & 0 deletions tests/check/gst/gstvalue.c
Expand Up @@ -3583,6 +3583,94 @@ GST_START_TEST (test_deserialize_with_pspec)

GST_END_TEST;

GST_START_TEST (test_deserialize_serialize_nested_structures)
{
gint i;
gchar *structure_str;
GstStructure *structure, *structure2;

/* *INDENT-OFF* */
struct
{
const gchar *serialized_struct;
gboolean should_fail;
const gchar *path_to_bool;
const gchar *subcaps_str;
} tests_data[] = {
{"s, substruct=[sub, is-deepest=true]", FALSE, "substruct"},
{"s, substruct=(structure) [sub, is-deepest=true]", FALSE, "substruct"},
{"s, substruct=[sub, is-substruct=true, subsubstruct=[subsub, is-deepest=true]]", FALSE, "substruct/subsubstruct"},
{"s, substruct=[sub, is-substruct=true, subsubstruct=[subsub, subsubsubstruct=[subsubsub, is-deepest=true]]]", FALSE, "substruct/subsubstruct/subsubsubstruct"},
{"s, substruct=[sub, an-array={a, b}, subsubstruct=[subsub, a-range=[1,2], a-string=\"this is a \\\"string\\\"\"]]", FALSE, NULL},
{"s, sub-caps=[nested-caps(some:Feature), is-caps=true; second, caps-structure=true]", FALSE, NULL, "nested-caps(some:Feature), is-caps=true; second, caps-structure=true"},
{"s, sub-caps=[nested-caps(some:Feature)]", FALSE, NULL, "nested-caps(some:Feature)"},
{"s, array=(structure){[struct, n=1], [struct, n=2]}"},
/* Broken structure with substructures */
{"s, substruct=[sub, is-substruct=true", TRUE},
{"s, substruct=[sub, is-substruct=true, sub=\"yes]", TRUE},
{"s, substruct=[sub, a-broken-string=$broken]", TRUE},
{"s, sub-caps=(int)[nested-caps(some:Feature)]", TRUE},
};
/* *INDENT-ON* */

for (i = 0; i < G_N_ELEMENTS (tests_data); i++) {
structure = gst_structure_new_from_string (tests_data[i].serialized_struct);
if (tests_data[i].should_fail) {
fail_if (structure, "%s not be deserialized",
tests_data[i].serialized_struct);
continue;
}
fail_unless (structure, "%s could not be deserialized",
tests_data[i].serialized_struct);
structure_str = gst_structure_to_string (structure);
structure2 = gst_structure_new_from_string (structure_str);
fail_unless (gst_structure_is_equal (structure, structure2));
g_free (structure_str);

if (tests_data[i].path_to_bool) {
const GstStructure *tmpstruct = structure;
gchar **tmpstrv = g_strsplit (tests_data[i].path_to_bool, "/", -1);
gint j;

for (j = 0; tmpstrv[j]; j++) {
const GValue *v = gst_structure_get_value (tmpstruct, tmpstrv[j]);

fail_unless (v, "Could not find '%s' in %s", tmpstrv[j],
gst_structure_to_string (tmpstruct));
tmpstruct = gst_value_get_structure (v);

fail_unless (GST_IS_STRUCTURE (tmpstruct));
if (!tmpstrv[j + 1]) {
gboolean tmp;

fail_unless (gst_structure_get_boolean (tmpstruct, "is-deepest", &tmp)
&& tmp);
}
}
g_strfreev (tmpstrv);
}
if (tests_data[i].subcaps_str) {
const GValue *v = gst_structure_get_value (structure, "sub-caps");
const GstCaps *caps = gst_value_get_caps (v);
GstCaps *caps2 = gst_caps_from_string (tests_data[i].subcaps_str);

fail_unless (gst_caps_is_equal (caps, caps2));
gst_caps_unref (caps2);
}

/* Ensure that doing a round trip works as expected */
structure_str = gst_structure_to_string (structure2);
gst_structure_free (structure2);
structure2 = gst_structure_new_from_string (structure_str);
fail_unless (gst_structure_is_equal (structure, structure2));
gst_structure_free (structure);
gst_structure_free (structure2);
g_free (structure_str);
}
}

GST_END_TEST;

static Suite *
gst_value_suite (void)
{
Expand Down Expand Up @@ -3638,6 +3726,7 @@ gst_value_suite (void)
tcase_add_test (tc_chain, test_transform_list);
tcase_add_test (tc_chain, test_serialize_null_aray);
tcase_add_test (tc_chain, test_deserialize_with_pspec);
tcase_add_test (tc_chain, test_deserialize_serialize_nested_structures);

return s;
}
Expand Down

0 comments on commit 322caf8

Please sign in to comment.