[FFmpeg-devel] [PATCH v2 0/8] [RFC] avtextformat: Transform text writing into an independent API
ffmpegagent
ffmpegagent at gmail.com
Fri Feb 28 07:02:08 EET 2025
Hello everybody,
this is an effort to pull out the text writing code from ffprobe and shape
it as an independent API. To avoid any other big refacoring in the future,
this patchset is building it up as a public API in avutil, but it's not that
I would be insisting this to happen (or maybe not yet). It could very well
remain private and only shared by ffprobe and ffmpeg (filtergraph printing)
for the time being. Would like to know what people are thinking about -
specifically and in general.
This patchset doesn't aim to improve or change the API itself - that would
be a bit too much at once. The primary functional changes made were those
which were required to gain independence of the specific way of use inside
ffprobe - without causing any change to the output. Locally, I had some
failing FATE tests locally but the CI build with FATE (here on GitHub)
finished without error.
Non-functional changes are naming, made in the spirit of ramping up for
becoming a public API (possibly), and there's also a separation of
functionality: Currently, everything is called "writer" in some way, without
making any distinctions. The main change is that there are now:
* Text Formatters: AVTextFormatter and AVTextFormatContext These are
obviously in charge of formatting the output (csv, json, xml, compact,
etc.)
* Text Writers: AVTextWriter and AVTextWriterContext Responsible for
writing the formatted text somewhere. Currently 3 writers exist
* stdouwriter
* aviowriter - can output to a file or to an existing avio context
* bufferwriter - writes to a provided AVBprint buffer
I'll be happy about any hints or comments!
Known Issues
* Mixes avutil and fftool changes in commits (I kept it preliminarily like
that as it's easier to follow. Otherwise it would be a huge single commit
to ffprobe.c)
* No changelog, ApiChange entries and version bumps
* Public API isn't documented yet
* Some variables haven't been renamed yet (like wctx to tctx)
* Not sure whether every single commit builds successfully, the final one
does
Open Questions
* I'm not sure about the best way to expose the formatters and writers to
consumers Currently, there's still the register_formatter() way in
ffprobe.c and writers are accessible only via creator functions (like
avtextwriter_create_avio)
* There are two header files (avtextformat.h, avtextwriters.h), reflecting
the separation betwen formatters and writers and a separate code file for
each formatter and writer, all in a subfolder named textformat. Not sure
what the preferences are, it was hard to find a comparable precedence.
Will change when asked to.
* Do we want flags to be #defines always or can they be enums?
* Will it need a public/private separation (like recently introduced for
other entities)?
Thanks
V2
* Fixed build and FATE issues. All should be green now (thanks, Michael for
the crash report)
softworkz (8):
ffprobe/avtextformat: Rename and move writer structs
ffprobe/avtextformat: Rename and move writer functions and options
ffprobe/avtextformat: Generalize ffprobe specifics
ffprobe/avtextformat: Rename flags and enums
ffprobe/avtextformat: Move flags
ffprobe/avtextformat: Rename writer to formatter
ffprobe/avtextformat: Move formatters to avutil
ffprobe/avtextformat: Split out text writers as independent classes
fftools/ffprobe.c | 2188 ++++-----------------------
libavutil/Makefile | 12 +
libavutil/avtextformat.h | 170 +++
libavutil/avtextwriters.h | 68 +
libavutil/textformat/avtextformat.c | 642 ++++++++
libavutil/textformat/tf_compact.c | 285 ++++
libavutil/textformat/tf_default.c | 150 ++
libavutil/textformat/tf_flat.c | 177 +++
libavutil/textformat/tf_ini.c | 165 ++
libavutil/textformat/tf_json.c | 220 +++
libavutil/textformat/tf_xml.c | 224 +++
libavutil/textformat/tw_avio.c | 129 ++
libavutil/textformat/tw_buffer.c | 91 ++
libavutil/textformat/tw_stdout.c | 82 +
14 files changed, 2738 insertions(+), 1865 deletions(-)
create mode 100644 libavutil/avtextformat.h
create mode 100644 libavutil/avtextwriters.h
create mode 100644 libavutil/textformat/avtextformat.c
create mode 100644 libavutil/textformat/tf_compact.c
create mode 100644 libavutil/textformat/tf_default.c
create mode 100644 libavutil/textformat/tf_flat.c
create mode 100644 libavutil/textformat/tf_ini.c
create mode 100644 libavutil/textformat/tf_json.c
create mode 100644 libavutil/textformat/tf_xml.c
create mode 100644 libavutil/textformat/tw_avio.c
create mode 100644 libavutil/textformat/tw_buffer.c
create mode 100644 libavutil/textformat/tw_stdout.c
base-commit: 99e2af4e7837ca09b97d93a562dc12947179fc48
Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-56%2Fsoftworkz%2Fsubmit_textformat-v2
Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-56/softworkz/submit_textformat-v2
Pull-Request: https://github.com/ffstaging/FFmpeg/pull/56
Range-diff vs v1:
1: 28b1c6d4cf ! 1: 442ab34094 ffprobe/avtextformat: Rename and move writer structs
@@ fftools/ffprobe.c: static void default_print_int(WriterContext *wctx, const char
}
-static const Writer default_writer = {
-+static const AVTextFormatter default_writer = {
++static const AVTextFormatter default_formatter = {
.name = "default",
.priv_size = sizeof(DefaultContext),
.print_section_header = default_print_section_header,
@@ fftools/ffprobe.c: static void xml_print_value(WriterContext *wctx, const char *
.name = "xml",
.priv_size = sizeof(XMLContext),
.init = xml_init,
+@@ fftools/ffprobe.c: static void writer_register_all(void)
+ return;
+ initialized = 1;
+
+- writer_register(&default_writer);
+- writer_register(&compact_writer);
+- writer_register(&csv_writer);
+- writer_register(&flat_writer);
+- writer_register(&ini_writer);
+- writer_register(&json_writer);
+- writer_register(&xml_writer);
++ writer_register(&default_formatter);
++ writer_register(&compact_formatter);
++ writer_register(&csv_formatter);
++ writer_register(&flat_formatter);
++ writer_register(&ini_formatter);
++ writer_register(&json_formatter);
++ writer_register(&xml_formatter);
+ }
+
+ #define print_fmt(k, f, ...) do { \
@@ fftools/ffprobe.c: static void writer_register_all(void)
memset( (ptr) + (cur_n), 0, ((new_n) - (cur_n)) * sizeof(*(ptr)) ); \
}
2: 89d538cab4 ! 2: f698715d66 ffprobe/avtextformat: Rename and move writer functions and options
@@ fftools/ffprobe.c: static void log_callback(void *ptr, int level, const char *fm
#define writer_w8(wctx_, b_) (wctx_)->writer_w8(wctx_, b_)
#define writer_put_str(wctx_, str_) (wctx_)->writer_put_str(wctx_, str_)
-@@ fftools/ffprobe.c: static void default_print_int(AVTextFormatContext *wctx, const char *key, int64_
- writer_printf(wctx, "%"PRId64"\n", value);
- }
-
--static const AVTextFormatter default_writer = {
-+static const AVTextFormatter default_formatter = {
- .name = "default",
- .priv_size = sizeof(DefaultContext),
- .print_section_header = default_print_section_header,
@@ fftools/ffprobe.c: static av_cold int xml_init(AVTextFormatContext *wctx)
return AVERROR(EINVAL); \
}
@@ fftools/ffprobe.c: static av_cold int xml_init(AVTextFormatContext *wctx)
return 0;
@@ fftools/ffprobe.c: static void writer_register_all(void)
- return;
- initialized = 1;
-
-- writer_register(&default_writer);
-- writer_register(&compact_writer);
-- writer_register(&csv_writer);
-- writer_register(&flat_writer);
-- writer_register(&ini_writer);
-- writer_register(&json_writer);
-- writer_register(&xml_writer);
-+ writer_register(&default_formatter);
-+ writer_register(&compact_formatter);
-+ writer_register(&csv_formatter);
-+ writer_register(&flat_formatter);
-+ writer_register(&ini_formatter);
-+ writer_register(&json_formatter);
-+ writer_register(&xml_formatter);
- }
-
#define print_fmt(k, f, ...) do { \
av_bprint_clear(&pbuf); \
av_bprintf(&pbuf, f, __VA_ARGS__); \
@@ libavutil/textformat/avtextformat.c (new)
+ { "fail", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = WRITER_STRING_VALIDATION_FAIL}, .unit = "sv" },
+ { "string_validation_replacement", "set string validation replacement string", OFFSET(string_validation_replacement), AV_OPT_TYPE_STRING, {.str=""}},
+ { "svr", "set string validation replacement string", OFFSET(string_validation_replacement), AV_OPT_TYPE_STRING, {.str="\xEF\xBF\xBD"}},
++ { NULL }
+};
+
+static void *trextcontext_child_next(void *obj, void *prev)
3: 624927c38b = 3: 01413dfbc0 ffprobe/avtextformat: Generalize ffprobe specifics
4: aa6b0ade81 ! 4: 857fd77ae7 ffprobe/avtextformat: Rename flags and enums
@@ libavutil/textformat/avtextformat.c: static const char *avtext_context_get_write
+ { "fail", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = AV_TEXTFORMAT_STRING_VALIDATION_FAIL}, .unit = "sv" },
{ "string_validation_replacement", "set string validation replacement string", OFFSET(string_validation_replacement), AV_OPT_TYPE_STRING, {.str=""}},
{ "svr", "set string validation replacement string", OFFSET(string_validation_replacement), AV_OPT_TYPE_STRING, {.str="\xEF\xBF\xBD"}},
- };
+ { NULL }
@@ libavutil/textformat/avtextformat.c: static inline int validate_string(AVTextFormatContext *wctx, char **dstp, const
invalid_chars_nb++;
5: fecd0456b0 = 5: 365ce9667e ffprobe/avtextformat: Move flags
6: c4b3bce223 ! 6: e59e16ab7c ffprobe/avtextformat: Rename writer to formatter
@@ fftools/ffprobe.c: static const AVOption xml_options[] = {
static av_cold int xml_init(AVTextFormatContext *wctx)
{
+@@ fftools/ffprobe.c: static AVTextFormatter xml_formatter = {
+ .priv_class = &xml_class,
+ };
+
+-static void writer_register_all(void)
++static void formatters_register_all(void)
+ {
+ static int initialized;
+
+@@ fftools/ffprobe.c: static void writer_register_all(void)
+ return;
+ initialized = 1;
+
+- writer_register(&default_formatter);
+- writer_register(&compact_formatter);
+- writer_register(&csv_formatter);
+- writer_register(&flat_formatter);
+- writer_register(&ini_formatter);
+- writer_register(&json_formatter);
+- writer_register(&xml_formatter);
++ formatter_register(&default_formatter);
++ formatter_register(&compact_formatter);
++ formatter_register(&csv_formatter);
++ formatter_register(&flat_formatter);
++ formatter_register(&ini_formatter);
++ formatter_register(&json_formatter);
++ formatter_register(&xml_formatter);
+ }
+
+ #define print_fmt(k, f, ...) do { \
@@ fftools/ffprobe.c: static void close_input_file(InputFile *ifile)
avformat_close_input(&ifile->fmt_ctx);
}
@@ fftools/ffprobe.c: int main(int argc, char **argv)
show_optional_fields, show_data_hash)) >= 0) {
- if (w == &xml_formatter)
- wctx->string_validation_utf8_flags |= AV_UTF8_FLAG_EXCLUDE_XML_INVALID_CONTROL_CODES;
-+ if (f == &avtextformatter_xml)
++ if (f == &xml_formatter)
+ tctx->string_validation_utf8_flags |= AV_UTF8_FLAG_EXCLUDE_XML_INVALID_CONTROL_CODES;
- avtext_print_section_header(wctx, NULL, SECTION_ID_ROOT);
@@ libavutil/avtextformat.h: struct AVTextFormatContext {
#define AV_TEXTFORMAT_PRINT_STRING_VALIDATE 2
-int avtext_context_open(AVTextFormatContext **pwctx, const AVTextFormatter *writer, const char *args,
-+int avtext_context_open(AVTextFormatContext **pwctx, const AVTextFormatter *formatter, const AVTextFormatter *writer, const char *args,
++int avtext_context_open(AVTextFormatContext **pwctx, const AVTextFormatter *formatter, const char *args,
const struct AVTextFormatSection *sections, int nb_sections,
const char *output_filename,
int show_value_unit,
@@ libavutil/textformat/avtextformat.c: int avtext_context_close(AVTextFormatContex
-int avtext_context_open(AVTextFormatContext **pwctx, const AVTextFormatter *writer, const char *args,
-+int avtext_context_open(AVTextFormatContext **pwctx, const AVTextFormatter *formatter, const AVTextFormatter *writer, const char *args,
++int avtext_context_open(AVTextFormatContext **pwctx, const AVTextFormatter *formatter, const char *args,
const struct AVTextFormatSection *sections, int nb_sections,
const char *output_filename,
int show_value_unit,
7: 47c4978975 ! 7: 53d68b7454 ffprobe/avtextformat: Move formatters to avutil
@@ fftools/ffprobe.c: static const AVTextFormatter *formatter_get_by_name(const cha
- .priv_class = &xml_class,
-};
-
--static void writer_register_all(void)
-+static void formatters_register_all(void)
+ static void formatters_register_all(void)
{
static int initialized;
-
-@@ fftools/ffprobe.c: static void writer_register_all(void)
+@@ fftools/ffprobe.c: static void formatters_register_all(void)
return;
initialized = 1;
-- writer_register(&default_formatter);
-- writer_register(&compact_formatter);
-- writer_register(&csv_formatter);
-- writer_register(&flat_formatter);
-- writer_register(&ini_formatter);
-- writer_register(&json_formatter);
-- writer_register(&xml_formatter);
+- formatter_register(&default_formatter);
+- formatter_register(&compact_formatter);
+- formatter_register(&csv_formatter);
+- formatter_register(&flat_formatter);
+- formatter_register(&ini_formatter);
+- formatter_register(&json_formatter);
+- formatter_register(&xml_formatter);
+ formatter_register(&avtextformatter_default);
+ formatter_register(&avtextformatter_compact);
+ formatter_register(&avtextformatter_csv);
@@ fftools/ffprobe.c: static void writer_register_all(void)
}
#define print_fmt(k, f, ...) do { \
+@@ fftools/ffprobe.c: int main(int argc, char **argv)
+ sections, FF_ARRAY_ELEMS(sections), output_filename, show_value_unit,
+ use_value_prefix, use_byte_value_binary_prefix, use_value_sexagesimal_format,
+ show_optional_fields, show_data_hash)) >= 0) {
+- if (f == &xml_formatter)
++ if (f == &avtextformatter_xml)
+ tctx->string_validation_utf8_flags |= AV_UTF8_FLAG_EXCLUDE_XML_INVALID_CONTROL_CODES;
+
+ avtext_print_section_header(tctx, NULL, SECTION_ID_ROOT);
## libavutil/Makefile ##
@@ libavutil/Makefile: OBJS = adler32.o \
8: 4aeb0ee9d3 ! 8: a47c649edf ffprobe/avtextformat: Split out text writers as independent classes
@@ libavutil/avtextformat.h: struct AVTextFormatContext {
#define AV_TEXTFORMAT_PRINT_STRING_OPTIONAL 1
#define AV_TEXTFORMAT_PRINT_STRING_VALIDATE 2
--int avtext_context_open(AVTextFormatContext **pwctx, const AVTextFormatter *formatter, const AVTextFormatter *writer, const char *args,
-+int avtext_context_open(AVTextFormatContext **pwctx, const AVTextFormatter *formatter, const AVTextWriterContext *writer, const char *args,
+-int avtext_context_open(AVTextFormatContext **pwctx, const AVTextFormatter *formatter, const char *args,
++int avtext_context_open(AVTextFormatContext **pwctx, const AVTextFormatter *formatter, AVTextWriterContext *writer, const char *args,
const struct AVTextFormatSection *sections, int nb_sections,
- const char *output_filename,
int show_value_unit,
@@ libavutil/textformat/avtextformat.c: int avtext_context_close(AVTextFormatContex
}
--int avtext_context_open(AVTextFormatContext **pwctx, const AVTextFormatter *formatter, const AVTextFormatter *writer, const char *args,
-+int avtext_context_open(AVTextFormatContext **pwctx, const AVTextFormatter *formatter, const AVTextWriterContext *writer_context, const char *args,
+-int avtext_context_open(AVTextFormatContext **pwctx, const AVTextFormatter *formatter, const char *args,
++int avtext_context_open(AVTextFormatContext **pwctx, const AVTextFormatter *formatter, AVTextWriterContext *writer_context, const char *args,
const struct AVTextFormatSection *sections, int nb_sections,
- const char *output_filename,
int show_value_unit,
--
ffmpeg-codebot
More information about the ffmpeg-devel
mailing list