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

[Nicolas George <george@nsup.org>]

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.
Copy-pasting non-trivial amounts of code is a big no, experienced
developers should know better. It is a big no among other reasons
precisely because bug fixes on one copy will likely be skipped on the
other copies.

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

For JSON, and more generally ffprobe's writers, since it was quite
obvious we need structured output at other places than ffprobe, I had
started working on it. IIRC, I had the JSON output done, with no dynamic
allocations if the structure is not too deep. But of course, since it
uses strings, it requires a good strings API, which is being blocked.

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