Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Nicolas George <george@nsup.org>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH 2/3] fftools/ffmpeg_graphprint: Add options for filtergraph printing
Date: Wed, 26 Feb 2025 15:42:42 +0100
Message-ID: <Z78oYvpM8_sU32SF@phare.normalesup.org> (raw)
In-Reply-To: <DM8P223MB0365A5D80D9B1A5750D9AB61BAC02@DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM>

Soft Works (HE12025-02-24):
> Accepted. I'll outfactor the writers first. It's a valid point of course; 
> I just wasn't sure whether you would dislike me doing it.

I dislike the fact that it will be done on top of a pedestrian or clumsy
strings API, but I know precisely where to lay the blame for that
absurdity and it is not you.

> Should each writer go into a separate code file, or all in one?

The writers in ffprobe are an ad-hoc construction to turn the shallow
data structures produced by ffprobe into a common denominator of JSON,
XML, CSV and a few custom formats. It is not a good candidate to be
turned into an API.

The writes make arbitrary decisions about how the data will be
represented. Not much in JSON, but for XML the choices of what is an
attribute and what is an element are very arbitrary. The XML writer in
ffprobe cannot output any (basic) XML file.

That part of the code is not duplicated in your patch anyway. What is
duplicated is the low-level code. So this is where you would be better
off starting.

That would mean something like this:

Move the low-level JSON writing code into lavu, with a low-level API
specific to JSON. Bonus points if the API can be used without dynamic
allocations when the data structure is not too deep.

Use the newly introduced av_json_write API in ffprobe; the JSON writer
becomes a trivial wrapper around it.

Also use it in your patch.

Later, possibly somebody else: do the same with XML. The low-level API
will need to be a little more complex, because XML has more room for
tweaks.

At some point in the future: unify the JSON, XML and other writers in
libavutil under a common API, but one that can be tweaked for any kind
of input data structure, not just a list of streams/packets/frames with
flat attributes.

Regards,

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

  reply	other threads:[~2025-02-26 14:42 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-19  9:59 [FFmpeg-devel] [PATCH 0/3] print_graphs: Complete Filtergraph Printing ffmpegagent
2025-02-19  9:59 ` [FFmpeg-devel] [PATCH 1/3] fftools/ffmpeg_filter: Move some declaration to new header file softworkz
2025-02-19  9:59 ` [FFmpeg-devel] [PATCH 2/3] fftools/ffmpeg_graphprint: Add options for filtergraph printing softworkz
2025-02-21  9:22   ` Andreas Rheinhardt
2025-02-21  9:42     ` Soft Works
2025-02-21 11:11       ` Andreas Rheinhardt
2025-02-21 11:25         ` Soft Works
2025-02-21 13:09   ` Nicolas George
2025-02-21 13:49     ` Soft Works
2025-02-24 10:41       ` Nicolas George
2025-02-24 13:19         ` Soft Works
2025-02-26 14:42           ` Nicolas George [this message]
2025-02-19  9:59 ` [FFmpeg-devel] [PATCH 3/3] fftools: Enable filtergraph printing and update docs softworkz
2025-02-21 11:27 ` [FFmpeg-devel] [PATCH v2 0/4] print_graphs: Complete Filtergraph Printing ffmpegagent
2025-02-21 11:27   ` [FFmpeg-devel] [PATCH v2 1/4] fftools/ffmpeg_filter: Move some declaration to new header file softworkz
2025-02-21 11:27   ` [FFmpeg-devel] [PATCH v2 2/4] avfilter/avfilter Add avfilter_link_get_hw_frames_ctx() softworkz
2025-02-21 11:27   ` [FFmpeg-devel] [PATCH v2 3/4] fftools/ffmpeg_graphprint: Add options for filtergraph printing softworkz
2025-02-21 11:27   ` [FFmpeg-devel] [PATCH v2 4/4] fftools: Enable filtergraph printing and update docs softworkz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Z78oYvpM8_sU32SF@phare.normalesup.org \
    --to=george@nsup.org \
    --cc=ffmpeg-devel@ffmpeg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git