From: Anton Khirnov <anton@khirnov.net>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Cc: Thilo Borgmann <thilo.borgmann@mail.de>
Subject: Re: [FFmpeg-devel] [PATCH 2/5] fftools/ffmpeg: move parsing of -stats_* specifiers to lavu/parseutils
Date: Wed, 13 Dec 2023 17:28:16 +0100
Message-ID: <170248489636.8914.14641721027481345642@lain.khirnov.net> (raw)
In-Reply-To: <e95e1147-5d14-4f79-9093-c108c3276f99@mail.de>
Quoting Thilo Borgmann via ffmpeg-devel (2023-12-13 13:50:09)
> Am 13.12.23 um 13:39 schrieb Anton Khirnov:
> > Quoting Thilo Borgmann via ffmpeg-devel (2023-12-13 13:15:27)
> >> Am 13.12.23 um 13:08 schrieb Anton Khirnov:
> >>> Quoting Thilo Borgmann via ffmpeg-devel (2023-12-13 13:05:35)
> >>>> Am 13.12.23 um 13:00 schrieb Anton Khirnov:
> >>>>> Quoting Thilo Borgmann via ffmpeg-devel (2023-12-11 16:07:22)
> >>>>>> ---
> >>>>>> fftools/ffmpeg.h | 31 +------
> >>>>>> fftools/ffmpeg_enc.c | 3 +-
> >>>>>> fftools/ffmpeg_mux_init.c | 152 +++-----------------------------
> >>>>>> libavutil/parseutils.c | 176 ++++++++++++++++++++++++++++++++++++++
> >>>>>> libavutil/parseutils.h | 102 ++++++++++++++++++++++
> >>>>>> libavutil/version.h | 2 +-
> >>>>>> 6 files changed, 296 insertions(+), 170 deletions(-)
> >>>>>
> >>>>> Absolutely not.
> >>>>>
> >>>>> This is application code and does not belong in the libraries.
> >>>>
> >>>> How else do we not have a redundant copy of all that and make sure that -stats_* options and the filter understand the same {..} directives?
> >>>
> >>> Why does that filter need to understand the same directives? No other
> >>> filter does.
> >>
> >> Because it is meant to use the file(s) the -stats_* option writes out. The most convenient and most error resilient way is to use the very same format string for -stats_* option as well as for the filter.
> >>
> >> Otherwise it could be a 'usual' scanf-format, but then the user has to translate it from one format into the other - without making mistakes.
> >> But that would also mean to update the filter (if someone realizes it) if the option ever changes.
> >
> > Why does it need a dynamic format at all? Just postulate a fixed format
> > for its input like every other filter and none of this complexity is
> > needed.
>
> That would unnecessarily limit the user of the -stats_* option to a
> specific format if the user also wants to use this filter later.
> Either sacrificing the other info the user wanted to process from the
> file created or having to run the same command (with distinct format
> strings for the -stats_* option) twice. Or do the conversion from his
> extensive format into the fixed format manually - without errors.
It is bad practice to design library features around the needs and
limitations of a single specific caller.
Many of the directives supported by -stats* make sense only in the
context of the ffmpeg CLI, and we may want to add many more in the
future. We definitely do not want to hardcode them into libavutil public
API.
If the problem is limiting ffmpeg CLI users to a specific stats format,
then you could extend these options to allow writing multiple stats
files with different formats.
--
Anton Khirnov
_______________________________________________
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".
next prev parent reply other threads:[~2023-12-13 16:28 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-11 15:07 [FFmpeg-devel] [PATCH 0/5] avfilter: Add fsync filter Thilo Borgmann via ffmpeg-devel
2023-12-11 15:07 ` [FFmpeg-devel] [PATCH 1/5] fftools/ffmpeg: split loop for parsing and validation of -stats_* specifiers Thilo Borgmann via ffmpeg-devel
2023-12-11 15:07 ` [FFmpeg-devel] [PATCH 2/5] fftools/ffmpeg: move parsing of -stats_* specifiers to lavu/parseutils Thilo Borgmann via ffmpeg-devel
2023-12-13 12:00 ` Anton Khirnov
2023-12-13 12:05 ` Thilo Borgmann via ffmpeg-devel
2023-12-13 12:08 ` Anton Khirnov
2023-12-13 12:15 ` Thilo Borgmann via ffmpeg-devel
2023-12-13 12:39 ` Anton Khirnov
2023-12-13 12:50 ` Thilo Borgmann via ffmpeg-devel
2023-12-13 16:28 ` Anton Khirnov [this message]
2023-12-13 18:17 ` Thilo Borgmann via ffmpeg-devel
2023-12-14 5:23 ` Anton Khirnov
2023-12-14 10:34 ` Thilo Borgmann via ffmpeg-devel
2023-12-14 17:51 ` Anton Khirnov
2023-12-14 18:42 ` Thilo Borgmann via ffmpeg-devel
2023-12-13 12:10 ` Nicolas George
2023-12-11 15:07 ` [FFmpeg-devel] [PATCH 3/5] reindent after last commit Thilo Borgmann via ffmpeg-devel
2023-12-11 15:07 ` [FFmpeg-devel] [PATCH 4/5] avfilter: Add fsync filter Thilo Borgmann via ffmpeg-devel
2023-12-11 15:14 ` Thilo Borgmann via ffmpeg-devel
2023-12-11 15:07 ` [FFmpeg-devel] [PATCH 5/5] fate: Add fsync filter tests Thilo Borgmann via ffmpeg-devel
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=170248489636.8914.14641721027481345642@lain.khirnov.net \
--to=anton@khirnov.net \
--cc=ffmpeg-devel@ffmpeg.org \
--cc=thilo.borgmann@mail.de \
/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