From 1a22ba50c19eacdd2e1427cf4feeae4f10781233 Mon Sep 17 00:00:00 2001 From: Stefano Sabatini Date: Fri, 13 Oct 2023 02:06:21 +0200 Subject: [PATCH] ffprobe: fix XML rendering, review XML layout Fix rendering of int values within a side data element, which was broken since commit d2d3a83ad93, where the side data element was correctly marked as a variable fields element. Logic to render a string variable was implemented already, but it was not implemented for the int fields path, which was enabled by that commit. Also, code and schema is changed in order to account for multiple variable-fields elements - such as side data, contained within the same parent. Previously it was assumed that a single variable-fields element was contained within the parent, which was the case for tags, but is not the case for side-data. Previously data was rendered as: Now as: Now variable-fields elements are rendered with a containing element containing generic key/values elements, enabling use of strict XML schema. Fix trac issue: https://trac.ffmpeg.org/ticket/10613 --- Changelog | 2 ++ doc/ffprobe.xsd | 31 +++++++++++++++++------- fftools/ffprobe.c | 49 ++++++++++++++++++++++++++++---------- tests/ref/fate/ffprobe_xml | 26 +++++++++++++------- 4 files changed, 78 insertions(+), 30 deletions(-) diff --git a/Changelog b/Changelog index 0c73f66546..259180e190 100644 --- a/Changelog +++ b/Changelog @@ -35,6 +35,8 @@ version : - CRI USM demuxer - ffmpeg CLI '-top' option deprecated in favor of the setfield filter - VAAPI AV1 encoder +- ffprobe XML output schema changed to account for multiple + variable-fields elements within the same parent element version 6.0: diff --git a/doc/ffprobe.xsd b/doc/ffprobe.xsd index 87ca265d63..aa2e870f70 100644 --- a/doc/ffprobe.xsd +++ b/doc/ffprobe.xsd @@ -43,9 +43,15 @@ - + + + + + + + @@ -69,14 +75,23 @@ + - - + + + + + + + + + + - + @@ -209,7 +224,7 @@ - + @@ -270,7 +285,7 @@ - + @@ -283,7 +298,7 @@ - + @@ -325,7 +340,7 @@ - + diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c index 40bb3f46e1..e490a9a9b2 100644 --- a/fftools/ffprobe.c +++ b/fftools/ffprobe.c @@ -269,7 +269,7 @@ static struct section sections[] = { [SECTION_ID_PACKET] = { SECTION_ID_PACKET, "packet", 0, { SECTION_ID_PACKET_TAGS, SECTION_ID_PACKET_SIDE_DATA_LIST, -1 } }, [SECTION_ID_PACKET_TAGS] = { SECTION_ID_PACKET_TAGS, "tags", SECTION_FLAG_HAS_VARIABLE_FIELDS, { -1 }, .element_name = "tag", .unique_name = "packet_tags" }, [SECTION_ID_PACKET_SIDE_DATA_LIST] ={ SECTION_ID_PACKET_SIDE_DATA_LIST, "side_data_list", SECTION_FLAG_IS_ARRAY, { SECTION_ID_PACKET_SIDE_DATA, -1 }, .element_name = "side_data", .unique_name = "packet_side_data_list" }, - [SECTION_ID_PACKET_SIDE_DATA] = { SECTION_ID_PACKET_SIDE_DATA, "side_data", SECTION_FLAG_HAS_VARIABLE_FIELDS|SECTION_FLAG_HAS_TYPE, { -1 }, .unique_name = "packet_side_data", .get_type = get_packet_side_data_type }, + [SECTION_ID_PACKET_SIDE_DATA] = { SECTION_ID_PACKET_SIDE_DATA, "side_data", SECTION_FLAG_HAS_VARIABLE_FIELDS|SECTION_FLAG_HAS_TYPE, { -1 }, .unique_name = "packet_side_data", .element_name = "side_datum", .get_type = get_packet_side_data_type }, [SECTION_ID_PIXEL_FORMATS] = { SECTION_ID_PIXEL_FORMATS, "pixel_formats", SECTION_FLAG_IS_ARRAY, { SECTION_ID_PIXEL_FORMAT, -1 } }, [SECTION_ID_PIXEL_FORMAT] = { SECTION_ID_PIXEL_FORMAT, "pixel_format", 0, { SECTION_ID_PIXEL_FORMAT_FLAGS, SECTION_ID_PIXEL_FORMAT_COMPONENTS, -1 } }, [SECTION_ID_PIXEL_FORMAT_FLAGS] = { SECTION_ID_PIXEL_FORMAT_FLAGS, "flags", 0, { -1 }, .unique_name = "pixel_format_flags" }, @@ -292,7 +292,7 @@ static struct section sections[] = { [SECTION_ID_STREAM_DISPOSITION] = { SECTION_ID_STREAM_DISPOSITION, "disposition", 0, { -1 }, .unique_name = "stream_disposition" }, [SECTION_ID_STREAM_TAGS] = { SECTION_ID_STREAM_TAGS, "tags", SECTION_FLAG_HAS_VARIABLE_FIELDS, { -1 }, .element_name = "tag", .unique_name = "stream_tags" }, [SECTION_ID_STREAM_SIDE_DATA_LIST] ={ SECTION_ID_STREAM_SIDE_DATA_LIST, "side_data_list", SECTION_FLAG_IS_ARRAY, { SECTION_ID_STREAM_SIDE_DATA, -1 }, .element_name = "side_data", .unique_name = "stream_side_data_list" }, - [SECTION_ID_STREAM_SIDE_DATA] = { SECTION_ID_STREAM_SIDE_DATA, "side_data", SECTION_FLAG_HAS_TYPE|SECTION_FLAG_HAS_VARIABLE_FIELDS, { -1 }, .unique_name = "stream_side_data", .get_type = get_packet_side_data_type }, + [SECTION_ID_STREAM_SIDE_DATA] = { SECTION_ID_STREAM_SIDE_DATA, "side_data", SECTION_FLAG_HAS_TYPE|SECTION_FLAG_HAS_VARIABLE_FIELDS, { -1 }, .unique_name = "stream_side_data", .element_name = "side_datum", .get_type = get_packet_side_data_type }, [SECTION_ID_SUBTITLE] = { SECTION_ID_SUBTITLE, "subtitle", 0, { -1 } }, }; @@ -1818,21 +1818,27 @@ static void xml_print_section_header(WriterContext *wctx, void *data) xml->within_tag = 0; writer_put_str(wctx, ">\n"); } - if (section->flags & SECTION_FLAG_HAS_VARIABLE_FIELDS) { - xml->indent_level++; - } else { + if (parent_section && (parent_section->flags & SECTION_FLAG_IS_WRAPPER) && wctx->level && wctx->nb_item[wctx->level-1]) writer_w8(wctx, '\n'); xml->indent_level++; - if (section->flags & SECTION_FLAG_IS_ARRAY) { - XML_INDENT(); writer_printf(wctx, "<%s>\n", section->name); + if (section->flags & (SECTION_FLAG_IS_ARRAY|SECTION_FLAG_HAS_VARIABLE_FIELDS)) { + XML_INDENT(); writer_printf(wctx, "<%s", section->name); + + if (section->flags & SECTION_FLAG_HAS_TYPE) { + AVBPrint buf; + av_bprint_init(&buf, 1, AV_BPRINT_SIZE_UNLIMITED); + av_bprint_escape(&buf, section->get_type(data), NULL, + AV_ESCAPE_MODE_XML, AV_ESCAPE_FLAG_XML_DOUBLE_QUOTES); + writer_printf(wctx, " type=\"%s\"", buf.str); + } + writer_printf(wctx, ">\n", section->name); } else { XML_INDENT(); writer_printf(wctx, "<%s ", section->name); xml->within_tag = 1; } - } } static void xml_print_section_footer(WriterContext *wctx) @@ -1846,8 +1852,6 @@ static void xml_print_section_footer(WriterContext *wctx) xml->within_tag = 0; writer_put_str(wctx, "/>\n"); xml->indent_level--; - } else if (section->flags & SECTION_FLAG_HAS_VARIABLE_FIELDS) { - xml->indent_level--; } else { XML_INDENT(); writer_printf(wctx, "\n", section->name); xml->indent_level--; @@ -1863,6 +1867,7 @@ static void xml_print_str(WriterContext *wctx, const char *key, const char *valu av_bprint_init(&buf, 1, AV_BPRINT_SIZE_UNLIMITED); if (section->flags & SECTION_FLAG_HAS_VARIABLE_FIELDS) { + xml->indent_level++; XML_INDENT(); av_bprint_escape(&buf, key, NULL, AV_ESCAPE_MODE_XML, AV_ESCAPE_FLAG_XML_DOUBLE_QUOTES); @@ -1873,6 +1878,7 @@ static void xml_print_str(WriterContext *wctx, const char *key, const char *valu av_bprint_escape(&buf, value, NULL, AV_ESCAPE_MODE_XML, AV_ESCAPE_FLAG_XML_DOUBLE_QUOTES); writer_printf(wctx, " value=\"%s\"/>\n", buf.str); + xml->indent_level--; } else { if (wctx->nb_item[wctx->level]) writer_w8(wctx, ' '); @@ -1887,9 +1893,26 @@ static void xml_print_str(WriterContext *wctx, const char *key, const char *valu static void xml_print_int(WriterContext *wctx, const char *key, long long int value) { - if (wctx->nb_item[wctx->level]) - writer_w8(wctx, ' '); - writer_printf(wctx, "%s=\"%lld\"", key, value); + XMLContext *xml = wctx->priv; + const struct section *section = wctx->section[wctx->level]; + + if (section->flags & SECTION_FLAG_HAS_VARIABLE_FIELDS) { + AVBPrint buf; + av_bprint_init(&buf, 1, AV_BPRINT_SIZE_UNLIMITED); + + xml->indent_level++; + XML_INDENT(); + av_bprint_escape(&buf, key, NULL, + AV_ESCAPE_MODE_XML, AV_ESCAPE_FLAG_XML_DOUBLE_QUOTES); + writer_printf(wctx, "<%s key=\"%s\"", + section->element_name, buf.str); + writer_printf(wctx, " value=\"%lld\"/>\n", value); + xml->indent_level--; + } else { + if (wctx->nb_item[wctx->level]) + writer_w8(wctx, ' '); + writer_printf(wctx, "%s=\"%lld\"", key, value); + } } static Writer xml_writer = { diff --git a/tests/ref/fate/ffprobe_xml b/tests/ref/fate/ffprobe_xml index 4e893edaa9..42abb08ea4 100644 --- a/tests/ref/fate/ffprobe_xml +++ b/tests/ref/fate/ffprobe_xml @@ -34,24 +34,32 @@ - - + + + + - - - + + + + + - + + + - - - + + + + + -- 2.34.1