From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTPS id A1F934DAAB for ; Fri, 28 Feb 2025 05:02:41 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id DEB6768DD5C; Fri, 28 Feb 2025 07:02:37 +0200 (EET) Received: from mail-pj1-f42.google.com (mail-pj1-f42.google.com [209.85.216.42]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id E092668D9C0 for ; Fri, 28 Feb 2025 07:02:25 +0200 (EET) Received: by mail-pj1-f42.google.com with SMTP id 98e67ed59e1d1-2f44353649aso2804590a91.0; Thu, 27 Feb 2025 21:02:25 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1740718942; x=1741323742; darn=ffmpeg.org; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=XoDeCUjx4q1s4CjmVQBJhYhyms2D/hY3Ey3TDZTQiWI=; b=PXqCERGUxyyC9H4yFNVsRlPTED1IWoVauvnWpN3GpB8m6jXEKFaqyK8nTQ7vgpPEw1 k6xcw/DylDJjGxTHb9HnMMrPMQbLj9YhcAJM4UexG4wxJI1kKsudLadpihGLULZd800i 9DW1pTH8+klLiDjhxu3uWx4uOX6ixoDKSLwEfLjvJ8bvRUtSDe2BFWYQqyM3rERhnX3a xTL2TwrrFlCHMSeG5SosLQdSf0+DMtRL5e6QF+rHXG1LncsQlWijnn29E38W2Ef1Moe9 Jx2QFja9c0R6NapkP7IUYx9jtTLj9iF1m1sjR/7Ow0aHQusZp8Z4pDt37Lnnm1f5E6mE oXvQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1740718942; x=1741323742; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=XoDeCUjx4q1s4CjmVQBJhYhyms2D/hY3Ey3TDZTQiWI=; b=vZrLpMFOR4KrLCkXXfJWVIRL7bO2XoHF/gRcJOTR3Jqa7Rlv8tNa74wH2RJxiFKcYJ gdsY7GS9XbIlKLRPsJba4X5dFKKpdtaFq6bxlqVJUWMmnC01qOAU8Nzc6Mc7jHzbTHHu XTW8sRGrrAqBPJR99S6WVxoO14X3POnJJWy3OWECWNBSSkxYe0yNMPkDACz/f95ez7tH A7arDWFl9AqulkHWsbjkc7M2Y9+pV8jJ/V8VLyvsUcOtAaVXNN4PCYaYnB9uZzAD5BhF SevFP00vO29JBMy1PO952k6/MILVlQfJVff/fmnVicCxHB17Egr0HeRr5wGFGNvbXAOt lecA== X-Forwarded-Encrypted: i=1; AJvYcCV/WQswFG+goMJtNxVY64FNTo7gWM7ijoOFPlAaG05oL30bt0mAx+pRMO1sd+VmD4MxlhcV9HS/OhM6utA7FonjYFS37FnZQ4Y=@ffmpeg.org X-Gm-Message-State: AOJu0YyTDiDvQSCFSuHGxlfNgh3UgBrDZC73Zo91gW7kqyjzRkK0/y4I SdZUISYgbhhBsVPYHw7bGcFqpNF0Zj/3YOABGYd7zUEEMiIPUt1ESBPBww== X-Gm-Gg: ASbGnct+1ZjW7CWYMIDha6zvXmlC+1dnHnavAb/YInvGs2ym3RR8a+DN1PuQvU1q6eb jJc7BIidRg0QHsjsR7tXgzDM8NI+IukcJ2oigNDHV8ZKu7woO409LWF0e4OkG6y9gymtAMkig0t Tlfi65H3RB7u1LY1T5Vmps4E4E6w6nCGUVNBzLwxw1n+dL4WwjBjZyvP76kBtfy3mZqOsXt2dwv RJ8o267EmXuTNPF9d5dbDlMn2y/LdOelghONvlxdZWKf3N/5IK82uwV/Ia5lfc7tG4MzbFO4Ku0 aN7/xxyLG46MBu9azyauF/7vu8fJpUD+ygTvzprxGb/j X-Google-Smtp-Source: AGHT+IHzwn0hkXayvm5G3iiNVKoX7+ACJwbWMH+2mD1UNjLhk1lh7vUP9ADm+vnAu226s5XGMx4S8Q== X-Received: by 2002:a05:6a21:7116:b0:1ee:d8c8:4b72 with SMTP id adf61e73a8af0-1f2f4d2f656mr3236790637.23.1740718942207; Thu, 27 Feb 2025 21:02:22 -0800 (PST) Received: from [127.0.0.1] (master.gitmailbox.com. [34.83.118.50]) by smtp.gmail.com with ESMTPSA id 41be03b00d2f7-aee7dedcf86sm2100892a12.64.2025.02.27.21.02.21 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 27 Feb 2025 21:02:21 -0800 (PST) Message-Id: In-Reply-To: References: From: ffmpegagent Date: Fri, 28 Feb 2025 05:02:08 +0000 Fcc: Sent MIME-Version: 1.0 To: ffmpeg-devel@ffmpeg.org Subject: [FFmpeg-devel] [PATCH v2 0/8] [RFC] avtextformat: Transform text writing into an independent API X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Cc: Michael Niedermayer , Soft Works , softworkz Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: 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 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".