Re: [FFmpeg-devel] [PATCH 2/3] fftools/ffmpeg_graphprint: Add options for filtergraph printing

From: Soft Works <softworkz-at-hotmail.com@ffmpeg.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: Fri, 21 Feb 2025 13:49:10 +0000
Message-ID: <DM8P223MB03656B321A0F7AF83C1732F9BAC72@DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <Z7h7Gfp2ZOKUlnYp@phare.normalesup.org>

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Nicolas George
> Sent: Freitag, 21. Februar 2025 14:10
> 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
> 
> softworkz (HE12025-02-19):
> > From: softworkz <softworkz@hotmail.com>
> >
> > The key benefits are:
> >
> > - Different to other graph printing methods, this is outputting:
> >   - all graphs with runtime state
> >     (including auto-inserted filters)
> >   - each graph with its inputs and outputs
> >   - all filters with their in- and output pads
> >   - all connections between all input- and output pads
> >   - for each connection:
> >     - the runtime-negotiated format and media type
> >     - the hw context
> >     - if video hw context, both: hw pixfmt + sw pixfmt
> > - Output can either be printed to stdout or written to specified file
> > - Output is machine-readable
> > - Use the same output implementation as ffprobe, supporting multiple
> >   formats
> >
> > Note: This commit includes only the default and JSON writers.
> 
> This patch contains a lot of code copy-pasted from ffprobe. Moreover, it
> is copy-pasted from 2018 ffprobe, with seven years of bugfixes omitted.

Hello Nicolas,

Yes, this is all true, but of course I did a diff to current ffprobe code and
the number of bugfixes is exactly zero.
Probably it's not seven but 4 years, as I've likely done the same when
I had submitted it initially, in 2021.

> When the same code is needed in multiple parts of the project, it needs
> to be moved into a library with a proper API.

Strictly speaking, it's not a duplication because one part lives only 
In ffprobe and the other only in ffmpeg. Also, the submitted code
prints to a buffer and the ffprobe code prints directly to stdout or
a file.
Nonetheless, I agree of course that it would be great to unify it.
When I had submitted this patchset in 2021, you had said the same
thing, that you want to work on it, but now it's 4 years later and
it hasn't happened.

I understand that you would like to get in your new strings API into
the code base and my only reservation was that I find it a little bit
too clever/tricky which requires thinking around three corners
each time when trying to follow what's happening. Probably also,
your earlier string API (AVBPrint) is too good already to consider 
it urgent for replacement. Overall, I'm not against it, though.

But this can't be a blocker. I can stub out the writers to a separate
and shared code file if this is OK for you. I don't want to get in your
way of things you have already started, but it wouldn't be much 
more than a move, not the kind of rewrite you are aiming for.

I think you'll agree that this patchset cannot wait for something
which might or might not happen in the future.

Thank you
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".