Re: [FFmpeg-devel] [PATCH v9 0/4] print_graphs: Complete Filtergraph Printing

["softworkz ." <softworkz-at-hotmail.com@ffmpeg.org>]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Andreas Rheinhardt
> Sent: Mittwoch, 2. April 2025 09:25
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v9 0/4] print_graphs: Complete
> Filtergraph Printing
> 
> softworkz .:
> >
> >
> >> -----Original Message-----
> >> From: ffmpegagent <ffmpegagent@gmail.com>
> >> Sent: Sonntag, 23. März 2025 04:42
> >> To: ffmpeg-devel@ffmpeg.org
> >> Cc: softworkz <softworkz@hotmail.com>
> >> Subject: [PATCH v9 0/4] print_graphs: Complete Filtergraph Printing
> >>
> >> Due to the additional work on graph visualization (see
> >> https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2025-
> March/341296.html),
> >> I
> >> have removed the two graph printing commits from this patchset rather
> >> than
> >> adding to it, as to avoid wasting the effort already spent on
> reviewing.
> >>
> >> So, now it's a graph printing patchset without graph printing :-)
> >>
> >> All other commits are unchanged from the previous version. I'm also
> >> resubmitting to give it a final run of the tests on various
> platforms.
> >>
> >>
> >> V9
> >> ==
> >>
> >>  * Fix outdir creation for out-of-tree builds
> >>    (thanks, Michael)
> >>
> >> softworkz (4):
> >>   fftools/textformat: Extract and generalize textformat api from
> >>     ffprobe.c
> >>   fftools/ffprobe: Change to use textformat api
> >>   fftools/ffprobe: Rename writer_print_section_* and WriterContext
> >>   fftools/ffprobe: Rename AVTextFormatContext variables (w => tfc)
> >>
> >>  fftools/Makefile                   |   15 +-
> >>  fftools/ffprobe.c                  | 2296 +++++---------------------
> --
> >>  fftools/textformat/avtextformat.c  |  672 ++++++++
> >>  fftools/textformat/avtextformat.h  |  171 +++
> >>  fftools/textformat/avtextwriters.h |   68 +
> >>  fftools/textformat/tf_compact.c    |  282 ++++
> >>  fftools/textformat/tf_default.c    |  145 ++
> >>  fftools/textformat/tf_flat.c       |  174 +++
> >>  fftools/textformat/tf_ini.c        |  160 ++
> >>  fftools/textformat/tf_json.c       |  215 +++
> >>  fftools/textformat/tf_xml.c        |  221 +++
> >>  fftools/textformat/tw_avio.c       |  129 ++
> >>  fftools/textformat/tw_buffer.c     |   92 ++
> >>  fftools/textformat/tw_stdout.c     |   82 +
> >>  14 files changed, 2776 insertions(+), 1946 deletions(-)
> >>  create mode 100644 fftools/textformat/avtextformat.c
> >>  create mode 100644 fftools/textformat/avtextformat.h
> >>  create mode 100644 fftools/textformat/avtextwriters.h
> >>  create mode 100644 fftools/textformat/tf_compact.c
> >>  create mode 100644 fftools/textformat/tf_default.c
> >>  create mode 100644 fftools/textformat/tf_flat.c
> >>  create mode 100644 fftools/textformat/tf_ini.c
> >>  create mode 100644 fftools/textformat/tf_json.c
> >>  create mode 100644 fftools/textformat/tf_xml.c
> >>  create mode 100644 fftools/textformat/tw_avio.c
> >>  create mode 100644 fftools/textformat/tw_buffer.c
> >>  create mode 100644 fftools/textformat/tw_stdout.c
> >>
> >>
> >> base-commit: 0b097ed9f141f57e2b91f0704c721a9eff0204c0
> >> Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-
> >> ffstaging-52%2Fsoftworkz%2Fsubmit_print_graphs5-v9
> >> Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-
> >> ffstaging-52/softworkz/submit_print_graphs5-v9
> >> Pull-Request: https://github.com/ffstaging/FFmpeg/pull/52
> >>
> >> Range-diff vs v8:
> >>
> >>  1:  f379c77e88 = 1:  f379c77e88 fftools/textformat: Extract and
> >> generalize textformat api from ffprobe.c
> >>  2:  67bc38ac96 ! 2:  5ebe2a6c20 fftools/ffprobe: Change to use
> >> textformat api
> >>      @@ fftools/Makefile: OBJS-ffmpeg +=                  \
> >>        OBJS-ffplay += fftools/ffplay_renderer.o
> >>
> >>        define DOFFTOOL
> >>      +@@ fftools/Makefile: ifdef HAVE_GNU_WINDRES
> >>      + OBJS-$(1) += fftools/fftoolsres.o
> >>      + endif
> >>      + $(1)$(PROGSSUF)_g$(EXESUF): $$(OBJS-$(1))
> >>      +-$$(OBJS-$(1)): | fftools
> >>      ++$$(OBJS-$(1)): | fftools fftools/textformat
> >>      + $$(OBJS-$(1)): CFLAGS  += $(CFLAGS-$(1))
> >>      + $(1)$(PROGSSUF)_g$(EXESUF): LDFLAGS += $(LDFLAGS-$(1))
> >>      + $(1)$(PROGSSUF)_g$(EXESUF): FF_EXTRALIBS += $(EXTRALIBS-$(1))
> >>      +@@ fftools/Makefile: all: $(AVPROGS)
> >>      +
> >>      + fftools/ffprobe.o fftools/cmdutils.o: libavutil/ffversion.h |
> >> fftools
> >>      + OUTDIRS += fftools
> >>      ++OUTDIRS += fftools/textformat
> >>      +
> >>      + ifdef AVPROGS
> >>      + install: install-progs install-data
> >>
> >>        ## fftools/ffprobe.c ##
> >>       @@
> >>  3:  52c41fdcd0 = 3:  41dab9d31a fftools/ffprobe: Rename
> >> writer_print_section_* and WriterContext
> >>  4:  e85abafb4d = 4:  ae472da3e3 fftools/ffprobe: Rename
> >> AVTextFormatContext variables (w => tfc)
> >>
> >> --
> >> ffmpeg-codebot
> >
> >
> > Does anyone need more time to review this? Please let me know.
> > Anyway, I'll wait until next Monday at least.
> >
> What is the reason for the +800 net line diff? Is it just license
> headers in new files or did you actually add something new? (If so, this
> should be mentioned in the commit message.)
> 
> - Andreas
> 
> _______________________________________________

Hi Andreas,

I wanted to mention that there's an early version with step-by-step commits, showing how I had approached that transformation. It's been primarily a re-factoring rather than a rewrite, but with all the renaming on top, it's difficult to follow the changes. This patchset gives a better idea of how it was done: https://github.com/ffstaging/FFmpeg/pull/56

Thanks,
sw



_______________________________________________
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".